Skip to content

runtime: cancellation skips Close/Drop cleanup; mutex.lock(block:) leaks the lock on throw #51

Description

@brianp

Found during the plan audit (Aug 2026) — two related correctness holes in the concurrency runtime's cleanup story. Unlike the roadmap items, these can produce wrong behavior in programs that are valid today.

1. Cancellation does not run user cleanup

A green thread cancelled at a safepoint just transitions to Cancelled (codegen/src/green/scheduler.rs:201 area) — live locals' Close/Drop methods never run. The FIR cleanup machinery exists and is correct for normal exits (emit_cleanup_calls_since / cleanup_scope_stack in fir/src/lower/stmt.rs, Close-before-Drop in reverse declaration order, covered by drop_called_on_break/drop_on_break_only_cleans_loop_locals), but the cancellation path bypasses it entirely. The planned aster_log_cleanup_error helper (cleanup errors during unwind should be logged, not thrown) was never written — zero hits in the repo.

Consequence: cancelling a task that holds a file handle, mutex, or channel leaks the resource silently. With supervised/structured concurrency this is the common path, not the rare one.

2. mutex.lock(block:) is not exception-safe

The scoped lock's unlock is emitted straight-line after the inlined block body (fir/src/lower/method.rs:195 area) rather than registered as a cleanup local. So:

  • a throw from inside the block propagates past the unlock — the mutex stays locked forever (waiters suspend permanently; non-green contexts hit the 30s spin fallback in codegen/src/runtime/mutex.rs);
  • cancellation inside the block (item 1) also never releases;
  • the plan's escape analysis (block must not smuggle the value out) and timeout:/LockTimeoutError were never implemented.

The structural guarantee that made straight-line emission look safe — inline lambdas are expression-only, so no return/break — does not cover throw or cancellation.

Suggested fix order

  1. Register the mutex release as a cleanup-local so the existing cleanup_scope_stack machinery covers throw paths — smallest fix, closes the worst leak.
  2. Run the cleanup stack on cancellation (safepoint unwind), adding aster_log_cleanup_error for errors raised during unwind.
  3. Then the deferred hardening: escape analysis, timeout:, release-on-cancel tests.

Grounding

Documented in docs/src/content/docs/reference/status.mdx (Concurrency → known correctness gaps) and docs/src/content/docs/concurrency/primitives.mdx. Related: #49 (GC soundness) for the other runtime-correctness question.

Metadata

Metadata

Assignees

No one assigned

    Labels

    asyncAsync, concurrency, channels, tasksbugSomething isn't workingcodegenJIT, AOT, FIR lowering, runtimehighImportant, address soonsoundnessType system or runtime correctness issue

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions