kernel32: fix per-thread FLS and Windows-faithful CRITICAL_SECTION (fixes intermittent MSVC c2.dll deadlock)

[jeffmcjunkin]

Fixes an intermittent (~1–2% of invocations) permanent deadlock when running the VS2013 MSVC compiler (cl.exe, multithreaded codegen in c2.dll) under wibo, traced to two thread-primitive divergences from Windows. With both fixes the hang rate went from ~1–2% to 0 in 5,100 stress runs, compiler output stays byte-identical to native Windows, and heavily contended workloads got ~6× faster as a side effect.

Bug 1: FLS values were process-global (dll/kernel32/fibersapi.cpp)

FlsAlloc/FlsGetValue/FlsSetValue kept one g_flsValues[] array shared by every thread. Fiber-local storage without fibers is thread-local on Windows (each thread is exactly one fiber).

This breaks msvcr120.dll thread creation: the VS2013 CRT stores its per-thread data block (_ptd) — which carries the _beginthreadex entry point and argument — via FlsSetValue, and the new thread's _threadstartex/_callthreadstartex re-read it via FlsGetValue. With one shared cell, two concurrently-starting threads overwrite each other's _ptd and can both start with the same argument.

Caught in an API trace of a live hang — c2.dll's codegen pool creates 4 workers back-to-back, each with its own auto-reset "go" event (7c/88/94/a0):

t251103 WaitForSingleObject h=88   <- worker
t251102 WaitForSingleObject h=88   <- a SECOND thread waits on the SAME go event
boss    SetEvent 7c, 88, 94, a0    <- 94 never gets a waiter, ever
boss    WaitForMultipleObjects({done events}, bWaitAll, INFINITE)
        -> hangs forever: the orphaned worker never runs, its done-event is never set

Fix: values move to a thread_local array (wibo maps guest threads 1:1 onto host threads); the index allocation map stays process-wide, guarded by a mutex. FLS callbacks are still not invoked (unchanged behavior; noted in the commit message).

Bug 2: CRITICAL_SECTION not faithful to the Windows state machine (dll/kernel32/synchapi.cpp)

Two defects:

1. Contended handoff was claim-by-race: waitForCriticalSection waited until OwningThread == 0 and then claimed with a plain store. Two waiters can both observe 0 after one Leave and both enter the section. The fingerprint — a free section left with LockCount == 0 instead of −1, created when the second "owner"'s Leave fails the ownership check and returns without decrementing — was captured with gdb in hung c2.dll processes.
2. Leave validated the caller; Windows doesn't. Real LeaveCriticalSection performs no ownership check — it unconditionally decrements RecursionCount/LockCount and releases exactly one waiter via a counted semaphore. wibo's silent bail-out strands the lock state for guest usage Windows tolerates. Instrumentation caught leave-not-owner cs=<c2 work-queue CS> tid=B owner=A lock=1 rec=1 in live runs, after which the work queue's drain condition became unsatisfiable → deadlock (~0.1–0.3% of compiles even with FLS fixed).

Fix mirrors the Windows protocol: LockSemaphore is modeled as a ticket count — each contended Leave posts exactly one ticket (InterlockedIncrement + WakeByAddressSingle), each blocked Enter consumes exactly one (CAS decrement, WaitOnAddress otherwise) and then owns by construction; Leave validates nothing. TryEnterCriticalSection can't steal while waiters exist because each waiter's LockCount increment persists until it owns and leaves.

Verification

• Stress harness: VS2013 cl.exe compiling an 18 KB real C unit, 25 s timeout = hang.
  • Before: 20–22 hangs / 1000 at 24-way parallelism (also reproduces ~2% fully sequential, so it's intra-process).
  • After (both fixes): 0 hangs in 5,100 runs (2×1000 + 2000 @ P=24, 300 @ P=1, plus 800 runs on two other source units).
• Output correctness: per-section COFF comparison of wibo-compiled objects vs native-Windows-compiled objects for three test units — .text$mn, .xdata, .pdata, .rdata, .drectve and all relocations byte-identical (only .debug$S differs, which embeds absolute source paths).
• Throughput: 1000 parallel compiles dropped from 26 s to 4 s wall; 300 sequential from 131 s to 7 s — the old wait loop woke every waiter to race on each release.

Both changes are host-side only and were validated on x86 (i386 build) under WSL2/Ubuntu 24.04, 24 logical CPUs.

🤖 Generated with Claude Code

[jeffmcjunkin]

This was 1h25m with Fable, btw. I was pretty impressed.