-
Notifications
You must be signed in to change notification settings - Fork 18k
runtime: new scheduler doesn't deadlock if you import net #4973
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
Labels
Milestone
Comments
The minimal reproducer is: package main import _ "runtime/cgo" func main() { select {} } And the culprit is "runtime: allow cgo callbacks on non-Go threads" https://code.google.com/p/go/source/detail?r=1d5a80b07916cd123d1aa4ef2ae04400f29ac6f6. Now that C can callback into Go, consider the following scenario. Go program loads a dynamic library that creates a background thread, Go program blocks solely waiting for callbacks from C. This looks like a valid scenario now, and old scheduler would just kill the program immediately. The new scheduler properly waits for the potential callbacks. So it looks like a correct behavior to me. However, of course, it's a pity that now it's impossible to detect deadlocks when import "runtime/cgo" or "C". What do you think? |
Makes sense. Sadly, it feels like this really degrades the whole deadlock detector experience, since anything that imports net has this issue. In our project it's every single binary because stuff at the lowest level imports a log package that imports net, and we also have some cgo packages that we need for some Linux-specific stuff. I know it's probably even worse, but how about a specific function in the runtime or runtime/cgo package that starts this goroutine? That way only the 1% of people that really need it will have to pay this cost? I don't think net's cgo parts even do callbacks? Or maybe there's a way to start the goroutine the first time C calls back into Go? Probably not. |
It was that way always. net package starts background polling goroutines that effectively disable the deadlock detector. Also if you have any background goroutines/times, the deadlock detector is effectively disabled. Also the most dangerous deadlocks are the ones that do not happen yet (potential deadlocks). So I am not sure how much it's worth fixing in this form. It would be awesome to implement a deadlock detector for partial and/or potential deadlocks, but there is not theory exist for that. |
This issue was closed by revision 34c67eb. Status changed to Fixed. |
This issue was closed.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
The text was updated successfully, but these errors were encountered: