tsan: intermittent lock-order-inversion in the full morph_tests run, observed once in six

[Yaraslaut]

Noticed while running the full test suite under clang-tsan as a pre-flight for #567's PR. Filed rather than folded in, per AGENTS.md — it is unrelated to that change and I could not reproduce it.

Verification status

Observed once, not reproduced. One lock-order-inversion warning in 1 of 6 full morph_tests runs under the clang-tsan preset, on branch core-structural-registration-surface (base cab05413), Arch Linux, clang 22.1.8, TSAN_OPTIONS unset (so cmake/tsan.supp was not active — that file suppresses race: entries only, none of which would match a lock-order report anyway).

I did not establish whether it is present on origin/master: reproducing it would need a TSan build of master and, at 1-in-6, several runs of each to say anything. I also did not capture the full report — the run that produced it was being read through grep -A 60 | head -70, and every subsequent run came back clean, so the rest of the stack is gone.

Real output

The whole of what was captured:

==================
WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock) (pid=266676)
  Cycle in lock order graph: M0 (0x55f8a7f684e0) => M1 (0x7ffd74360a48) => M2 (0x7ffd74360cb0) => M0

  Mutex M1 acquired here while holding mutex M0 in thread T2719:
    #0 pthread_mutex_lock <null> (morph_tests+0x1c7b8b) (BuildId: b8e27091b137a43db2ec07f9cd046ef40d76714f)
    #1 __gthread_mutex_lock(pthread_mutex_t*) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/16/../../../../include/c++/16/x86_64-pc-linux-gnu/bits/gthr-default.h:795:21 (morph_tests+0x26cef7) (BuildId: b8e27091b137a43db2ec07f9cd046ef40d76714f)
    #2 std::mutex::lock() /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/16/../../../../include/c++/16/bits/std_mutex.h:116:17 (morph_tests+0x26cef7)

What little the addresses say: M0 (0x55f8…) is in the binary's static/heap range, while M1 and M2 (0x7ffd…) are stack addresses — so two of the three mutexes in the cycle are locals, which points at test fixtures or short-lived objects rather than three long-lived framework locks. No morph frame survives in the captured portion.

Run tallies on that build:

Selection	Runs	Warnings
Full suite	6	1
Full suite, exclude:[registration-surface]	3	0
[registration-surface] alone	5	0

The last two rows are why I do not think it belongs to the PR's new tests, but 3 and 5 runs against a 1-in-6 event is not evidence of anything, and I am not claiming it is.

Why it is worth a record anyway

A lock-order-inversion is TSan's potential deadlock report, not a materialised one — it fires when TSan observes two acquisition orders over the same mutexes, whether or not they ever raced. This repository already treats a sanitizer leg that cannot fail as worse than none (#541), and the sanitizer legs have been able to fail since ecc2f6c. A cycle that shows up one run in six will therefore eventually turn a CI leg red on a PR that did not cause it, and whoever gets that run will start from nothing unless there is a record.

What would change the verdict

• Close it if a deliberate reproduction attempt — morph_tests under clang-tsan with --order rand, 50+ runs on master, with the full report captured to a file rather than grepped — produces no warning. At 1-in-6 that is a strong negative.
• Re-scope it into a real bug as soon as any run captures the full report: the two stack-allocated mutexes should name a test or a framework type directly, at which point this stops being a sighting and becomes a lock-ordering defect with an owner.
• If it reproduces only on the branch above, it is mine and belongs on core: give IBackend a registration surface whose threading contract is structural, not prose #567's PR instead.

Anyone reproducing: capture with ./build/clang-tsan/tests/morph_tests --order rand > log 2>&1 and read the file. Piping into grep/head is how this report was lost.

[Yaraslaut]

Triage verdict: unverified

This is the label's definition almost exactly — "the premise is plausible but untested; blocked on an experiment, not on a decision" — and the issue reaches that conclusion about itself before anyone else does.

I did not attempt the reproduction, and that is a budget statement rather than a judgement. The issue specifies the experiment precisely: a clang-tsan build of current master, morph_tests --order rand, 50+ runs, full report captured to a file. At a 1-in-6 event rate nothing short of that says anything, and it is well beyond what a triage sweep should spend. The verdict is therefore about the state of the evidence, not about whether the finding is real.

What makes it worth keeping open rather than closing

Three things, in order of weight:

1. The failure mode it predicts is now reachable. Since ecc2f6cc (ci: make the sanitizer legs able to fail, and the install able to be consumed #555) the sanitizer legs can actually fail, and PR Four gates that could not fail: UBSan modes, sanitizer instrumentation, spec-sync, and the forms key vocabulary #576 (merged today as e3a70833) newly instrumented five more suites — tests/qt, tests/net_qt_interop, tests/soak, tests/bench and src/qt/forms — and made ladder-sanitizers genuinely run the Qt transport suites. The population of TSan-instrumented code just grew substantially. A one-in-six cycle will eventually redden a CI leg on a PR that did not cause it, and this record is what stops that person starting from zero.
2. The evidence is reported honestly. The run tallies are given with the statement that 3 and 5 runs against a 1-in-6 event prove nothing, and the lost stack is attributed to the grep | head that lost it rather than glossed. AGENTS.md asks for exactly this — "'Observed once in CI, not reproduced in 25 local runs on another platform' is a useful record; silence is not."
3. The address analysis is a real lead, not decoration. M0 in the static/heap range with M1/M2 on the stack points at fixtures or short-lived objects rather than three long-lived framework locks, which is a meaningfully different hypothesis from "morph has a lock-ordering bug" and narrows where to look.

One correction to the framing

The issue says TSAN_OPTIONS was unset so cmake/tsan.supp was not active, then notes the file suppresses race: entries only. That is right, and it means the missing suppressions are irrelevant to this report — a lock-order-inversion would have been reported either way. Worth knowing before someone "explains" the sighting by pointing at the unset variable.

Note also that PR #576 added TSAN_OPTIONS=suppressions=cmake/tsan.supp to the kanban-tsan leg, which was missing it. That changes which legs carry suppressions but, per the above, not whether this report could appear.

The experiment this is blocked on

As the issue states, plus one addition:

./build/clang-tsan/tests/morph_tests --order rand > log 2>&1

Redirect to a file; do not pipe into grep or head — that is how the original report was lost. 50+ runs on current master.

Three outcomes, all useful:

• Clean across 50+ runs on master → close it. At 1-in-6 that is a strong negative, and the re-open condition is a single captured report.
• Full report captured → re-scope to a real lock-ordering defect. The two stack mutexes should name a test or a framework type directly, at which point it has an owner.
• Reproduces only on core-structural-registration-surface → it belongs to core: give IBackend a registration surface whose threading contract is structural, not prose #567's work. Note that PR core: give IBackend a registration surface whose threading contract is structural #577 has since merged to master as 8858450d, so that branch's content is now in master and this third outcome can no longer be distinguished from the first by branch alone. Whoever runs it should compare against cab05413 if they want to separate the two.

Not closing, not promoting to bug-with-a-plan. It is a sighting with a named experiment, which is what unverified is for.

[Yaraslaut]

The experiment this was blocked on: 39 runs on master, clean — with a control proving they could have failed

This issue's own "What would change the verdict" names the experiment, and the triage comment calls it a budget statement rather than a judgement. Here is the budget, spent.

Stated up front: 40 full morph_tests runs under clang-tsan on origin/master, --order rand, complete output per run to its own file. 39 completed.

Setup

• cmake --preset clang-tsan at 6d6a5353 (current master), clang 22.1.8, Arch Linux, 12-core.
• TSAN_OPTIONS=suppressions=<abs>/cmake/tsan.supp:second_deadlock_stack=1 — matching the clang-tsan ctest preset, which has carried second_deadlock_stack=1 since morph#736. (See also cmake/tsan.supp says second_deadlock_stack=1 is not set; the clang-tsan ctest preset has set it since morph#736 #770: cmake/tsan.supp's header still says this option is not set.)
• Instrumentation verified, not assumed: nm -C build/clang-tsan/tests/morph_tests | grep -c __tsan_ → 173, including __tsan_init and __tsan_func_entry.
• OomInjector and morph#108 excluded — the same six the ctest preset excludes, which fail under TSan by design ("OomInjector: unusable under ASan/TSan (operator new/delete overrides are…)"). 1583 → 1577 cases.
• No pipes. Every run's stdout+stderr redirected straight to its own file, exactly as this issue instructs. 41 files on disk.

Result

Selection	Runs	lock-order-inversion	Any TSan warning
morph_tests --order rand, machine otherwise quiet	27	0	0
morph_tests --order rand, 6–12 competing CPU-bound processes	12	0	0
total	39	0	0

grep -rl "ThreadSanitizer" across all 41 captured files returns nothing. Every completed run exited 0 — TSan's default exitcode on a report is 66, so a warning could not have been produced and silently ignored.

Two runs were killed by my own harness timeout and are excluded from the 39 as inconclusive. Both were slow rather than stuck: the tail of each shows the morph#717 strand-race watchdog reporting normal progress (completed 2984/3200 … inFlight 1). Not a hang.

At the reported 1-in-6 rate, P(zero in 39) ≈ 0.08 %.

The control — because 39 clean runs are worth nothing until you know a report could have appeared

AGENTS.md: "Ask whether the check would still pass if the feature did nothing." A silent TSan run and a TSan run that cannot report look identical. So a lock-order inversion of exactly the shape this issue describes — two stack-allocated mutexes, taken in both orders across two threads — was compiled into this same morph_tests binary and run with these same TSAN_OPTIONS:

==================
WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock) (pid=2303090)
  Cycle in lock order graph: M0 (0x7ffe2a019528) => M1 (0x7ffe2a019500) => M0

  Mutex M1 acquired here while holding mutex M0 in main thread:
    #0 pthread_mutex_lock <null> (morph_tests+0x1d450b)
    #1 __gthread_mutex_lock(pthread_mutex_t*) …/gthr-default.h:795:21
    #2 std::mutex::lock() …/bits/std_mutex.h:116:17
    #3 std::scoped_lock<std::mutex>::scoped_lock(std::mutex&) …/mutex:775:19
    #4 CATCH2_INTERNAL_TEST_5() /…/tests/test_example.cpp:66:32
    #5 Catch::RunContext::invokeActiveTestCase() <null>

  Mutex M0 previously acquired by the same thread here:
    …
    #4 CATCH2_INTERNAL_TEST_5() /…/tests/test_example.cpp:65:32

  Mutex M0 acquired here while holding mutex M1 in thread T1:
    …
    #4 CATCH2_INTERNAL_TEST_5()::$_0::operator()() /…/tests/test_example.cpp:70:32

  Mutex M1 previously acquired by the same thread here:
    …
    #4 CATCH2_INTERNAL_TEST_5()::$_0::operator()() /…/tests/test_example.cpp:69:32

