Skip to content
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

cmd/cgo: panic from cgo due to signal can't be caught #3774

Closed
dvyukov opened this issue Jun 24, 2012 · 11 comments

Comments

@dvyukov
Copy link
Member

commented Jun 24, 2012

package main
//int mydiv(int x, int y) {
//  return x/y;
//}
import "C"
func main() {
    defer func() {
        recover()
    }()
    C.mydiv(0, 0)
}

Output (darwin/amd64):
$ go run test.go
panic: runtime error: integer divide by zero
[signal 0x8 code=0x7 addr=0x210f pc=0x210f]

There is another less visible problem - the signal handler starts executing go code in
Gsyscall status (that is, potentially concurrently with GC).
@rsc

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2012

Comment 1:

Is it really executing Go code in Gsyscall status? The scheduler code is allowed to run
as long as it is not doing things to pointers.

Labels changed: added priority-later, removed priority-triage.

Status changed to Accepted.

@dvyukov

This comment has been minimized.

Copy link
Member Author

commented Sep 12, 2012

Comment 2:

It is. It executes the defer handlers. The problem is that the signal handler does not
check Gsyscall status, and if the goroutine is in Gsyscall it does not try to "acquire
the CPU" to run Go code onto.
@rsc

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2012

Comment 3:

Labels changed: added go1.1.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Nov 9, 2012

Comment 4:

I think we may need to crash the program if cgo code gets a synchronous signal.  I think
the only other plausible alternative would be to throw an exception in the signal
handler and catch it in some new function invoked by asmcgocall, but that would add even
more overhead to every cgo call for a very minimal benefit.  Anything else seems like it
might leave the non-Go code in an inconsistent state.
@dvyukov

This comment has been minimized.

Copy link
Member Author

commented Nov 9, 2012

Comment 5:

Well, the crash is roughly what happens now.
Can't we mimic the second part of cgocall() in the signal handler (entersyscall(),
unlock the goroutine, unregister defer if required) and then handle signal as usual
(execute defers and potentially recover)?
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Nov 9, 2012

Comment 6:

We could do that, but it would mean that the non-Go code would have no chance to do any
cleanups.  In the general case it might be left holding locks, not freeing memory, etc. 
I don't think it's a good idea to simply assume that the non-Go code is OK after a
synchronous signal and to just keep running the program.  I think we could assume that
it is OK if we throw a signal, since that will run any destructors; I don't think we
could assume it is OK if we don't do that.
@rsc

This comment has been minimized.

Copy link
Contributor

commented Nov 12, 2012

Comment 7:

I agree: a sync signal (SIGSEGV etc) to C++ must kill the program.
@dvyukov

This comment has been minimized.

Copy link
Member Author

commented Nov 14, 2012

Comment 8:

It's still required to kill it reliably (not call defers, not run concurrently with GC).
It's required to check that sigprof/sigqueue do not do allocations, and perhaps add
warning comments.
And there is also related issue that crash in cgo produces misleading crash reports:
https://golang.org/issue/3797
@dvyukov

This comment has been minimized.

Copy link
Member Author

commented Nov 14, 2012

Comment 9:

OK, while I am looking at sigqueue :)
https://golang.org/issue/4383
@rsc

This comment has been minimized.

Copy link
Contributor

commented Dec 10, 2012

Comment 10:

Labels changed: added size-l.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Feb 1, 2013

Comment 11:

This issue was closed by revision b0a29f3.

Status changed to Fixed.

@dvyukov dvyukov added fixed labels Feb 1, 2013

@rsc rsc added this to the Go1.1 milestone Apr 14, 2015

@rsc rsc removed the go1.1 label Apr 14, 2015

@golang golang locked and limited conversation to collaborators Jun 24, 2016

This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants
You can’t perform that action at this time.