Skip to content

fix: replace mutex unwrap with proper error handling (#333)#339

Merged
Wirasm merged 2 commits into
mainfrom
kild/333-mutex-unwrap-panic
Feb 11, 2026
Merged

fix: replace mutex unwrap with proper error handling (#333)#339
Wirasm merged 2 commits into
mainfrom
kild/333-mutex-unwrap-panic

Conversation

@Wirasm

@Wirasm Wirasm commented Feb 11, 2026

Copy link
Copy Markdown
Owner

Summary

get_process_metrics() at crates/kild-core/src/process/operations.rs:136 used .unwrap() on SYSTEM.lock(), which is the only production .unwrap() on a fallible mutex operation in the codebase. If the mutex is poisoned (another thread panicked while holding the lock), this panics instead of returning an error.

Root Cause

Missing .map_err() conversion from PoisonError to ProcessError::SystemError. The SystemError variant already existed but wasn't used for this case.

Changes

File Change
crates/kild-core/src/process/operations.rs Replace .unwrap() with .map_err() converting to ProcessError::SystemError

The caller (health/handler.rs:82) already handles Result gracefully via Option, so the error is logged and health monitoring degrades gracefully.

Testing

  • cargo fmt --check passes
  • cargo clippy --all -- -D warnings passes
  • cargo test --all passes
  • cargo build --all passes

Validation

cargo fmt --check && cargo clippy --all -- -D warnings && cargo test --all

Issue

Fixes #333

…etrics (#333)

`get_process_metrics()` used `.unwrap()` on `SYSTEM.lock()` which would
panic if the mutex was poisoned. Replace with `.map_err()` to convert
into `ProcessError::SystemError`, which the caller already handles
gracefully via `Option`.

Fixes #333
@Wirasm

Wirasm commented Feb 11, 2026

Copy link
Copy Markdown
Owner Author

Self-Review

Summary

Clean, minimal fix that eliminates the only production .unwrap() on a fallible mutex operation in the codebase.

Findings

Strengths

  • Follows the established mutex error handling pattern from kild-ui/src/terminal/state.rs:332-335
  • Reuses existing ProcessError::SystemError variant — no new types needed
  • Event name core.process.metrics_lock_failed follows the documented {layer}.{domain}.{action}_{state} convention
  • Caller in health/handler.rs:82 already handles Err gracefully, so degradation is seamless

Security

  • No security concerns. Removes a panic vector.

Checklist

  • Fix addresses root cause (.unwrap().map_err())
  • Code follows codebase patterns
  • No obvious bugs introduced
  • All validation passes (fmt, clippy, test, build)

@Wirasm Wirasm merged commit 574ea0f into main Feb 11, 2026
6 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.

bug: mutex unwrap in get_process_metrics can panic in production

1 participant