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

runtime: possible deadlock when asynchronous signal occurs on C thread #16054

Closed
ianlancetaylor opened this issue Jun 13, 2016 · 3 comments
Closed
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@ianlancetaylor
Copy link
Contributor

This is the general case of issue #15994, which focused on SIGPROF.

A somewhat unlikely deadlock is possible when an asynchronous signal occurs on a C thread. Here a C thread means a thread started by calling pthread_create or equivalent. This will lead to a series of calls:

cgoSigtramp
sigtramp
sigtrampgo
sigfwdgo (which will return false if there is no C signal handler)
badsignal
cgocallback
badsignalgo

The call to cgocallback may cause the Go runtime to wait for a thread to be available to run badsignalgo in a goroutine. In some case this may cause the runtime to start a new thread. Starting a new thread in a cgo-using program means calling _cgo_thread_start, which calls malloc.

In other words, we've called malloc in a signal handler, which is not a good idea. If the asynchronous signal occurred while the C thread was itself running malloc, the whole program can deadlock at this point.

We may be able to fix this by simply removing the call to cgocallback. It was introduced in https://golang.org/cl/10757044, but with the knowledge that we are running on the system stack, and some care, it may be possible to avoid it.

@ianlancetaylor ianlancetaylor added this to the Go1.8 milestone Jun 13, 2016
@ianlancetaylor ianlancetaylor self-assigned this Jun 13, 2016
@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 11, 2016
@rsc rsc modified the milestones: Go1.9, Go1.8 Nov 11, 2016
@aclements
Copy link
Member

Ping @ianlancetaylor. Is this still a problem? needm no longer creates threads itself (I think it used to, but I may be mis-remembering). Or is there still a danger that it may wait forever for another thread to create a thread for it if that other thread is blocked on malloc?

@ianlancetaylor
Copy link
Contributor Author

I believe this can still happen. The problem is not that needm creates the thread itself, but that it blocks waiting for a thread to be created, in lockextra. That block may be happening while running a signal handler for a signal that may have been delivered while executing the C malloc function, meaning that we are blocking while malloc is holding locks. Eventually the Go runtime will create a new thread, but in a cgo program that means calling the C malloc function, so thread creation blocks waiting for the malloc lock to be released. At that point we are deadlocked.

@ianlancetaylor
Copy link
Contributor Author

I'm going to reverse myself. I think that this is no longer a problem. We stopped calling cgocallback in https://golang.org/cl/30218. The current code only calls needm. I was wrong to say that needm blocks waiting for a thread to be created; it only blocks waiting for an m to be allocated, and, as you observe, that does not require creating a thread.

@golang golang locked and limited conversation to collaborators Jun 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants