x/tools/godoc: memory grows exponentially if symlink loop exists in $GOPATH #21061
Comments
|
Reading over that CL I don't see anything obvious causing the problem... maybe ReadDir is calling itself in a loop? So I think the next step may be to back it out, or run the program with -benchmem on and see where the allocations are occurring. |
|
Is there a symlink loop anywhere? |
|
What does
GODEBUG=gctrace=1 ./myAppReproducer
say. This will tell us if it is growth in the Go heap or growth somewhere
else.
…On Tue, Jul 18, 2017 at 2:12 AM, Daniel Martí ***@***.***> wrote:
Is there a symlink loop anywhere?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#21061 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AA7Wn8sa93HFBwAUCcjFHPWYJVJcAIl3ks5sPGjRgaJpZM4Oa4Nb>
.
|
|
I think there's a symlink loop somewhere because if I do this: diff --git a/godoc/corpus.go b/godoc/corpus.go
index f2c7ebbf..61408745 100644
--- a/godoc/corpus.go
+++ b/godoc/corpus.go
@@ -147,7 +147,7 @@ func (c *Corpus) Init() error {
}
func (c *Corpus) initFSTree() error {
- dir := c.newDirectory(pathpkg.Join("/", c.testDir), -1)
+ dir := c.newDirectory(pathpkg.Join("/", c.testDir), 30)
if dir == nil {
return errors.New("godoc: corpus fstree is nil")
}The memory usage settles at ~500 MB. That said, is a symlink loop that uncommon, and can we have godoc error (instead of growing RAM use infinitely) if it finds one? |
|
Did https://go-review.googlesource.com/c/45096/ help or hurt? |
|
Oh, interesting, these are pretty common Go libraries, I think. |
|
That change hurt. Running godoc on the commit immediately before it, memory settles at 200MB. Recompiling with that change and it grows exponentially. |
kevinburke
changed the title
|
I can take a go at fixing this... does anyone have any ideas about a solution? The dumb one would be to create a map of symlinks, e.g. map[existing]new. Any time we come across a new symlink, if the file it points to is in the "existing" map, we have a loop, and should stop following those links, and/or panic. I don't think we should panic because there are legitimate reasons to have symlink loops in your file tree, see above. Another (much easier to implement) solution would be to cap godoc directory recursion at, say, a depth of 20 or 30. |
|
How can I repro? For any given directory, we should only read it once, regardless of the path we got to it. Subsequent reads can just return empty results probably. |
|
It should be enough to clone the most recent version of go-bindata into your GOPATH, then recompile godoc with the tip of x/tools master, and restart it, you should see the memory just grow and grow. |
|
I think this may be a dup of #17344. There are memprofiles too, there. |
|
I've been running godoc locally for about a year now, I've had go-bindata
(and the other symlink loops) locally for over a year and I only saw it eat
all of my RAM in the last week or so. I can also make the problem go away
by reverting the CL mentioned above and reinstalling so I think this is a
recent issue.
On Wed, Jul 19, 2017 at 14:03 Alberto Donizetti ***@***.***> wrote:
I think this may be a dup of #17344
<#17344>. There are memprofiles too,
there.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#21061 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAOSI65x2vTccRjIFD68W408-HsnIAEWks5sPm8CgaJpZM4Oa4Nb>
.
--
…--
Kevin Burke
925.271.7005 | kev.inburke.com
|
|
@kevinburke sorry for the noise. I overlooked the fact that the commit you bisected this to was very recent. |
|
I can't reproduce. I go got go-bindata and ran godoc before & after. I see no change in memory. Definitely nothing growing "exponentially". What am I doing wrong? |
|
Okay, I removed the symlinks as well as chromium.googlesource.com/infra and now godoc settles at 300MB of memory usage - previously it settled at about 17. Still trying to repro the "exponential growth" section, though maybe it was just that with my old repo Godoc would prevously "settle" at, say, 10GB and I never actually got to that point. |
|
I can reproduce with an otherwise empty According to their docs, you can get it either vendored or not. I can reproduce with both.
Confirmed to be caused by CL 45096. Prior to commit, memory use is <30MB. After commit, memory grows to >15GB. I quit because I was about to start rapidly swapping to my fragile SSD. Memory growth appears to be arithmetic and unbounded. Web server doesn't start because it is continually scanning. In verbose mode, no info or errors are generated. I think go-bindata doesn't cause the issue because it only has one recursive symlink. etcd has two recursive symlinks:
If I remove the
If I remove the
etcd is the only package in my 1.5GB |
|
@broady, @ianlancetaylor, you probably want to revert https://go-review.googlesource.com/c/45096 for Go 1.9. |
bradfitz
added
the
NeedsFix
|
Change https://golang.org/cl/53634 mentions this issue: |
|
Reopening to port over to 1.9 release branch. |
|
I can confirm this as well to be caused by symlinks - in go1.9rc2 Memory: Strace on either pid shows: |
|
Change https://golang.org/cl/57870 mentions this issue: |
Revert https://golang.org/cl/45096. Original change description: godoc: follow symbolic links to folders in GOROOT Directory walking in godoc relies on ReadDir which returns the result of os.Lstat. Instead make the the OS VFS's ReadDir use os.Stat on symlinks before returning. Updates golang/go#15049 Fixes golang/go#21061 Change-Id: Ieaa7923d85842f3da5696a7f46134d16407dae66 Reviewed-on: https://go-review.googlesource.com/53634 Run-TryBot: Ian Lance Taylor <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]> Reviewed-on: https://go-review.googlesource.com/57870 Reviewed-by: Russ Cox <[email protected]>
ianlancetaylor
added
NeedsDecision
|
It looks like @broady cherry-picked the "don't follow symlinks" fix into the x/tools release-branch.go1.9 an hour before tagging go1.9 in the main repo. I assume that means this is fixed in Go 1.9 and we just forgot to close the issue? Can someone check and see if the blowup still exists? If so we need to figure out why - either we used a stale release-branch from x/tools or we didn't fix the bug. |
rsc
added
NeedsInvestigation
ianlancetaylor
added
NeedsDecision
|
I did this ($HOME/go1.9 is an unpacked go1.9.1 binary distribution, to make sure I'm using the godoc as built in the distribution). And then I visited http://localhost:6060/pkg/github.com/coreos/etcd/ and it does not show an infinite tree as described above. So I am going to conclude that godoc is fixed as of Go 1.9.1. It was probably also fixed as of Go 1.9, and the fact that the fix was in a subrepo just meant that this issue never got closed. Closing now. |
Revert https://golang.org/cl/45096. Original change description: godoc: follow symbolic links to folders in GOROOT Directory walking in godoc relies on ReadDir which returns the result of os.Lstat. Instead make the the OS VFS's ReadDir use os.Stat on symlinks before returning. Updates golang/go#15049 Fixes golang/go#21061 Change-Id: Ieaa7923d85842f3da5696a7f46134d16407dae66 Reviewed-on: https://go-review.googlesource.com/53634 Run-TryBot: Ian Lance Taylor <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]>
Please answer these questions before submitting your issue. Thanks!
What did you do?
Compile and run godoc as
godoc -http=:6060.What did you expect to see?
I expected the program to use a reasonable amount of memory; on my machine it gets to about 200 MB and then remains constant.
What did you see instead?
The program's memory consumption grew in an unbounded way; up to 30 GB RAM before I killed it. In addition, a number of error output lines (thousands) were streamed to the console:
Does this issue reproduce with the latest release (go1.8.3)?
Yes
System details
I've bisected the commits in x/tools and https://go-review.googlesource.com/c/45096/ is the first one where the program's memory begins growing.
The text was updated successfully, but these errors were encountered: