Skip to content

Runner stop deadlock#1037

Closed
jabolina wants to merge 1 commit into
belaban:masterfrom
jabolina:fix/runner-stop-deadlock
Closed

Runner stop deadlock#1037
jabolina wants to merge 1 commit into
belaban:masterfrom
jabolina:fix/runner-stop-deadlock

Conversation

@jabolina

@jabolina jabolina commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator
  • Runner.stop and run methods compete for the same intrinsic lock when stopping.
  1. t1: The stop() method is in a synchronized block and updates the state to stopping;
  2. t1: While holding the lock, the stop method calls Thread.join(timeout) in the stopping thread.
  3. t2: The run() methods sees state != running and proceed to stop;
  4. t2: The run() method tries to acquire the same intrinsic lock held by t1, currently in Thread.join.
  5. t1: Timeout in stop() elapses and the method returns; the lock is released.
  6. t2: The run() method acquires the intrinsic lock and updates state = stopped.

* Runner.stop and run methods compete for the same intrinsic lock when
  stopping.
@belaban

belaban commented Jul 8, 2026

Copy link
Copy Markdown
Owner

@belaban

belaban commented Jul 8, 2026

Copy link
Copy Markdown
Owner

Hi @jabolina
I don't see this as a deadlock: it's just that we always wait for join_timeout ms (default: 100ms). By moving this outside the lock scope, you eliminate this wait. Correct, or am I missing something?

@jabolina

jabolina commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

Correct, they eventually proceed after join_timeout elapses.

@belaban

belaban commented Jul 8, 2026

Copy link
Copy Markdown
Owner

Why the word 'deadlock' then?

@belaban

belaban commented Jul 8, 2026

Copy link
Copy Markdown
Owner

OK, I see that the advantage of this change is for the runner to go from stopping -> stopped more quickly. Applied manually, with unit test RunnerTest.testStop2()

@belaban belaban closed this Jul 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants