runtime: setevent failed; errno=6, fatal error: runtime.semawakeup #43720
Labels
FrozenDueToAge
NeedsInvestigation
Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
OS-Windows
release-blocker
Milestone
Comments
|
Another slightly different one: |
That's the error for invalid handle. Are we setevent'ing a closed handle somehow? |
|
I can't reproduce this so far. If you're able to trigger it easily, would you mind pasting the output for: diff --git a/src/runtime/os_windows.go b/src/runtime/os_windows.go
index d389d38ab9..ad4e744759 100644
--- a/src/runtime/os_windows.go
+++ b/src/runtime/os_windows.go
@@ -792,7 +792,7 @@ func semasleep(ns int64) int32 {
func semawakeup(mp *m) {
if stdcall1(_SetEvent, mp.waitsema) == 0 {
systemstack(func() {
- print("runtime: setevent failed; errno=", getlasterror(), "\n")
+ print("runtime: setevent failed; errno=", getlasterror(), ", mp.waitsema=", mp.waitsema, "\n")
throw("runtime.semawakeup")
})
} |
|
I reproduced it too finally. (Will try again with my own patch now.) |
|
Got it. |
|
I added the debug line to diff --git a/src/runtime/os_windows.go b/src/runtime/os_windows.go
index d389d38ab9..d893e68369 100644
--- a/src/runtime/os_windows.go
+++ b/src/runtime/os_windows.go
@@ -737,8 +737,10 @@ func semasleep(ns int64) int32 {
)
var result uintptr
+ var waitsema uintptr
if ns < 0 {
- result = stdcall2(_WaitForSingleObject, getg().m.waitsema, uintptr(_INFINITE))
+ waitsema = getg().m.waitsema
+ result = stdcall2(_WaitForSingleObject, waitsema, uintptr(_INFINITE))
} else {
start := nanotime()
elapsed := int64(0)
@@ -747,8 +749,9 @@ func semasleep(ns int64) int32 {
if ms == 0 {
ms = 1
}
+ waitsema = getg().m.waitsema
result = stdcall4(_WaitForMultipleObjects, 2,
- uintptr(unsafe.Pointer(&[2]uintptr{getg().m.waitsema, getg().m.resumesema})),
+ uintptr(unsafe.Pointer(&[2]uintptr{waitsema, getg().m.resumesema})),
0, uintptr(ms))
if result != _WAIT_OBJECT_0+1 {
// Not a suspend/resume event
@@ -774,13 +777,13 @@ func semasleep(ns int64) int32 {
case _WAIT_FAILED:
systemstack(func() {
- print("runtime: waitforsingleobject wait_failed; errno=", getlasterror(), "\n")
+ print("runtime: waitforsingleobject wait_failed; errno=", getlasterror(), ", waitsema=", waitsema, "\n")
throw("runtime.semasleep wait_failed")
})
default:
systemstack(func() {
- print("runtime: waitforsingleobject unexpected; result=", result, "\n")
+ print("runtime: waitforsingleobject unexpected; result=", result, ", waitsema=", waitsema, "\n")
throw("runtime.semasleep unexpected")
})
} |
|
@bcmills This seems like it might be a 1.16 blocker. |
|
When do we actually close these event objects? It seems like these might just leak every time the thread exits? |
|
I also found a remark in
I'm not sure, how relevant this is. I'm not aware that any thread would be creating any windows. |
|
I found an old forum post with a similar problem https://groups.google.com/g/golang-nuts/c/9Q70fcygg-c |
|
Change https://golang.org/cl/284132 mentions this issue: |
This doesn't appear to fully fix the issue, but it's at least related. |
|
I got an even crazier crash this time: |
|
I was trying whether Application Verifier gives me more information, but it doesn't look like it: |
|
So based on that Edit: this seems to be failing because, previously |
ALTree
added
the
NeedsInvestigation
Unclear yet whether it's related, but it would be a good thing to fix anyway. Can you send a CL with your error checking? That should squash other things down the line. I can see that bug being hit in api monitor too. |
|
Change https://golang.org/cl/284133 mentions this issue: |
|
At the moment I suspect that the handle is being closed by CreateProcess. |
|
So, what I've gathered is that the failures seem to happen where the |
|
Change https://golang.org/cl/284135 mentions this issue: |
|
@zx2c4, so one thing I'm not understanding:
Maybe, by closing the thread, it'll also take down the events that are used for waiting on locks? |
|
Does moving unminit below the lock taking fix it? Especially with https://go-review.googlesource.com/c/go/+/284132/ that's probably a good idea. |
|
Change https://golang.org/cl/284137 mentions this issue: |
|
@egonelbre I can still repro after the CL I just pushed. I still think that CL is a good idea though. |
|
I can reproduce the problem even with this: diff --git a/src/runtime/os_windows.go b/src/runtime/os_windows.go
index d66e30ee3c..30bc978f95 100644
--- a/src/runtime/os_windows.go
+++ b/src/runtime/os_windows.go
@@ -942,6 +942,7 @@ func minit() {
// Called from dropm to undo the effect of an minit.
//go:nosplit
func unminit() {
+ return
mp := getg().m
lock(&mp.threadLock)
stdcall1(_CloseHandle, mp.thread)So I don't think it's related to unminit. |
|
I still got CloseHandle failed on |
|
We posted at the same time. Objects created by the thread should keep existing after the thread is closed anyway. I don't think this is related to |
|
I'm slowly running out of ideas. I'm currently exploring, maybe the scheduler is scheduling on dead |
|
I wonder if there's some code that calls CloseHandle twice, which races with an interleaved semacreate, perhaps in exec/. Or any other code that calls CloseHandle twice erroneously. |
|
Hmm, so both |
|
But |
|
@aclements @ianlancetaylor I suspect that @egonelbre and I could use a fresh set of eyes on this one... We're both running out of ideas. |
|
Yeah, I think I've been looking for too long this and going slowly crazy with suspecting every tiny thing. :D |
gopherbot
pushed a commit
that referenced
this issue
Jan 15, 2021
These functions rely on DuplicateHandle succeeding, but they don't check the return value, which might be masking subtle bugs that cause other problems down the line. Updates #43720. Change-Id: I77f0e6645affa534777ffc173144a52e4afa5f81 Reviewed-on: https://go-review.googlesource.com/c/go/+/284135 Run-TryBot: Jason A. Donenfeld <[email protected]> Reviewed-by: Alex Brainman <[email protected]> Reviewed-by: Austin Clements <[email protected]> Trust: Alex Brainman <[email protected]> Trust: Jason A. Donenfeld <[email protected]>
|
It really smells like Really grasping at straws here. |
|
After running with static lock ranking, I got: |
|
Seems unrelated. That happens at startup only. |
|
If I disable |
|
I think I see the bug... |
|
Change https://golang.org/cl/284140 mentions this issue: |
|
My fix seems correct to me, but it doesn't actually fix the issue after enough iterations... |
|
Ah, finally happened to me as well. |
|
I just pushed a small change that uses DuplicateHandle instead of GetStdHandle. Can you test that? So far so good for me. |
|
Seems to be stable so far. After running 5x |
|
Likewise. I can't get it to crash. It just times out after 10 minutes per usual every time. |
|
After a few hours of this, it seems safe to conclude that https://golang.org/cl/284140 fixes the issue. |
gopherbot
pushed a commit
that referenced
this issue
Jan 18, 2021
Calls to lock may need to use global members of mOS that also need to be cleaned up before the thread exits. Before this commit, these resources would leak. Moving them to be cleaned up in unminit, however, would race with gstack on unix. So this creates a new helper, mdestroy, to release resources that must be destroyed only after locks are no longer required. We also move highResTimer lifetime to the same semantics, since it doesn't help to constantly acquire and release the timer object during dropm. Updates #43720. Change-Id: Ib3f598f3fda1b2bbcb608099616fa4f85bc1c289 Reviewed-on: https://go-review.googlesource.com/c/go/+/284137 Run-TryBot: Jason A. Donenfeld <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]> Reviewed-by: Austin Clements <[email protected]> Trust: Alex Brainman <[email protected]> Trust: Jason A. Donenfeld <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
and others
While trying to reproduce #42637 locally and ran into crashes, I was running
ostests on Windows with:Using e125ccd:
The tests failed with:
It seems to take some time before it fails, here are two additional traces:
CC: @alexbrainman, @zx2c4
The text was updated successfully, but these errors were encountered: