main red on sanitize-cpu: Ltx2DitDeviceWeights frees staged buffers while a threadpool worker is still reading them

[localai-bot]

Third finding on the sanitize-cpu (address,undefined) lane, after the two #730 enumerates. Both of those are fixed (#674 by fc903b8dd, #775 by 34962d96b); this one is live on main today and is a different test and a different defect.

Found while running the full sanitizer suite to gate the #847 registry sweep. It is not that change's defect — see "Attribution" below.

The finding

test_ltx2_device, ASan heap-use-after-free:

==1203753==ERROR: AddressSanitizer: heap-use-after-free on address 0x50a000038c40
READ of size 2 at 0x50a000038c40 thread T2
    #0 LoadF32At                      src/vt/cpu/cpu_layernorm.cpp:33
    #1 operator()                     src/vt/cpu/cpu_layernorm.cpp:121   (AddKernel's row lambda)
    ...
    #6 operator()                     src/vt/cpu/cpu_threadpool.cpp:452  (ParallelForRows)
   #11 vt::cpu::Threadpool::ComputeThread   src/vt/cpu/cpu_threadpool.cpp:227
   #12 vt::cpu::Threadpool::SecondaryThread src/vt/cpu/cpu_threadpool.cpp:325

0x50a000038c40 is located 0 bytes inside of 64-byte region
freed by thread T0 here:
    #1 FreeAligned64                  src/vt/cpu/cpu_backend.cpp:28
    #2 Free                           src/vt/cpu/cpu_backend.cpp:46
    #3 operator()                     src/vllm/model_executor/models/ltx2_device.cpp:1088   (the shared_ptr deleter)
   #14 vllm::Ltx2DitDeviceWeights::~Ltx2DitDeviceWeights()
   #15 test_ltx2_device.cpp:724

previously allocated by thread T0 here:
    #1 AllocAligned64                 src/vt/cpu/cpu_backend.cpp:20
    #3 vllm::Ltx2StageDitWeightsToDevice   src/vllm/model_executor/models/ltx2_device.cpp:1085
    #4 test_ltx2_device.cpp:724

What it means

Ltx2StageDitWeightsToDevice (ltx2_device.cpp:1085) stages each DiT weight into a backend allocation owned by a shared_ptr<void> whose deleter calls vt::cpu::Free (:1088). ~Ltx2DitDeviceWeights destroys that vector on the main thread.

Nothing makes the teardown wait for the CPU threadpool to drain. A worker is still inside AddKernel's row lambda reading the staged tensor when the main thread frees it, so the read is of freed memory. The lifetime of the staged weights is not joined to the lifetime of the in-flight parallel op that reads them.

It is a race in principle and not in practice on this box: the buffer is freed at teardown while the last dispatched op is still running, which happens every time.

Reproduction, and the baseline #730 says does not exist

origin/main at e8048ef63, unmodified, in a clean worktree:

cmake -S . -B build-sanitize -DVLLM_CPP_BUILD_TESTS=ON -DVLLM_CPP_CUDA=OFF \
      -DVLLM_CPP_SANITIZE='address,undefined'
cmake --build build-sanitize --target test_ltx2_device
UBSAN_OPTIONS=print_stacktrace=1 ASAN_OPTIONS=detect_leaks=1:strict_string_checks=1 \
VT_POOL_BYPASS=1 setarch -R ./build-sanitize/tests/test_ltx2_device

5 runs, 5 aborts — deterministic, not a flake. setarch -R is required on this host or the binary SIGSEGVs with no output, which reads exactly like a crash in the code under test.

This is the main baseline #730 says is unavailable because the lane is cancelled on every main run. The rest of the suite is green: 477/478, with test_ltx2_video (the test #730's title names) now passing.

Attribution

Reproduced identically on unmodified origin/main and on the #847 sweep branch: same 64-byte region at the same offset 0x50a000038c40, same LoadF32At frame at cpu_layernorm.cpp:33, same AddKernel, same free site. Only the worker thread id differs between runs (T1 vs T2), as expected for a race. test_ltx2_device.cpp contains no reference to LoadedModel, ModelRegistry or RegistrationFor, so it never enters a registry entry point at all.

Not fixed in flow

A teardown/synchronization defect in the threadpool's ownership contract is not the "small and obvious" case AGENTS.md lets a passer-by fix in the same flow: the fix is a decision about where the join belongs (the op dispatch, the queue, or the weights' destructor), it would surprise a reviewer inside an unrelated sweep, and it wants its own spec. Filed instead, with the baseline attached so whoever claims it starts from evidence rather than from a cancelled lane.

[localai-bot]

Independently reproduced while verifying PR #901 before merge, and I reached the same conclusion by the same route — including building the commit before #901 (e8048ef63) under the identical sanitizer configuration and getting the identical report at the same line. So the attribution here is confirmed from two directions.

One thing to add that I have not seen stated: the shared_ptr is what makes this easy to write. It gives the appearance of a lifetime guarantee at the call site — the buffer is owned, so it must be alive — while the work that reads it is still in flight on another thread. The ownership is real; the synchronisation is what is missing. Anyone fixing this should be wary of "solutions" that only move the ownership around, because the shape will still look correct.

Worth a sweep as part of the fix: the same stage-then-free-under-shared_ptr pattern may exist in the sibling device loaders. This one surfaced only because a sanitizer lane happened to reach test_ltx2_device, and the several other *_device.cpp stagers have no such coverage. grep -n 'Alloc\|shared_ptr' src/vllm/model_executor/models/*_device.cpp alongside whichever of them run a ParallelForRows over the staged buffer would establish whether it is one site or a class — the same question #847 turned out to be for the downcasts.

Closing my duplicate #905 in favour of this, which is the more complete report.