-
Notifications
You must be signed in to change notification settings - Fork 4
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
Avoid locking all threads on debugger suspension #16
Conversation
This commit stops locking all threads during suspension, which may prevent users from investigating threading problem. But it's still better than freezing the process just by running a ActiveRecord query, or doing anything that calls `Timeout.timeout`. History around this issue: 1. It originally was reported in Shopify/team-ruby-dx#724, and we reported it upstream too. 1. We introduced 1e0f45c as a workaround. 1. Upstream proposed a solution to fix the query evaluation case by restarting threads before evaluating console/DAP input. So the above commit was reverted. 1. However, the process could still hang if `Timeout.timeout` is called: 1. During a suspension 2. And outside of input evaluation An example is typing Arrow-up in the console as `reline` calls `Timeout.timeout` underneath. 1. So we need to reintroduce the workaround until we can find a better with the upstream maintainer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is anything lost by not stopping all threads? Or in other words, why isn't this the optimal solution?
Yes it has some drawbacks:
But since multi-thread support to us is not a significant, it's a tradeoff we can happily take to fix the serious issue. That being said, I also asked if it's possible to make thread stopping configurable. And perhaps when configured as "not stopping threads by default", we can have commands to control individual threads. |
Without this change, threads would still be locked after evalution, which would still cause the problem described in #16.
Otherwise, we'd still face the same problem as in #16, where the debugger would hang because the timeout thread is locked.
This commit stops locking all threads during suspension, which may prevent users from investigating threading problem. But it's still better than freezing the process just by running a ActiveRecord query, or doing anything that calls
Timeout.timeout
.History around this issue:
Timeout.timeout
is called: 1. During a suspension 2. And outside of input evaluation An example is typing Arrow-up in the console asreline
callsTimeout.timeout
underneath.