-
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
runtime: "PowerRegisterSuspendResumeNotification failed with errno= 87" when running in Windows docker containers #36557
Comments
Looks like error 87 is |
Does Go tooling need to be aware of system-sleep? Maybe we need a switch to disable runtime.monitorSuspendResume(). |
I think this errors means - this code is not supported on Docker - in this context. There are only so many Windows error messages available, so, as a programmer, you have to pick best exiting number that fits. And ERROR_INVALID_PARAMETERS fits much better than ERROR_FILE_NOT_FOUND. We are looking for ERROR_FILE_NOT_FOUND returned from PowerRegisterSuspendResumeNotification at this moment to detect Docker environment. I think we should add ERROR_INVALID_PARAMETERS to the list of errors we are looking for there. Or maybe just ignore any errors returned from PowerRegisterSuspendResumeNotification, as @zx2c4 originally suggested. It does not feel right ignoring all errors on non-Docker. Unrelated, but maybe we should run one of our builders in Windows Docker container. I believe, Windows Docker containers are available on GCP. Maybe having Docker builder will pick errors like this. @bradfitz what do you think? Alex PS: @ianlancetaylor I normally google for |
Change https://golang.org/cl/214917 mentions this issue: |
@gopherbot Please open backport issues. This same code was added to the 1.12 and 1.13 branches, and should be fixed there as well. |
Backport issue(s) opened: #36574 (for 1.12), #36575 (for 1.13). Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases. |
I still sort of think this is best. If we fail silently for some bad reason, how bad is it actually? And if the power notifier is borked, how much else from the power stack is borked? We can roll with this and see if we get additional reports at some point, I guess, but if this becomes a frequent thing, it doesn't seem like that bad of an assumption that power management notifier failures are always related to the kernel not advancing the clock anyway for WaitFor*Object and such. |
Change https://golang.org/cl/215002 mentions this issue: |
Change https://golang.org/cl/215017 mentions this issue: |
… on Windows Docker Updates #36557 Fixes #36575 Change-Id: Ia8125f382d5e14e5612da811268a58971cc9ac08 Reviewed-on: https://go-review.googlesource.com/c/go/+/214917 Run-TryBot: Ian Lance Taylor <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]> Reviewed-by: Jason A. Donenfeld <[email protected]> Reviewed-by: Austin Clements <[email protected]> (cherry picked from commit d2de9bd) Reviewed-on: https://go-review.googlesource.com/c/go/+/215002
… on Windows Docker Updates #36557 Fixes #36574 Change-Id: Ia8125f382d5e14e5612da811268a58971cc9ac08 Reviewed-on: https://go-review.googlesource.com/c/go/+/214917 Run-TryBot: Ian Lance Taylor <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]> Reviewed-by: Jason A. Donenfeld <[email protected]> Reviewed-by: Austin Clements <[email protected]> (cherry picked from commit d2de9bd) Reviewed-on: https://go-review.googlesource.com/c/go/+/215017
You assume that if PowerRegisterSuspendResumeNotification returns error than Windows power stack is broken. But I am not convinced that this is true. It seems that way on Docker, but we haven't had any reports of PowerRegisterSuspendResumeNotification error on non Docker. So it seems to be working fine on non Docker, and we should care about any PowerRegisterSuspendResumeNotification failure on Windows. And things might change in time. For example, Microsoft might decide to implement PowerRegisterSuspendResumeNotification in Docker. Also maybe we could somehow determine, if we running inside Docker, and only ignore errors, if in Docker. https://stackoverflow.com/questions/43002803/detect-if-process-executes-inside-a-windows-container And I was under impression that you cared about Windows programs clocks, because of WireGuard. How can you be certain about clocks, if you ignore PowerRegisterSuspendResumeNotification errors? You know it is broken in Docker. But you still want to be sure about your Windows clients. Aren't you? Alex |
Change https://golang.org/cl/219657 mentions this issue: |
It appears that PowerRegisterSuspendResumeNotification is not supported when running inside Docker - see issues #35447, #36557 and #37149. Our current code relies on error number to determine Docker environment. But we already saw PowerRegisterSuspendResumeNotification return ERROR_FILE_NOT_FOUND, ERROR_INVALID_PARAMETERS and ERROR_ACCESS_DENIED (see issues above). So this approach is not sustainable. Just ignore PowerRegisterSuspendResumeNotification returned error. Fixes #37149 Change-Id: I2beba9d45cdb8c1efac5e974e747827a6261915a Reviewed-on: https://go-review.googlesource.com/c/go/+/219657 Run-TryBot: Alex Brainman <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Austin Clements <[email protected]> Reviewed-by: Jason A. Donenfeld <[email protected]>
Change https://golang.org/cl/224585 mentions this issue: |
Change https://golang.org/cl/224586 mentions this issue: |
…erSuspendResumeNotification It appears that PowerRegisterSuspendResumeNotification is not supported when running inside Docker - see issues #35447, #36557 and #37149. Our current code relies on error number to determine Docker environment. But we already saw PowerRegisterSuspendResumeNotification return ERROR_FILE_NOT_FOUND, ERROR_INVALID_PARAMETERS and ERROR_ACCESS_DENIED (see issues above). So this approach is not sustainable. Just ignore PowerRegisterSuspendResumeNotification returned error. For #37149 Fixes #37230 Change-Id: I2beba9d45cdb8c1efac5e974e747827a6261915a Reviewed-on: https://go-review.googlesource.com/c/go/+/219657 Run-TryBot: Alex Brainman <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Austin Clements <[email protected]> Reviewed-by: Jason A. Donenfeld <[email protected]> (cherry picked from commit d467f3b) Reviewed-on: https://go-review.googlesource.com/c/go/+/224585 Run-TryBot: Ian Lance Taylor <[email protected]>
…erSuspendResumeNotification It appears that PowerRegisterSuspendResumeNotification is not supported when running inside Docker - see issues #35447, #36557 and #37149. Our current code relies on error number to determine Docker environment. But we already saw PowerRegisterSuspendResumeNotification return ERROR_FILE_NOT_FOUND, ERROR_INVALID_PARAMETERS and ERROR_ACCESS_DENIED (see issues above). So this approach is not sustainable. Just ignore PowerRegisterSuspendResumeNotification returned error. For #37149 Fixes #37699 Change-Id: I2beba9d45cdb8c1efac5e974e747827a6261915a Reviewed-on: https://go-review.googlesource.com/c/go/+/219657 Run-TryBot: Alex Brainman <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Austin Clements <[email protected]> Reviewed-by: Jason A. Donenfeld <[email protected]> (cherry picked from commit d467f3b) Reviewed-on: https://go-review.googlesource.com/c/go/+/224586 Run-TryBot: Ian Lance Taylor <[email protected]>
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
yes
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
Trying to build an application for windows inside of a Windows docker container, using the following as the base image:
golang:1.13.6-windowsservercore-1809
What did you expect to see?
I expected the build to be successful without a PowerRegisterSuspendResumeNotification failure
What did you see instead?
The text was updated successfully, but these errors were encountered: