You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
After fixing the heap-use-after-free in #127 (BackendRig's teardown racing QtExecutor::post()), the Kanban / ThreadSanitizer CI job on PR #121
still fails — now with 165 ThreadSanitizer warnings of a different
character: not a lifecycle use-after-free in application/testkit code, but
apparent races centered on Qt's own internal QCallableObject/ invokeMethod machinery and on kanban::GetBoardResult's move
constructor, both reached exclusively through morph::qt::QtExecutor::post()
and morph::async::detail::CompletionState<T>::setValue/setException.
I could not build a deterministic standalone repro for this one (see
"What I tried" below) — this issue documents the evidence gathered and the
concrete leads for whoever picks it up, rather than a proven root cause.
The fix for #127 is confirmed correct and sufficient for the bug it targets
(verified via a 200-concurrent-chain repro, 10/10 clean runs — see #127).
This is a separate, second finding, uncovered only because #127's fix
removed the crash that was previously happening first and masking it.
A load-bearing discovery: the CI job's own premise is wrong
.github/workflows/ci.yml's kanban-tsan job carries this comment:
# test_kanban_stress.cpp's [tsan]-tagged TEST_CASE runs entirely on
# Mode::Local's ThreadPoolExecutor{4} with no Qt/GUI involvement (see the
# test file's own header comment), so the "a GUI stack under TSan is
# mostly noise" rationale that keeps the ladder out of linux-sanitizers
# does not apply to this one test.
This is the only exception this codebase makes to an otherwise
consistent, explicit policy: linux-sanitizers' own comment states
"QML and the fuzzers stay out of this matrix entirely — they are covered by
the linux-all-features job, and a GUI stack under TSan is mostly noise,"
and the clang-tsan preset is never combined with MORPH_BUILD_QT=ON
anywhere else in the workflow.
The claim "no Qt/GUI involvement" is false.examples/common/testkit/ backend_rig.hpp's Mode::Local unconditionally constructs a real morph::qt::QtExecutor:
case Mode::Local: {
_workerPool = std::make_unique<::morph::exec::ThreadPoolExecutor>(4);
_qtExecutor = std::make_unique<::morph::qt::QtExecutor>(); // <-- real Qt object
_clientExecutor = _qtExecutor.get();
...
}
— and every one of the 165 warnings' stack traces bottoms out in genuine Qt
internals: QMetaObject::invokeMethod, QCallableObject::QCallableObject, QObject::event, QtPrivate::FunctorCall::call. This test does exercise a
real (if headless) Qt event-loop stack, contradicting the premise the job's
own exception to the "GUI under TSan is noise" policy relies on.
If they turn out to be TSan false positives from Qt's own internal
synchronization being invisible to an uninstrumented, prebuilt Qt package
(a well-known category — Qt is not built with -fsanitize=thread in this
CI, so TSan cannot see whatever internal locking Qt itself relies on),
then the kanban-tsan job's entire premise for existing as an exception to
this repo's "Qt/GUI under TSan is noise" policy is invalid, and the job
should either be redesigned to avoid Qt entirely (e.g. run the same
concurrent-move logic against BoardModel directly, bypassing BackendRig's QtExecutor) or be given a documented, scoped suppressions
file.
Evidence gathered
Tally of the 165 warnings' SUMMARY lines by distinct signature:
84 data race in kanban::GetBoardResult::GetBoardResult(GetBoardResult&&)
(examples/kanban/include/kanban/dto/board_dto.hpp:119:8)
18 data race in QtPrivate::FunctorCall<...>::call(...)::lambda::operator()()
(Qt/6.8.1/gcc_64/include/QtCore/qobjectdefs_impl.h:141:24)
12 data race in CompletionState<shared_ptr<void>>::setValue(...)::lambda::operator()()
(include/morph/core/completion.hpp:68:41)
12 data race in CompletionState<shared_ptr<void>>::setValue(...)::lambda::operator()()
(include/morph/core/completion.hpp:60:62)
12 data race in __tsan_memcpy (inside std::function<void()>::function(std::function<void()>&&),
called from QCallableObject::QCallableObject, called from QtExecutor::post)
12 data race in operator delete(void*, unsigned long)
6 data race ON VPTR (ctor/dtor vs virtual call) in CompletionState<shared_ptr<void>>::
setValue(...)::lambda::operator()() (completion.hpp:68:25) -- a shared_ptr<void>
destructor call racing
6 data race in completion.hpp:68:25 (non-vptr variant of the above)
3 data race in CompletionState<shared_ptr<void>>::setValue(...)::lambda::~()
(completion.hpp:49:28)
The most diagnostic single report (full stacks in the CI log, gh api repos/LASTRADA-Software/morph/actions/jobs/95966930869/logs):
WARNING: ThreadSanitizer: data race (pid=8475)
Read of size 8 at 0x720c00007850 by main thread:
#0 std::_Function_base::_M_empty() const bits/std_function.h:247
#1 std::function<void ()>::operator()() const
#2 QtPrivate::FunctorCall<...>::call(...)::lambda::operator()() const
qobjectdefs_impl.h:141:24
...
#7 QObject::event(QEvent*)
#8 morph::ladder::testkit::awaitQt<kanban::GetBoardResult>(...)
examples/common/testkit/pump.hpp:110:26
#9 test_kanban_stress.cpp:157
Previous write of size 8 at 0x720c00007850 by thread T2 (a ThreadPoolExecutor worker):
#0 __tsan_memset
#1 std::function<void ()>::function(std::function<void ()>&&)
#2 QtPrivate::QCallableObject<...>::QCallableObject(std::function<void ()>&&)
qobjectdefs_impl.h:586:78
#3-5 QMetaObject::invokeMethod(...)
#6 morph::qt::QtExecutor::post(std::function<void ()>) qt_executor.hpp:40:9
#7 morph::async::detail::CompletionState<std::shared_ptr<void>>::setValue(...)
completion.hpp:76:21
#8 morph::backend::LocalBackend::execute(...)::lambda backend.hpp:845:28
...
#13 morph::exec::detail::StrandExecutor::scheduleNext(...) strand.hpp:150:17
...
#18 morph::exec::ThreadPoolExecutor::loop() executor.hpp:106:17
Location is heap block of size 48 at 0x720c00007830 allocated by thread T2:
#0 operator new(unsigned long)
#1 QMetaObject::invokeMethodCallableHelper<...>(...) qobjectdefs.h:622:25
... (same QtExecutor::post -> CompletionState::setValue -> LocalBackend::execute
-> StrandExecutor::scheduleNext -> ThreadPoolExecutor::loop chain as above)
Both the read site (main thread, delivering a queued Qt event via QObject::event) and the write site (thread T2, constructing a brand-new QCallableObject via its own independent post() call) trace through the
identical call chain (QtExecutor::post <- CompletionState::setValue <- LocalBackend::execute <- StrandExecutor::scheduleNext <- ThreadPoolExecutor::loop) and land on the same heap address. Since invokeMethodCallableHelper does new QCallableObject(...) fresh on every
call (qobjectdefs.h:645-646), two unrelated post() calls should never
share an address while both are live — landing on the same address is the
signature of the firstQCallableObject having already been freed
(Qt deletes it internally after dispatching) and the second allocation
reusing that freed block, while the first one's delivery machinery is still
finishing execution. That is a use-after-free presenting through allocator
reuse, not two live objects racing — the same family of bug as #127, just
manifesting differently, and again reached only through QtExecutor::post()/CompletionState::setValue/setException.
What I tried (repro attempts that did NOT reproduce this)
Bridge::publishResult's exact fan-out shape (bridge.hpp:1020-1045):
N subscriber threads each receiving an independent std::any copy of the
same underlying result, posted concurrently through one shared QtExecutor, at kanban's own scale (4 subscribers × 4 clients × 50
actions = 800 deliveries): 5/5 clean runs, no crash. This was my
leading hypothesis (publishResult's [sink, value]-by-value capture
fanning one result out to every subscriber on a shared model instance),
but it did not reproduce, at least not as a plain SIGSEGV without TSan
instrumentation available to me locally (Windows, no working -fsanitize=thread toolchain).
Neither repro used ThreadSanitizer (not available in my local environment);
both only rule out a plain-crash reproduction, not a genuine (if
non-fatal-without-TSan) race. A repro built and run under TSan itself —
which I could not do locally — is very likely necessary to make further
progress here, since the signature (heap block reuse racing an in-flight
delivery) may only be observable with TSan's own instrumentation of the
allocator.
Suggested next steps
Confirm or rule out the Qt-instrumentation gap first. Check whether
the Qt6 package this CI job installs (jurplel/install-qt-action) is
built with -fsanitize=thread itself. If not — and prebuilt Qt
distributions essentially never are — TSan cannot see Qt's own internal
locking (event queue mutexes, QObject internals), and every warning
whose stack bottoms out inside QtCore proper (as opposed to purely
morph/libstdc++ code) is a candidate false positive by construction, not
just by suspicion.
Fix or retire the kanban-tsan job's incorrect premise regardless of
(1). Its own comment claims no Qt involvement; that's demonstrably
false. Either:
Redesign the test to exercise BoardModel's concurrency directly
through LocalBackend/ThreadPoolExecutor alone, with a hand-rolled
completion mechanism that doesn't route through QtExecutor/Qt's event
loop at all (matching what the comment already believed was true), or
Accept that this job legitimately needs Qt in the sanitizer matrix (an
explicit policy change, reversing the "GUI under TSan is noise" stance
for this one case) and follow through on what that implies: either a
TSan-instrumented Qt build, or a documented, scoped suppressions file
for known-safe Qt-internal patterns TSan can't see through.
If neither of the above resolves it, treat the remaining warnings
(particularly the 84 GetBoardResult move-constructor races and the 12 operator delete races, which don't obviously reduce to "Qt's own
internals") as still-open and pursue a TSan-instrumented repro directly —
ideally on the same runner image/Qt version CI uses, since the local
attempts in this issue could not reproduce it without that instrumentation.
Summary
After fixing the heap-use-after-free in #127 (
BackendRig's teardown racingQtExecutor::post()), theKanban / ThreadSanitizerCI job on PR #121still fails — now with 165 ThreadSanitizer warnings of a different
character: not a lifecycle use-after-free in application/testkit code, but
apparent races centered on Qt's own internal
QCallableObject/invokeMethodmachinery and onkanban::GetBoardResult's moveconstructor, both reached exclusively through
morph::qt::QtExecutor::post()and
morph::async::detail::CompletionState<T>::setValue/setException.I could not build a deterministic standalone repro for this one (see
"What I tried" below) — this issue documents the evidence gathered and the
concrete leads for whoever picks it up, rather than a proven root cause.
Why this blocks work on #127 / PR #121
The fix for #127 is confirmed correct and sufficient for the bug it targets
(verified via a 200-concurrent-chain repro, 10/10 clean runs — see #127).
This is a separate, second finding, uncovered only because #127's fix
removed the crash that was previously happening first and masking it.
A load-bearing discovery: the CI job's own premise is wrong
.github/workflows/ci.yml'skanban-tsanjob carries this comment:This is the only exception this codebase makes to an otherwise
consistent, explicit policy:
linux-sanitizers' own comment states"QML and the fuzzers stay out of this matrix entirely — they are covered by
the linux-all-features job, and a GUI stack under TSan is mostly noise,"
and the
clang-tsanpreset is never combined withMORPH_BUILD_QT=ONanywhere else in the workflow.
The claim "no Qt/GUI involvement" is false.
examples/common/testkit/ backend_rig.hpp'sMode::Localunconditionally constructs a realmorph::qt::QtExecutor:— and every one of the 165 warnings' stack traces bottoms out in genuine Qt
internals:
QMetaObject::invokeMethod,QCallableObject::QCallableObject,QObject::event,QtPrivate::FunctorCall::call. This test does exercise areal (if headless) Qt event-loop stack, contradicting the premise the job's
own exception to the "GUI under TSan is noise" policy relies on.
This matters for triage either way:
bugs, not kanban-specific ones (same conclusion as QtExecutor::post() heap-use-after-free when a nested Completion chain's post outlives the executor's teardown #127).
synchronization being invisible to an uninstrumented, prebuilt Qt package
(a well-known category — Qt is not built with
-fsanitize=threadin thisCI, so TSan cannot see whatever internal locking Qt itself relies on),
then the
kanban-tsanjob's entire premise for existing as an exception tothis repo's "Qt/GUI under TSan is noise" policy is invalid, and the job
should either be redesigned to avoid Qt entirely (e.g. run the same
concurrent-move logic against
BoardModeldirectly, bypassingBackendRig'sQtExecutor) or be given a documented, scoped suppressionsfile.
Evidence gathered
Tally of the 165 warnings'
SUMMARYlines by distinct signature:The most diagnostic single report (full stacks in the CI log,
gh api repos/LASTRADA-Software/morph/actions/jobs/95966930869/logs):Both the read site (main thread, delivering a queued Qt event via
QObject::event) and the write site (thread T2, constructing a brand-newQCallableObjectvia its own independentpost()call) trace through theidentical call chain (
QtExecutor::post<-CompletionState::setValue<-LocalBackend::execute<-StrandExecutor::scheduleNext<-ThreadPoolExecutor::loop) and land on the same heap address. SinceinvokeMethodCallableHelperdoesnew QCallableObject(...)fresh on everycall (qobjectdefs.h:645-646), two unrelated
post()calls should nevershare an address while both are live — landing on the same address is the
signature of the first
QCallableObjecthaving already been freed(Qt deletes it internally after dispatching) and the second allocation
reusing that freed block, while the first one's delivery machinery is still
finishing execution. That is a use-after-free presenting through allocator
reuse, not two live objects racing — the same family of bug as #127, just
manifesting differently, and again reached only through
QtExecutor::post()/CompletionState::setValue/setException.What I tried (repro attempts that did NOT reproduce this)
that triggered QtExecutor::post() heap-use-after-free when a nested Completion chain's post outlives the executor's teardown #127, extended from 1 chain to 200, matching kanban's
real 4-clients-×-50-actions load), posted from 4 real
ThreadPoolExecutorworker threads, torn down with QtExecutor::post() heap-use-after-free when a nested Completion chain's post outlives the executor's teardown #127's fix (drain 5slices before freeing
QtExecutor): 10/10 clean runs, no crash. Thisrules out "the QtExecutor::post() heap-use-after-free when a nested Completion chain's post outlives the executor's teardown #127 fix's fixed drain size is simply insufficient at real
scale" as the explanation — the same fix that resolved QtExecutor::post() heap-use-after-free when a nested Completion chain's post outlives the executor's teardown #127 continues to
hold under equivalent concurrent load in isolation.
Bridge::publishResult's exact fan-out shape (bridge.hpp:1020-1045):N subscriber threads each receiving an independent
std::anycopy of thesame underlying result, posted concurrently through one shared
QtExecutor, at kanban's own scale (4 subscribers × 4 clients × 50actions = 800 deliveries): 5/5 clean runs, no crash. This was my
leading hypothesis (
publishResult's[sink, value]-by-value capturefanning one result out to every subscriber on a shared model instance),
but it did not reproduce, at least not as a plain SIGSEGV without TSan
instrumentation available to me locally (Windows, no working
-fsanitize=threadtoolchain).Neither repro used ThreadSanitizer (not available in my local environment);
both only rule out a plain-crash reproduction, not a genuine (if
non-fatal-without-TSan) race. A repro built and run under TSan itself —
which I could not do locally — is very likely necessary to make further
progress here, since the signature (heap block reuse racing an in-flight
delivery) may only be observable with TSan's own instrumentation of the
allocator.
Suggested next steps
the Qt6 package this CI job installs (
jurplel/install-qt-action) isbuilt with
-fsanitize=threaditself. If not — and prebuilt Qtdistributions essentially never are — TSan cannot see Qt's own internal
locking (event queue mutexes,
QObjectinternals), and every warningwhose stack bottoms out inside
QtCoreproper (as opposed to purelymorph/libstdc++ code) is a candidate false positive by construction, not
just by suspicion.
kanban-tsanjob's incorrect premise regardless of(1). Its own comment claims no Qt involvement; that's demonstrably
false. Either:
BoardModel's concurrency directlythrough
LocalBackend/ThreadPoolExecutoralone, with a hand-rolledcompletion mechanism that doesn't route through
QtExecutor/Qt's eventloop at all (matching what the comment already believed was true), or
explicit policy change, reversing the "GUI under TSan is noise" stance
for this one case) and follow through on what that implies: either a
TSan-instrumented Qt build, or a documented, scoped suppressions file
for known-safe Qt-internal patterns TSan can't see through.
(particularly the 84
GetBoardResultmove-constructor races and the 12operator deleteraces, which don't obviously reduce to "Qt's owninternals") as still-open and pursue a TSan-instrumented repro directly —
ideally on the same runner image/Qt version CI uses, since the local
attempts in this issue could not reproduce it without that instrumentation.
Related
surfaced once that one was fixed.
.github/workflows/ci.yml'skanban-tsanjob and itslinux-sanitizersjob (the latter's own comment: "a GUI stack under TSan is mostly noise").
Environment
ubuntu-latestGitHub Actions runner, Clang (perCLANG_VERSIONin
ci.yml), Qt 6.8.1, ThreadSanitizer. PR ladder: rung 4 -- kanban #121, run following commit917ea54(the QtExecutor::post() heap-use-after-free when a nested Completion chain's post outlives the executor's teardown #127 fix), job95966930869.