SUMMARY: ThreadSanitizer: lock-order-inversion (potential deadlock) … in pthread_mutex_lock
==================
ThreadSanitizer: reported 1 warnings

Exit code 66. Four things this establishes, each of which the clean result depends on:

1. This build does report lock-order-inversion on stack-allocated mutexes — the 0x7ffe… addresses are the same class as this issue's M1/M2.
2. cmake/tsan.supp does not suppress it. The suppressions file was loaded for that run. This issue said so from reading the file, and the triage comment repeated it; it is now measured on this tree rather than argued.
3. second_deadlock_stack=1 supplies the two Mutex Mn previously acquired by the same thread here: stacks — precisely the frames that would have named this issue's two stack-allocated mutexes.
4. morph's own frames resolve (/…/tests/test_example.cpp:66:32). The original report's "No morph frame survives in the captured portion" was the grep -A 60 | head -70 truncation, not a symbolization failure. A real morph-side inversion would have named a morph frame.

The control was reverted and is not in PR #773.

Verdict

Not reproduced on origin/master in 39 runs, in a configuration proven capable of reporting it. That is the strong negative this issue's first bullet asks for — but it names 50+, and 39 is not 50, so this is evidence posted rather than a close.

What would change the verdict now

• Close on 11 further clean runs (to 50), or on anyone's independent clean 50. The re-open condition stays what this issue set: a single captured report.
• Re-scope to a real defect the moment any run captures one. With second_deadlock_stack=1 now on the ctest preset, such a report carries four stacks instead of two, and per point 4 above it will name morph frames. Capture with > log 2>&1 and read the file.
• The third outcome in the triage comment — "reproduces only on core-structural-registration-surface" — is now undecidable by branch, since core: give IBackend a registration surface whose threading contract is structural #577 merged that content as 8858450d. These 39 runs are against master-with-that-content, so if the finding was branch-local it is in scope here and still did not appear.

Sighting unchanged in status by this comment: triage: unverified is still the right label until either 50 clean runs or one captured report.