Skip to content

fix: Deadlock under load#36

Open
timfish wants to merge 1 commit intomainfrom
timfish/fix/deadlock
Open

fix: Deadlock under load#36
timfish wants to merge 1 commit intomainfrom
timfish/fix/deadlock

Conversation

@timfish
Copy link
Copy Markdown
Collaborator

@timfish timfish commented Apr 14, 2026

This PR resolves the deadlock caused by CaptureStackTraces holding a global mutex while waiting on futures, which prevented ThreadPoll from acquiring the same mutex to update thread data.

The fix introduces a two-mutex architecture: threads_map_mutex protects the map structure during iteration/insertion/deletion, while each ThreadInfo has its own mutex protecting only its mutable data (poll_state, last_seen). Additionally, async_store is now wrapped in shared_ptr to prevent use-after-free when async tasks outlive the thread's map entry, and CaptureStackTrace now includes a 5-second timeout to handle cases where isolates never process the interrupt request. The futures vector is moved outside the lock scope, ensuring the map lock is released before calling fut.get().

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 14, 2026

Semver Impact of This PR

🟢 Patch (bug fixes)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).


Bug Fixes 🐛

  • Deadlock under load by timfish in #36

🤖 This preview updates automatically when you update the PR.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

@timfish timfish force-pushed the timfish/fix/deadlock branch from 9de613f to 6fcbd72 Compare April 14, 2026 10:50
@timfish timfish requested a review from JPeer264 April 14, 2026 11:42
Copy link
Copy Markdown
Member

@JPeer264 JPeer264 left a comment

Choose a reason for hiding this comment

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

I have absolutely no clue about C++, but I trust you on this one

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.

Deadlock in CaptureStackTrace

2 participants