perf(tui): migrate runtime_threads maps to parking_lot::Mutex (#4149)#4243
Merged
Conversation
…#4149) The RuntimeThreadManager held four synchronous maps behind std::sync::Mutex: task_manager, automations, pending_approvals, and pending_dynamic_tools. All are locked in short, non-async critical sections (attach/register/cancel/ deliver/count), so they are safe parking_lot::Mutex sites. parking_lot guards are !Send and cannot cross .await, and none of these sites do. parking_lot::Mutex does not poison, so lock() returns the guard directly. This drops the Result-handling boilerplate (if-let-Ok / match-Err) at every call site. The only behavior change: after a panic while holding one of these locks, callers now continue with the recovered map instead of silently skipping via the poisoned-Err branch, which is the intended parking_lot tradeoff for these best-effort registries. Left as-is (correctly): the async state/active locks (tokio::sync::Mutex, held across .await) and config (already parking_lot::RwLock). The app-server 'runtime locks' named in the issue are tokio async locks, not parking_lot candidates. Verified: cargo check -p codewhale-tui clean; 178 approval/pending tests pass.
|
Thanks @wuisabel-gif for taking the time to contribute. This repository is observing a maintainer-managed PR intake gate in dry-run mode, so this pull request is staying open. This note helps maintainers prepare the allowlist before any enforcement is considered. Please read |
Closed
83 tasks
This was referenced Jul 8, 2026
Owner
|
thank you so much for contributing!! |
Contributor
Author
you're very welcome~ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4149 (v0.8.68 §6.1 / B1.2 — finish parking_lot migration at hot lock sites).
I claimed this on the issue before starting — see my comment.
What
RuntimeThreadManagerheld four synchronous maps behindstd::sync::Mutex:task_managerautomationspending_approvalspending_dynamic_toolsAll four are locked in short, non-async critical sections (attach / register / cancel / deliver / count / snapshot-clone), so they're safe
parking_lot::Mutexsites.parking_lotguards are!Sendand cannot cross.await; I verified none of these sites hold a guard across an await.Because
parking_lot::Mutexdoesn't poison,lock()returns the guard directly — which removes theif let Ok(..)/match ... Errboilerplate at every call site (net −21 lines).Behavior note
The one intentional behavior change: after a panic while holding one of these locks, callers now continue with the recovered map instead of silently skipping via the poisoned-
Errbranch. That's the standard, intendedparking_lottradeoff, and for these best-effort registries (pending sender maps, manager slots) continuing is the safer default.Deliberately left as-is
state/active—tokio::sync::Mutex, held across.awaiton purpose; not parking_lot candidates.config— alreadyparking_lot::RwLock.runtime locks(also named in the issue) — those aretokio::sync::{Mutex, RwLock}async locks held across.await; migrating them toparking_lotwould be incorrect, so they're out of scope.Acceptance
parking_lotwhere safe.awaitcargo check -p codewhale-tuiclean; 178 approval/pending tests pass, 0 failed