depends: fix non-atomic reads in capnp debug assertions #36556

pull xyzconstant wants to merge 1 commits into bitcoin:master from xyzconstant:depends-capnp-kj-atomic-reads changing 3 files +174 −1

xyzconstant commented at 12:22 AM on October 17, 2026: contributor

KJ's debug assertions read the mutex futex word and AtomicRefcounted refcounts non-atomically. The TSan job builds capnp without optimization, which automatically enables these assertions, and TSan reports these reads as data races the moment KJ objects start communicating across threads.

libmultiprocess enables cross-thread communication for the first time in bitcoin-core/libmultiprocess#376, which replaces the pipe used in EventLoop::sync() with kj::Executor::executeSync(). This is what triggered the failure.

Patches are the upstream commits capnproto/capnproto@b91a6ba7c7 and capnproto/capnproto@cc13c8c45f, which are backports of the v2 fixes in capnproto/capnproto#2781 and capnproto/capnproto#2843 (backport PR: capnproto/capnproto#2855).

depends: fix non-atomic reads in capnp debug assertions

KJ's debug assertions read the mutex futex word and `AtomicRefcounted`
refcounts non-atomically. The TSan job builds capnp without optimization,
which automatically enables these assertions, and TSan reports these reads
as data races the moment KJ objects start communicating across threads.

libmultiprocess enables cross-thread communication for the first time
in https://github.com/bitcoin-core/libmultiprocess/pull/376, which replaces
the pipe used in `EventLoop::sync()` with `kj::Executor::executeSync()`.

Patches are the upstream commits
https://github.com/capnproto/capnproto/commit/b91a6ba7c7 and
https://github.com/capnproto/capnproto/commit/cc13c8c45f, which are backports
of the v2 fixes in capnproto/capnproto#2781 and capnproto/capnproto#2843
(https://github.com/capnproto/capnproto/pull/2855).

6e8e85ddcf

DrahtBot added the label Build system on Oct 17, 2026
DrahtBot commented at 12:22 AM on October 17, 2026: contributor


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



External sites
- Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36556
- Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36556.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36556#author-ralph


Reviews

See the guideline and AI policy for information on the review process.

Type Reviewers

ACK Sjors

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.
fanquake commented at 7:41 AM on October 19, 2026: member

cc @Sjors @ryanofsky
Sjors commented at 8:11 AM on October 21, 2026: member

ACK 6e8e85ddcf2eb4a062fab3cd2b9c046edfbf72ec

I verified that the patches match upstream, and that TSan needs both (if we continue with https://github.com/bitcoin-core/libmultiprocess/pull/376).

If we want capnp's tests to pass under TSan, we'd have to backport several other changes. But given that we set BUILD_TESTING=OFF, I guess it's unnecessary to backport things just to make tests pass? (I did manage locally)
xyzconstant commented at 4:50 PM on October 21, 2026: contributor

Thanks @Sjors! I'm not sure which exact tests would fail and need backports, but I'd lean toward keeping BUILD_TESTING=OFF as you said, so we don't unnecessarily backport things just to make them work.

Type	Reviewers
ACK	Sjors

Contributors

Labels

depends: fix non-atomic reads in capnp debug assertions #36556

External sites

Reviews