-
Notifications
You must be signed in to change notification settings - Fork 224
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
hotfix(exec) 0.9.25.7: remove possible RemoteExec deadlock cause #1776
Conversation
val waitUntilMs = System.currentTimeMillis() + unit.toMillis(timeout) | ||
// looping in case thread is woken up early | ||
while (!_shutdown && System.currentTimeMillis() < waitUntilMs) { | ||
val waitMs = waitUntilMs - System.currentTimeMillis() |
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.
This can possibly become negative between your checks and calculation and will end up with IllegalArgumentException
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.
Good catch -- I assumed wrongly that negative values were ignored. Updated ✅
@@ -52,7 +52,7 @@ trait RemoteExec extends LeafExecPlan with StrictLogging { | |||
val waitUntilMs = System.currentTimeMillis() + unit.toMillis(timeout) | |||
// looping in case thread is woken up early | |||
while (!_shutdown && System.currentTimeMillis() < waitUntilMs) { | |||
val waitMs = waitUntilMs - System.currentTimeMillis() | |||
val waitMs = Math.max(0, waitUntilMs - System.currentTimeMillis()) |
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.
If its 0, you want till you are notified, is that the expected behavior?
Just to clarify, if the value is positive, it will come out of waiting state, go check the while condition and go back to waiting state, but with 0 it will never come out of wait until notified.
Closed -- we can revisit this solution later if needed. |
Pull Request checklist