runtime: Timer buckets may get "stuck" for long periods of time after Windows 8/10 systems wake from sleep [1.13 backport]

[gopherbot]

@zx2c4 requested issue #31528 to be considered for backport to the next 1.13 minor release.

@gopherbot Please backport this to 1.13

[gopherbot]

Change https://golang.org/cl/193607 mentions this issue: [release-branch.go1.13] runtime: monitor for suspend/resume to kick timeouts

[ianlancetaylor]

This seems like a lot of complex changes to the Windows support. As far as I can tell, it doesn't fix a regression. I'm not convinced this should go onto the release branch without a lot more testing on different versions of Windows.

[zx2c4]

On Windows 7 it's a no'op.

On Windows 10 it works. On Windows 8.1 it works.

Without this, Go does not work on Windows laptops. That's not very good.

[ianlancetaylor]

Are you saying that Go has never worked on Windows laptops?

[zx2c4]

The code was written in the Windows 7 era and it worked then. But then Microsoft changed things and Go stopped working.

So it's definitely a regression of sorts, the key oddity being that it's a Microsoft-induced regression rather than a Go-induced regression. I'm not sure how this factors into the backporting logic, but I would hope it does in the affirmative.

Otherwise I have to tell consumers of my library, "if you intend to use this on a computer with S3, copy GOROOT into a local folder, patch it with this odd patch, set GOROOT to that local folder, and then compile," which isn't super friendly.

[zx2c4]

CC @alexbrainman

[jmontgomery-jc]

I agree with @zx2c4 that this should ideally be backported to 1.13. It's a very subtle bug in that you're not going to usually see crashes or errors returned or anything overt like that but it causes programs to behave badly in ways that are very hard to debug and reproduce. I would have never been able to get enough information to file the original issue had I not been lucky enough to encounter it on a VM that I could snapshot and debug over the course of a few days. Because timers are so central to how Go works the only workaround is using a patched build of Go to build your programs.

[alexbrainman]

Are you saying that Go has never worked on Windows laptops?

What @zx2c4 said.

Go uses WaitForSingleObject Windows API to wait for periods of time. The API works properly on some Windows versions, but does not work on others. From https://docs.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-waitforsingleobject (I made important bits in bold)

Windows XP, Windows Server 2003, Windows Vista, Windows 7, Windows Server 2008 and Windows Server 2008 R2: The dwMilliseconds value does include time spent in low-power states. For example, the timeout does keep counting down while the computer is asleep.

Windows 8, Windows Server 2012, Windows 8.1, Windows Server 2012 R2, Windows 10 and Windows Server 2016: The dwMilliseconds value does not include time spent in low-power states. For example, the timeout does not keep counting down while the computer is asleep.

So when Go programs sleeps on new versions of Windows, it does not account for time laptop is in low-powered state.

We did not know about that until #31528. So it was always broken this way for Windows 10 users. And always worked properly (still works) for Windows 7 users.

But I also agree that the change is not trivial. And the change is just 1 week old. And it only lives on current master. I doubt many users tested it yet.

Alex

[zx2c4]

But I also agree that the change is not trivial. And the change is just 1 week old. And it only lives on current master. I doubt many users tested it yet.

I'm shipping the patch to a somewhat substantial userbase now, which has a pretty good track record of turning up bugs and emailing me about them. If you'd like, we can just leave this GH issue open for a few weeks, and I'll pipe up if folks encounter problems.

[alexbrainman]

I'm shipping the patch to a somewhat substantial userbase now, which has a pretty good track record of turning up bugs and emailing me about them. If you'd like, we can just leave this GH issue open for a few weeks, and I'll pipe up if folks encounter problems.

Sounds good to me. Thank you.

Alex

[connected-nkozul]

Chiming in with the same comment from go-review #191957:

Confirmed on two Windows 10 machines (1903 and 1809). Placing the system in an S3 sleep state does indeed interfere with time.Sleep(). The latest provided patchset fixes the issue.

I'm part of a project which is very much reliant on working around this issue manually, which does not make for a sane pipeline. Would really love to see this in a minor release of 1.13.

[networkimprov]

Could we also backport to 1.12, where the issue was first reported? I believe the patch has already been tested thereon.

EDIT: the above patch doesn't apply cleanly to 1.12.5 due to an unrelated change.