refactor: now_unix_secs/nanos + LockExt::lock_ignore_poison - #356
Merged
Conversation
added 4 commits
June 2, 2026 17:54
…dirge-xhoo] #4 time_util: SystemTime::now().duration_since(UNIX_EPOCH).map(|d| d.as_X()) .unwrap_or(0) was hand-rolled ~16 times (secs + nanos). Hoisted to crate::time_util::now_unix_secs() / now_unix_nanos(); converted the uniform multi-line sites. The variant forms (file-mtime, subsec_nanos as u64, non-.map error handling) are genuinely different and left as-is. #lock: the cryptic .lock().unwrap_or_else(|e| e.into_inner()) poison-recovery idiom appeared 123 times across 43 files. Replaced with a named LockExt::lock_ignore_poison() extension method (crate::sync_util) so the intent ('dirge never relies on poison') is legible at every call site. All sites are std::sync::Mutex; behavior identical. 2456 default / 2550 all-features tests pass (incl. a new poison-recovery test); clean under -D warnings in both default + all-features.
…figs The windows-default config (--no-default-features) gates out the feature blocks (mcp/plugin/lsp/…) that hold many lock_ignore_poison() call sites, leaving the LockExt import unused there. The trait is genuinely conditionally-used per feature set, so annotate the imports. Verified with cargo build --no-default-features --features windows-default.
…HECK global The two tests that mutate the process-global DAP_PERM_CHECK static used per-statement locks, so under parallel execution no_perm_check_when_none could set None between dap_perm_check_roundtrips' write and read-back — failing read_back.is_some() intermittently. Both tests now hold the global's mutex for their whole body (one critical section), serializing them on that lock so they can't interleave. Uses lock_ignore_poison for poison-robustness. Stress-ran 25x clean.
…ner) stragglers The single-line perl in the prior commit missed the multi-line form (.lock() and .unwrap_or_else() on separate lines) — ~32 sites. Converted them to lock_ignore_poison() too (+ LockExt import in the 3 files that had ONLY the multi-line form). The 3 remaining unwrap_or_else(into_inner) sites are RwLock .read()/.write() (notifications.rs) — a different lock type, left as-is (not worth a second ext-trait for 3 sites). All configs build clean.
allen-munsch
pushed a commit
to allen-munsch/dirge
that referenced
this pull request
Jun 3, 2026
…de#356) * refactor: now_unix_secs/nanos + LockExt::lock_ignore_poison helpers [dirge-xhoo] #4 time_util: SystemTime::now().duration_since(UNIX_EPOCH).map(|d| d.as_X()) .unwrap_or(0) was hand-rolled ~16 times (secs + nanos). Hoisted to crate::time_util::now_unix_secs() / now_unix_nanos(); converted the uniform multi-line sites. The variant forms (file-mtime, subsec_nanos as u64, non-.map error handling) are genuinely different and left as-is. #lock: the cryptic .lock().unwrap_or_else(|e| e.into_inner()) poison-recovery idiom appeared 123 times across 43 files. Replaced with a named LockExt::lock_ignore_poison() extension method (crate::sync_util) so the intent ('dirge never relies on poison') is legible at every call site. All sites are std::sync::Mutex; behavior identical. 2456 default / 2550 all-features tests pass (incl. a new poison-recovery test); clean under -D warnings in both default + all-features. * fix: allow(unused_imports) on LockExt imports for minimal feature configs The windows-default config (--no-default-features) gates out the feature blocks (mcp/plugin/lsp/…) that hold many lock_ignore_poison() call sites, leaving the LockExt import unused there. The trait is genuinely conditionally-used per feature set, so annotate the imports. Verified with cargo build --no-default-features --features windows-default. * test(dap): fix flaky dap_perm_check_roundtrips race on the DAP_PERM_CHECK global The two tests that mutate the process-global DAP_PERM_CHECK static used per-statement locks, so under parallel execution no_perm_check_when_none could set None between dap_perm_check_roundtrips' write and read-back — failing read_back.is_some() intermittently. Both tests now hold the global's mutex for their whole body (one critical section), serializing them on that lock so they can't interleave. Uses lock_ignore_poison for poison-robustness. Stress-ran 25x clean. * refactor: convert remaining multi-line .lock().unwrap_or_else(into_inner) stragglers The single-line perl in the prior commit missed the multi-line form (.lock() and .unwrap_or_else() on separate lines) — ~32 sites. Converted them to lock_ignore_poison() too (+ LockExt import in the 3 files that had ONLY the multi-line form). The 3 remaining unwrap_or_else(into_inner) sites are RwLock .read()/.write() (notifications.rs) — a different lock type, left as-is (not worth a second ext-trait for 3 sites). All configs build clean. --------- Co-authored-by: Yogthos <yogthos@gmail.com>
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
1 participant
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.
Round C of the duplication consolidation (epic dirge-9l12, dirge-xhoo) — the volume idioms.
#4
time_util— theSystemTime::now().duration_since(UNIX_EPOCH).map(|d| d.as_*()).unwrap_or(0)idiom was hand-rolled ~16× (secs + nanos). Hoisted tocrate::time_util::now_unix_secs()/now_unix_nanos(); converted the uniform multi-line sites. Variant forms (file-mtime timestamps,subsec_nanos() as u64, non-.maperror handling) are genuinely different and left as-is — onlynow_unix_secs/now_unix_nanoswere actually needed (dropped the unusedmillishelper rather than ship dead code).LockExt::lock_ignore_poison— the cryptic.lock().unwrap_or_else(|e| e.into_inner())poison-recovery idiom appeared 123 times across 43 files. Replaced with a namedLockExt::lock_ignore_poison()extension method (crate::sync_util) so the intent — dirge never relies on lock poisoning for correctness — is legible at every call site instead of buried in a closure. All sites arestd::sync::Mutex; behavior identical.2456 default / 2550 all-features tests pass (incl. a new poison-recovery test that verifies the guard is recovered after a panic-while-locked); clean under
-D warningsin both configs. One Round D PR (text/transcript helpers) remains.