-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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.
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
x/build: windows gomote push failed to delete dist.exe #10706
Comments
I've noticed that after a couple minutes, this resolves itself. Why? Is there something we could be doing after the command is interrupted (in our |
When the HTTP connection hangs up, the buildlet calls cmd.Process.Kill() Hard to say what we could do beyond this. Maybe do the cmd.Wait in a On 6 May 2015 at 07:07, Brad Fitzpatrick notifications@github.com wrote:
|
I am trying to debug this, so I followed steps in golang.org/x/build/cmd/buildlet/README and I got stuck on:
The repo from https://go.googlesource.com/go/+archive/3b76b017cabb.tar.gz does not have .git directory. What do I need to do to get past this message? As to the reason of remove C:\workdir\go\src\cmd\dist\dist.exe: Access is denied. (just a guess) you kill process when connection is disconnected, but I suspect you don't disconnect properly. Does killing gomote process clears remote side of its TCP connection? Maybe it takes a while. You also don't check cmd.Process.Kill() returned error, perhaps, if you add more error checking / printing, we'll know more. But if you help me with error above maybe I can reproduce it here. Thank you. Alex |
The coordinator and gomote binaries push a VERSION file to the buildlets first, so git isn't required to build. The gomote binary accepts a "host:port" string as a name, to use it against localhost. |
Thank you Brad. I caught him (C:\workdir\go\src\cmd\dist\dist.exe) in the act. More details coming. Alex |
Exciting. :) |
You have used your jokes quota today. Please stop. Alex |
No, I'm actually excited. I love a good debugging mystery, and clues! Thanks for investigating this. |
I can see make.bat process gets killed, but none of its children are. cmd/dist.exe process runs to completion even after gomote is disconnected. That is why you saw "remove C:\workdir\go\src\cmd\dist\dist.exe: Access is denied." message, because you tried to rebuild executable for process that is still running - this is not allowed on Windows. But then cmd/dist.exe completes (it takes minute or so), and you can rebuild it then. Our mistake. We all (mainly me) assumed that killing process kills all its children. But that is not true on Windows. Not by default. I can come up with 2 ways to implement process tree killing. We can enumerate all processes on the system and kill all children of a process. See syscall.Getppid for something similar. Or we can use "recommended" way - associate our process with a job object https://msdn.microsoft.com/en-us/library/ms684161(VS.85).aspx We already used job objects before to gather performance stats on windows (golang.org/x/benchmarks/driver/driver_windows.go). I don't know how important it is to kill all children here in buildlet. All these computers are rebooted often anyway. Brad? Alex |
I want to fix this. The purpose of gomote is interactive development. If you can't stop a build and restart it quickly, it's very annoying. Rather than change Go for now, we can just change the buildlet to enumerate all the processes on the children. Thanks for the tips and debugging. |
CL https://golang.org/cl/9837 mentions this issue. |
Interrupting a Windows build with gomote seems to keep dist.exe running somewhere, preventing it from being deleted later:
Or maybe we need to delete harder somehow?
/cc @alexbrainman @adg @crawshaw
The text was updated successfully, but these errors were encountered: