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

ReviewabilityReviewsAgreementSize
#36556 xyzconstant depends: fix non-atomic reads in capnp debug assertionsReady1Strong174
  • Stop false-positive data races in Cap'n Proto debug assertions during ThreadSanitizer runs
  • Prevent CI test failures in unoptimized multiprocess builds
  • Ready to review
  • Strong support with no open objections (Sjors)
  • Verified patches match upstream and confirmed ThreadSanitizer requires both (Sjors)
  • Resolved scope question confirming extra test backports are not needed (Sjors)
  • 174 lines added or modified outside tests
  • 0 lines added or modified in tests
  • 1 lines removed in total
  • 3 files, 1 commits
Build and CI#20 of 56
P3 · bug fix
  • P3 because it resolves false-positive ThreadSanitizer failures in Cap'n Proto debug assertions
  • Unblocks multiprocess CI testing across threads without altering production code
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

Applies two upstream backport patches to Cap'n Proto in depends to fix non-atomic reads in debug assertions. It replaces raw memory reads of mutex futex states and atomic reference counters with relaxed atomic loads.

Problem: Unoptimized builds, including ThreadSanitizer (TSan) CI jobs, enable Cap'n Proto debug assertions that inspect mutex futex words and atomic reference counts non-atomically. When objects communicate across threads, TSan flags these debug reads as data races, failing CI runs.

Evidence of importance:

  • PR description: '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.'
  • Links to a concrete CI failure in bitcoin-core/libmultiprocess#376 triggered by switching EventLoop::sync() to kj::Executor::executeSync().
  • Patches correspond directly to upstream Cap'n Proto backport commits b91a6ba7c7 and cc13c8c45f (capnproto/capnproto#2855).

Scope: Adds two patch files directly to depends/patches/capnp/ and updates depends/packages/capnp.mk to apply them during extraction.

Risk of a mistake: low (build). A flaw in the backported capnp patches could cause build failures or concurrency issues in experimental multiprocess builds.

Changed since the previous assessment: Diff provided matches the previous description; no changes to the approach or commit contents.

Reviewability

Ready

Nothing known that would invalidate a review now.

Waiting on (from the thread read):

  • nothing: change has support and no pending actions

Agreement

Strong

substantive support, no open objection (Sjors)

Thread notes: Added Sjors' ACK and question regarding whether additional backports were needed for tests, which the author confirmed was unnecessary.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
Sjors (member)question, scopewhether additional changes should be backported to make capnp's tests pass under TSananswered · author agreed that additional test-only backports are unnecessaryansweredyes2026-10-21: 'given that we set `BUILD_TESTING=OFF`, I guess it's unnecessary to backport things just to make tests pass?'
Status from: 2026-10-21: xyzconstant: 'I'd lean toward keeping `BUILD_TESTING=OFF` as you said, so we don't unnecessarily backport things just to make them work.'

Support:

  • Sjors (member) (ACK, 2026-10-21) [both]: verified that patches match upstream and TSan needs both

Participants:

  • fanquake (member, neutral, 1 statements 2026-10-19 to 2026-10-19): pinged reviewers
  • Sjors (member, support, 1 statements 2026-10-21 to 2026-10-21): ACKed after verifying upstream patches and TSan requirements

Dependencies

Enables:

Files

0 lines under test/bench/ci.

  • depends/patches/capnp/kj_atomicaddref_atomic_read.patch +104/-0
  • depends/patches/capnp/kj_mutex_atomic_futex_reads.patch +65/-0
  • depends/packages/capnp.mk +5/-1

Card

The short text the ranking pass compares across PRs.

Adds upstream Cap'n Proto patches to depends to fix non-atomic reads in mutex and reference count debug assertions. When capnp is compiled without optimization, such as in ThreadSanitizer CI runs, these non-atomic reads are reported as data races once objects communicate across threads. This fixes a TSan failure blocking bitcoin-core/libmultiprocess#376.

Data

dossier JSON · extract JSON · model ledger, generated 2026-10-22T01:16, confidence high, input hash 8f02537f27c6162d
thread read through 2026-10-21 (3 statements), head 6e8e85ddcf
labels: Build system

Categories

Build and CI

P3 · bug fix

Fixes false-positive data races in ThreadSanitizer debug assertion builds of Cap'n Proto. PR description notes 'The TSan job builds capnp without optimization, which automatically enables these assertions, and TSan reports these reads as data races', resolving CI failures and unblocking libmultiprocess#376.

Membership: Matches the 'Build system' label and directly modifies depends package recipes and patches under depends/packages/capnp.mk and depends/patches/capnp/.

Factors: security/stability 1, bug 1, performance 0, user value 0, leverage 1