Courier spray: count offer spans instead of flagging one

Cross-model review (agy) found a false negative in the overlap detector added
in the previous commit: with a boolean flag, a nested offer's `defer` clears it
while the OUTER offer is still between its scan and its commit, so every later
overlap with that outer offer goes unreported.

A depth count fixes it — report when the count is already above zero, then
increment; decrement on the way out.

Pinned by a test that provokes two overlaps against one outer offer. Reverting
the counter to flag semantics reports one of them, which is the mutation that
proves the test guards the behaviour rather than the implementation.

The same review confirmed no deadlock (the store queue is never held across
beginSprayOfferSpan, endSprayOfferSpan or the accepting callback), that bit 12
is wire-compatible with main's bits 8-10, and that notifyUI's non-async closure
does make `await` a compile error at the call site.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
ecgang 2026-07-26 11:45:09 -07:00
parent 77b31f2188
commit 85ef044af6
2 changed files with 39 additions and 7 deletions

View File

@ -152,7 +152,7 @@ final class CourierStore {
private let queue = DispatchQueue(label: "chat.bitchat.courier.store")
#if DEBUG
/// True while a spray offer is between its scan and its last commit.
/// How many spray offers are between their scan and their last commit.
///
/// Both spray paths spend from one budget and both release the store queue
/// across `accepting` (they must: it enters BLE/collections queues). The
@ -161,7 +161,11 @@ final class CourierStore {
/// isolation, because the violation that would actually ship is a
/// suspension *inside* an already-MainActor block, which every isolation
/// check passes.
private var sprayOfferInFlight = false
///
/// A count rather than a flag: with a flag, a nested offer's `defer` clears
/// it while the outer offer is still in flight, so any later overlap with
/// that outer offer goes unreported.
private var sprayOffersInFlight = 0
/// Test seam for the overlap detector, mirroring `_test_onOutboundPacket`
/// in `BLEService`. Unset in normal debug runs, where an overlap trips
/// `assertionFailure` instead.
@ -173,10 +177,8 @@ final class CourierStore {
private func beginSprayOfferSpan(_ function: StaticString = #function) {
#if DEBUG
queue.sync {
guard sprayOfferInFlight else {
sprayOfferInFlight = true
return
}
defer { sprayOffersInFlight += 1 }
guard sprayOffersInFlight > 0 else { return }
if let hook = Self._test_onSprayOfferOverlap {
hook()
} else {
@ -193,7 +195,7 @@ final class CourierStore {
private func endSprayOfferSpan() {
#if DEBUG
queue.sync { sprayOfferInFlight = false }
queue.sync { sprayOffersInFlight = max(0, sprayOffersInFlight - 1) }
#endif
}
private let fileURL: URL?

View File

@ -792,6 +792,36 @@ struct CourierStoreTests {
#expect(overlaps.count == 1)
}
/// A nested offer must not end the OUTER offer's span when it returns.
///
/// This is why the detector counts rather than sets a flag: with a flag,
/// the inner offer's `defer` clears it while the outer one is still between
/// its scan and its commit, so every later overlap with that outer offer
/// goes unreported. Two overlaps are provoked here; a flag reports one.
@Test func nestedOfferDoesNotEndTheOuterSpan() {
let store = makeStore()
let recipientKey = Data(repeating: 0xB0, count: 32)
let envelope = makeEnvelope(recipientKey: recipientKey).withCopies(8)
#expect(store.deposit(envelope, from: depositorA))
let courierA = Data(repeating: 0xC1, count: 32)
let courierB = Data(repeating: 0xC2, count: 32)
let courierC = Data(repeating: 0xC3, count: 32)
let overlaps = OverlapCounter()
CourierStore._test_onSprayOfferOverlap = { overlaps.count += 1 }
defer { CourierStore._test_onSprayOfferOverlap = nil }
_ = store.offerSprayCopies(to: courierA) { _ in
// B overlaps A and finishes; A is still in flight afterwards.
_ = store.offerSprayCopies(to: courierB) { _ in true }
// C therefore also overlaps A, and must be reported too.
_ = store.offerSprayCopies(to: courierC) { _ in true }
return true
}
#expect(overlaps.count == 2)
}
/// Box so the detector's escaping closure can tally without capturing a
/// local `var`.
private final class OverlapCounter: @unchecked Sendable {