Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cmd/go: some temp dirs not cleaned up #19449

Closed
josharian opened this issue Mar 8, 2017 · 6 comments
Closed

cmd/go: some temp dirs not cleaned up #19449

josharian opened this issue Mar 8, 2017 · 6 comments
Labels
FrozenDueToAge help wanted Suggested Issues that may be good for new contributors looking for work to do.
Milestone
Go1.9Maybe

Comments

@josharian
Copy link
Contributor

Clean your temp dir, run all.bash, and examine your temp dir. There are a few new temp files left not cleaned up. Some are from pprof and will hopefully soon be fixed upstream (google/pprof#108).

I also see these:

drwx------  2 josh  staff   68 Mar  7 15:55 go-build943960338
drwx------  2 josh  staff   68 Feb 19 03:46 go-link-974961831
drwx------  3 josh  staff  102 Mar  4 18:50 gotest068114942
drwx------  3 josh  staff  102 Feb 20 15:12 gotest796150247

I speculate that these are created by the cmd/go tests and/or test dir tests that run with -x, but I haven't investigated.
@josharian josharian added help wanted Suggested Issues that may be good for new contributors looking for work to do. labels Mar 8, 2017
@josharian josharian added this to the Go1.9Maybe milestone Mar 8, 2017
@mostynb
Copy link
Contributor

mostynb commented Apr 16, 2017

As of commit a306a85, I don't see the stray go-link-* or gotest* directories. The stray go-build* directory appears to be created by cmd/go/internal/work/build.go's Init() function, but I have not yet tracked down why. There are > 3000 calls to this function, only one of them does this AFAICT (good excuse for me to learn to use a go debugger, I suppose).

@mostynb
Copy link
Contributor

mostynb commented Apr 16, 2017

I couldn't figure out a way to get stack traces inside cmd/go/internal/work/build.go (not sure why it's not possible to import runtime/debug there?), but a git bisect run narrowed this down to e9bb9e5. If I remove the TestRespectGroupSticky function that was added by that commit, there are no stray go-build* temp directories left behind.

@josharian
Copy link
Contributor Author

@mostynb nice digging! And that points to the solution--looks to me like in moveOrCopyFile, if we choose the copy path, instead of just copying, we need to copy and then unlink the old.

@mostynb
Copy link
Contributor

mostynb commented Apr 16, 2017

I'm not familiar with this code (I'm new to go), so I was looking into calling base.Fatalf instead of testing.T.Fatalf inside TestRespectGroupSticky - this seems to work, because the AtExit handling is done by base. But I'm not sure if this breaks the assumptions of the test setup.

I will play around with your suggested change to moveOrCopyFile...

@josharian
Copy link
Contributor Author

I would hope that none of the Fatalfs are actually being reached. :) But if there's a custom Fatalf present, we should probably use it regardless.

But I don't know much about this code. Dig in and figure it out! You have a good start. :)

@gopherbot
Copy link

CL https://golang.org/cl/40912 mentions this issue.

@golang golang locked and limited conversation to collaborators Apr 17, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge help wanted Suggested Issues that may be good for new contributors looking for work to do.
Projects
None yet
Milestone
Go1.9Maybe
Development

No branches or pull requests
3 participants
@josharian @mostynb @gopherbot