Skip to content

Fix deadlock when dropping a Context while the runtime lock is held - #724

Merged
richarddd merged 8 commits into
DelSkayn:masterfrom
richarddd:fix/parallel-try-lock-deadlock
Sep 4, 2026
Merged

Fix deadlock when dropping a Context while the runtime lock is held#724
richarddd merged 8 commits into
DelSkayn:masterfrom
richarddd:fix/parallel-try-lock-deadlock

Conversation

@richarddd

Copy link
Copy Markdown
Collaborator

Mut::try_lock calls the blocking lock under the parallel feature, so dropping a Context while the same thread already holds the runtime lock deadlocks.

Track the locks held per thread so drop_context can free directly when it is already on that thread, and leave genuine cross-thread contention blocking as before.

Reachable without any explicit nested drop: a JsClass holding a Context deadlocks when its finalizer runs inside with. Works without the parallel feature, hangs with it.

@richarddd
richarddd force-pushed the fix/parallel-try-lock-deadlock branch 3 times, most recently from e61c85c to d5c5192 Compare August 9, 2026 17:22
@richarddd

Copy link
Copy Markdown
Collaborator Author

@DelSkayn would like your eyes on this one if possible

@richarddd
richarddd requested a review from DelSkayn August 9, 2026 17:23
@richarddd
richarddd force-pushed the fix/parallel-try-lock-deadlock branch from d5c5192 to 4e9f2e7 Compare August 9, 2026 17:30

@Sytten Sytten left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To the best of my ability this is good, but see my comments please

Comment thread core/src/runtime/raw.rs Outdated
Comment on lines +144 to +146
if let Ok(mut queued) = self.0.lock() {
queued.push(CtxPtr(ctx));
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No a fan of hiding a poison lock

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Swapped it for an mpsc channel, so there's no lock to poison.

Comment thread core/src/context/base.rs Outdated
…hared by RawRuntime, unifying the sync and async drop-context parking paths.
@richarddd
richarddd force-pushed the fix/parallel-try-lock-deadlock branch from cef2767 to 41d05f9 Compare September 3, 2026 18:11
@richarddd
richarddd merged commit 3e2ccbc into DelSkayn:master Sep 4, 2026
35 checks passed
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