Skip to content

Fix SMP mutex abandonment lock order during thread termination - #1035

Open
Vikram-Lex wants to merge 1 commit into
gambit:masterfrom
Vikram-Lex:codex/gambit-smp-fix
Open

Fix SMP mutex abandonment lock order during thread termination#1035
Vikram-Lex wants to merge 1 commit into
gambit:masterfrom
Vikram-Lex:codex/gambit-smp-fix

Conversation

@Vikram-Lex

@Vikram-Lex Vikram-Lex commented Sep 8, 2026

Copy link
Copy Markdown

Terminating a thread while another thread calls mutex-unlock! on one of its mutexes can deadlock the SMP runtime. The terminating thread holds its low-level thread lock and waits for the mutex lock, while the unlocking processor holds the mutex lock and waits for the same thread lock. Both public operations are allowed by the manual.

The patch releases the terminating thread's lock before acquiring an owned mutex, then rechecks ownership with both locks held. If another thread has already unlocked or transferred the mutex, cleanup leaves its new state alone. Waiting mutex callers use the existing ordered locking path, with an explicit abandonment flag to preserve their exception behavior.

The continuation's existing #t termination marker prevents the thread from running or acquiring new mutexes during cleanup. The end condition variable remains available until cleanup finishes, so both thread-join! and a terminal thread-state report wait for all owned mutexes to be abandoned. The unicore implementation is unchanged.

Validation on macOS arm64, based on e0236bccd8c2a06c315c2970c21e3dae8673b0bb:

  • Unmodified unicore and SMP with one processor complete the new regression test.
  • Unmodified SMP with two processors hits an external timeout; a process sample shows processors stuck in mutex-unlock! and ##btq-abandon!.
  • The final regression passes 18/18 runs: interpreted and compiled with 1, 2, and 4 processors, three runs per configuration. It also passes on unicore.
  • All 25 existing non-stress thread tests pass on both the original and patched SMP builds with two processors, with identical normal diagnostic output.
  • The wider interpreted unit suite passes 372/374 files after the patch, compared with 371/373 before it. The same two existing serialization tests fail in both versions: 09-io/write_read.scm and 15-misc/serdes.scm. Each file runs with a 15-second external deadline.
  • Tests cover concurrent unlock and ownership transfer; normal return, uncaught exceptions, and self-termination; terminal-state and join completion after cleanup; and blocked callers requesting themselves, another live thread, or no owner, with finite and indefinite waits.

A separate 1,000-iteration exception/abandonment diagnostic encountered a crash on both unmodified and patched SMP builds with a 64 MB minimum heap. That broader runtime issue remains outside this fix; this contribution does not claim to resolve all SMP problems.

Builds use --enable-c-opt=-O1 --enable-c-opt-rts=-O1. SMP also uses --enable-smp --enable-multiple-threaded-vms. Runtime tests use -:p1,m64M, -:p2,m64M, or -:p4,m64M to avoid the separately reported default heap sizing issue.

@Vikram-Lex
Vikram-Lex marked this pull request as draft September 8, 2026 04:42
@Vikram-Lex
Vikram-Lex marked this pull request as ready for review September 8, 2026 04:43
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.

1 participant