-
Notifications
You must be signed in to change notification settings - Fork 17.5k
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
os/signal: "morestack on gsignal" on NetBSD #19652
Comments
And also on the build.golang.org dashboard: And: |
I don't have a lot of context on how If it only checks for stack size, then we're looking for a path in the signal handler that fails to restore If it also checks for GC preemption, then I suspect we just need more |
GC (and non-GC) preemption is implemented by storing an impossible value in the stack guard field, so I think the first thing to do to try to figure this out is to try to recreate the problem with |
I updated the NetBSD builders to NetBSD 7.1. Maybe something was fixed. We'll see. /cc @bsiegert |
Nope. Still fails: I'm going to disable the NetBSD builders (but keep them available via gomote) until builds work. They're eating into resources too much right now. |
$ ./signal.test --test.parallel 1 -test.v goroutine 0 [idle]: goroutine 19 [syscall]: goroutine 1 [chan receive]: goroutine 2 [force gc (idle)]: goroutine 3 [GC sweep wait]: goroutine 5 [syscall]: goroutine 17 [sleep]: goroutine 7 [select, locked to thread]: goroutine 18 [select]: |
I fixed it in the pkgsrc version of go. It was a NetBSD kernel bug. |
@zoulasc, those statements are not consistent. If it was a NetBSD kernel bug, you should've fixed it in NetBSD. If it was a Go bug, then we should fix it in Go. The pkgsrc version of Go should have zero NetBSD-specific patches or workarounds ideally. I assume you mean you worked around the kernel bug in the pkgsrc patches? Where? Got a URL? |
On Apr 20, 7:49pm, notifications@github.com (Brad Fitzpatrick) wrote:
-- Subject: Re: [golang/go] os/signal: "morestack on gsignal" on NetBSD (#196
| @zoulasc, those statements are not consistent.
Not necessarily, read below.
| If it was a NetBSD kernel bug, you should've fixed it in NetBSD.
I will, it is still being discussed how here:
http://mail-index.netbsd.org/tech-kern/2017/04/19/msg021837.html
| If it was a Go bug, then we should fix it in Go.
It was not, but it needs the workaround for compatibility for older
NetBSD versions.
| The pkgsrc version of Go should have zero NetBSD-specific patches or
| workarounds ideally.
Yes, in the steady state, but not transiently.
| I assume you mean you worked around the kernel bug in the pkgsrc
| patches? Where? Got a URL?
http://cvsweb.netbsd.org/bsdweb.cgi/pkgsrc/lang/go/patches/patch-src_runtime_os__netbsd.go?rev=1.1
christos
|
@ianlancetaylor, you might be interested in the patch and mailing list discussion. |
I definitely do not understand the argument in tech-kern that the new thread should inherit the signal stack. It can not possibly work to have two threads using the same signal stack. It seems pointless to force any thread with a signal stack creating a new thread to have to do a little dance to allocate a new signal stack for itself. Not only pointless, but unlike every other Unix system. |
I agree with you @ianlancetaylor -- sharing the signal stack can't possibly work. My proposed patch had the necessary functionality in the kernel so that the c library _lwp_makecontext could automatically DTRT for the signal stack, but I ended up committing a patch in the kernel that will just give the new thread an SS_DISABLE stack_t. That does not prevent future improvements :-) |
CL https://golang.org/cl/47036 mentions this issue. |
Fixed by requiring NetBSD 8+ for Go 1.10. Documentation bug is #22911 |
While debugging NetBSD failures, I got:
Sometimes it passes, though.
(I meant to run a different package, but ended up testing os/signal instead.)
/cc @bcmills @ianlancetaylor
The text was updated successfully, but these errors were encountered: