From a937a0b07e38a4180ba4e606b3f78f45883f72bc Mon Sep 17 00:00:00 2001 From: Meta Vens Date: Thu, 17 Sep 2026 02:09:50 +0200 Subject: [PATCH] Let the bridge's own timeout, refusal and two-phase poll be seen Three rules the bridge has always promised, none of which anything committed could see. The timeout was the structural one. Both scenarios end their waits with a close, so the duration the bridge waits on governed nothing either of them observes: a hard-coded 2000 ms survived the whole Linux matrix and Windows, and a host asking for five seconds would have got two forever with nothing reporting it. The fix is not another entry at another constant. A timeout's only observable is how long the call takes, so `polltimeout` lets an idle runtime's poll be ended by its own timeout and measures it, twice at different values, because no single constant sits in both bands. The seam's hold moves behind that too: it parks only a call the wait woke, so a poll whose own timeout expired is on its way out rather than held there by the fixture and counted as the bridge's doing. The refusal during a close (the header's section 4) had no scenario at all. A call admitted after the closer has begun increments the in-flight count behind a wait that has already read it. `latecall` holds the close open with the seam and makes a call into it. And the two-phase poll was written off as needing a listener, a credential and a connection. It needs a closed loopback port: the failure always carries a reason, which is an 87-byte payload, in about a millisecond. Moving the pop above the copy crashes on both targets. 18 killed, 0 survived, 0 skipped on linux-x64; 16 killed, 0 survived on win32-x64, where the two ThreadSanitizer entries are honestly skipped. Both runs restored the source to the same sha256, b163cab7d48ebc93. Co-Authored-By: Claude Opus 5 --- .../cultmesh_quic_native.cpp | 20 +- .../tests/cultmesh_quic_native_tests.cpp | 330 +++++++++++++++++- scripts/mutate-cultmesh.mjs | 200 +++++++++-- 3 files changed, 517 insertions(+), 33 deletions(-) diff --git a/native/GameCult.Mesh.Quic.Native/cultmesh_quic_native.cpp b/native/GameCult.Mesh.Quic.Native/cultmesh_quic_native.cpp index 621be18c..bfd09afc 100644 --- a/native/GameCult.Mesh.Quic.Native/cultmesh_quic_native.cpp +++ b/native/GameCult.Mesh.Quic.Native/cultmesh_quic_native.cpp @@ -83,9 +83,11 @@ // - HELD parks a call inside the library until the scenario releases it, so the // close races calls that are genuinely there for as long as the scenario // wants them there, instead of for as long as the schedule happens to give. -// It returns true when it parked, and touches nothing afterwards: a bridge -// that frees the runtime out from under a held call is then reported by the -// scenario rather than crashing inside it. +// It returns true when it parked, and a held call makes no further call of +// its own; what it does still do is leave its CallScope, which takes the gate, +// decrements the count and notifies. That is the ordinary exit every export +// makes, and it is the bridge's wait — not the hold — that keeps the runtime +// alive underneath it. #if defined(CULTMESH_QUIC_DEBUG_ASSERTS) #define CULTMESH_QUIC_DEBUG_PEAK(count) DebugRaisePeak(count) #define CULTMESH_QUIC_DEBUG_AT_CLOSE(count) DebugRecordCallsAtClose(count) @@ -1172,12 +1174,16 @@ CULTMESH_API int32_t cultmesh_quic_next_event( std::unique_lock lock(runtime->gate); if (runtime->events.empty() && timeout_ms > 0) { - runtime->signal.wait_for(lock, std::chrono::milliseconds(timeout_ms), + // The host's timeout bounds the wait; an event or the close ends it + // sooner, and `woken` is which of the two happened. + const bool woken = runtime->signal.wait_for(lock, std::chrono::milliseconds(timeout_ms), [runtime] { return !runtime->events.empty() || runtime->closing; }); // Development builds only; a no-op unless a scenario has armed the hold. - // A held call returns the same 0 an empty queue would have, and touches - // the runtime no further. - if (CULTMESH_QUIC_DEBUG_HELD(lock)) return 0; + // A held call returns the same 0 an empty queue would have, and makes no + // further call of its own. Only a woken call is held: one whose own + // timeout expired is on its way out, and parking it would let a scenario + // report a call the bridge kept when what kept it was the fixture. + if (woken && CULTMESH_QUIC_DEBUG_HELD(lock)) return 0; } if (runtime->events.empty()) return 0; diff --git a/native/GameCult.Mesh.Quic.Native/tests/cultmesh_quic_native_tests.cpp b/native/GameCult.Mesh.Quic.Native/tests/cultmesh_quic_native_tests.cpp index 2b2fa3a3..afb529f7 100644 --- a/native/GameCult.Mesh.Quic.Native/tests/cultmesh_quic_native_tests.cpp +++ b/native/GameCult.Mesh.Quic.Native/tests/cultmesh_quic_native_tests.cpp @@ -19,6 +19,8 @@ // // cultmesh_quic_native_tests closerace [iterations] [pollers] // cultmesh_quic_native_tests holdclose [iterations] [pollers] +// cultmesh_quic_native_tests polltimeout [iterations] +// cultmesh_quic_native_tests latecall [iterations] // // `closerace` hunts the race: many pollers, many iterations, and whatever the // schedule gives. `holdclose` takes the guessing out of it — the bridge's @@ -27,6 +29,12 @@ // bridge actually counted, so the scenario asserts the quiesce instead of // assuming its own fixture worked. It needs CULTMESH_QUIC_DEBUG_ASSERTS. // +// In both of those the wait is ended by the close, so the host's timeout governs +// nothing either of them can see, and a bridge waiting on a constant of its own +// passes both. `polltimeout` is the one that lets the timeout govern the return +// and measures it. `latecall` covers the other half of section 4: a call that +// races the start of a close is refused rather than counted behind its wait. +// // Exit code 0 means every assertion held. Anything else, including a sanitizer // abort or the bridge's own assertion, is a failure. Under ThreadSanitizer run // it with `TSAN_OPTIONS=halt_on_error=1`, or a reported race leaves the exit @@ -34,6 +42,7 @@ #include +#include #include #include #include @@ -139,6 +148,222 @@ std::string CloseRaceOnce(int pollers) { return {}; } +// The two timeouts `polltimeout` asks for. Two, and not one, because a single +// value cannot tell a bridge that honours the host's timeout from one that waits +// on that same number of its own: a constant has to satisfy both bands, and no +// constant is in both. +constexpr int32_t kShortPollMs = 200; +constexpr int32_t kLongPollMs = 900; + +// How much sooner than the timeout a poll may return. A wait may be late; it may +// not be early, because returning early is exactly what waiting on a shorter +// duration looks like. This is scheduler granularity, not slack. +constexpr int kEarlyToleranceMs = 20; + +// How much later than the timeout a poll may return. Wide enough for a loaded +// machine and a container, and narrower than the gap between the two timeouts, +// which is what closes the constant off: 200 admits [180, 500] and 900 admits +// [880, 1200], and nothing is in both. +constexpr int kLateToleranceMs = 300; + +struct TimedPoll { + int32_t result; + long long elapsed_ms; +}; + +TimedPoll PollFor(void* runtime, int32_t timeout_ms) { + std::vector payload(static_cast(kPayloadCapacity)); + cultmesh_quic_event event{}; + int32_t required = 0; + const auto started = std::chrono::steady_clock::now(); + const int32_t result = cultmesh_quic_next_event( + runtime, timeout_ms, &event, payload.data(), kPayloadCapacity, &required); + const auto elapsed = std::chrono::duration_cast( + std::chrono::steady_clock::now() - started).count(); + return {result, elapsed}; +} + +// The host's timeout, and the only thing it is: how long a poll with nothing to +// deliver stays inside the library. Nothing else here checks it — `closerace` +// and `holdclose` both end their waits with a close, so in both of them the +// bridge could wait on any duration at all and every assertion would still hold. +// A runtime with no listener and no connection has nothing to deliver, so the +// timeout is what ends this poll, and the elapsed time is what it waited on. +std::string PollTimeoutOnce() { + void* runtime = nullptr; + const int32_t opened = cultmesh_quic_runtime_open("cultmesh-quic-native-tests", &runtime); + if (opened != 0 || runtime == nullptr) + return "cultmesh_quic_runtime_open returned " + std::to_string(opened); + + std::string failure; + for (const int32_t timeout_ms : {kShortPollMs, kLongPollMs}) { + const TimedPoll poll = PollFor(runtime, timeout_ms); + const std::string asked = "a poll asking for " + std::to_string(timeout_ms) + " ms "; + if (poll.result != 0) { + failure = asked + "returned " + std::to_string(poll.result) + + ", not the 0 an idle runtime owes it"; + break; + } + if (poll.elapsed_ms < timeout_ms - kEarlyToleranceMs) { + failure = asked + "returned after " + std::to_string(poll.elapsed_ms) + + " ms: the wait ended on some shorter duration than the one it was given"; + break; + } + if (poll.elapsed_ms > timeout_ms + kLateToleranceMs) { + failure = asked + "returned after " + std::to_string(poll.elapsed_ms) + + " ms: the wait ended on some longer duration than the one it was given"; + break; + } + } + + cultmesh_quic_runtime_close(runtime); + return failure; +} + +int PollTimeout(int iterations) { + for (int iteration = 0; iteration < iterations; ++iteration) { + const std::string failure = PollTimeoutOnce(); + if (!failure.empty()) + return Fail("polltimeout iteration " + std::to_string(iteration) + ": " + failure); + } + std::printf("polltimeout %dx: ok\n", iterations); + return 0; +} + +// Nothing listens on the discard port, so a connection to it fails and the +// bridge says why. That reason is the payload this scenario needs, and reaching +// it takes no listener, no credential and no established connection. +constexpr uint16_t kClosedPort = 9; + +// How long the failure is given to arrive. The loopback refusal is immediate; +// if the refusal is swallowed instead, MsQuic's own handshake timeout ends the +// connection at 10 seconds and event 4 still arrives, so this only bounds a +// bridge that reports nothing at all. +constexpr int kEventDeadlineMs = 20000; + +// The bridge writes its own reason sentences and every one of them starts here. +// Checking the bytes, and not only the length, is what separates a payload +// copied out of the queued event from one copied out of an event that was popped +// first: the second reads freed memory, which does not spell this. +constexpr char kReasonPrefix[] = "CultMesh QUIC connection"; + +// The two-phase poll, which is the shape every host uses to size its buffer: +// ask with nothing, be told what it needs, ask again. What makes it work is that +// the refusal consumes nothing — the event is still queued, payload and all, +// after a return of 2 — and that the copy happens before the event is popped. +// Both are section 6 of the header, and nothing committed reached either. +std::string PayloadFitOnce() { + void* runtime = nullptr; + const int32_t opened = cultmesh_quic_runtime_open("cultmesh-quic-native-tests", &runtime); + if (opened != 0 || runtime == nullptr) + return "cultmesh_quic_runtime_open returned " + std::to_string(opened); + + std::string failure; + uint64_t connection_id = 0; + const int32_t connecting = + cultmesh_quic_connection_open(runtime, "127.0.0.1", kClosedPort, &connection_id); + if (connecting != 0) { + cultmesh_quic_runtime_close(runtime); + return "cultmesh_quic_connection_open returned " + std::to_string(connecting); + } + + // An out parameter the bridge must not write on a refusal, filled with + // something the bridge never writes so that a write is visible. + cultmesh_quic_event poisoned{}; + std::memset(&poisoned, 0xab, sizeof(poisoned)); + cultmesh_quic_event event = poisoned; + int32_t required = 0; + int32_t result = 0; + + // Ask with no room at all until the event carrying the failure reason + // arrives. An event with no payload fits in nothing, so it is delivered and + // polled past rather than mistaken for this one. + const auto deadline = std::chrono::steady_clock::now() + std::chrono::milliseconds(kEventDeadlineMs); + for (;;) { + event = poisoned; + required = 0; + result = cultmesh_quic_next_event(runtime, 250, &event, nullptr, 0, &required); + if (result == 2 && required > 0) break; + if (result < 0) { + failure = "cultmesh_quic_next_event returned " + std::to_string(result); + break; + } + if (std::chrono::steady_clock::now() >= deadline) { + failure = "no event carrying a payload arrived within " + + std::to_string(kEventDeadlineMs) + " ms of a connection to a closed port"; + break; + } + } + + if (failure.empty() && std::memcmp(&event, &poisoned, sizeof(event)) != 0) + failure = "the refusal wrote to *out_event, which the header says it leaves alone"; + + // Again, with the same empty buffer: a refusal that consumed the event + // leaves nothing to find the second time. + if (failure.empty()) { + cultmesh_quic_event again = poisoned; + int32_t required_again = 0; + const int32_t second = cultmesh_quic_next_event(runtime, 0, &again, nullptr, 0, &required_again); + if (second != 2 || required_again != required) + failure = "the second ask returned " + std::to_string(second) + " needing " + + std::to_string(required_again) + ", not the 2 needing " + std::to_string(required) + + " the first one did: the refusal consumed the event"; + } + + // And with room, which is where the copy happens. The tail beyond the + // payload is written too, so a copy that ran past what it was told to write + // is visible. + if (failure.empty()) { + constexpr uint8_t kTail = 0x5a; + std::vector payload(static_cast(required) + 16, kTail); + cultmesh_quic_event delivered = poisoned; + int32_t required_delivered = 0; + const int32_t third = cultmesh_quic_next_event( + runtime, 0, &delivered, payload.data(), static_cast(payload.size()), + &required_delivered); + const std::string reason(reinterpret_cast(payload.data()), + static_cast(required)); + if (third != 1 || required_delivered != required) + failure = "the ask with room returned " + std::to_string(third) + " needing " + + std::to_string(required_delivered) + ", not the 1 needing " + + std::to_string(required) + " the refusal promised"; + else if (delivered.type != CULTMESH_QUIC_EVENT_CONNECTION_SHUTDOWN || + delivered.connection_id != connection_id || + delivered.payload_length != required) + failure = "the delivered event was type " + std::to_string(delivered.type) + + " on connection " + std::to_string(delivered.connection_id) + " carrying " + + std::to_string(delivered.payload_length) + " byte(s), not the failed connection's own"; + else if (reason.rfind(kReasonPrefix, 0) != 0) + failure = "the payload delivered was '" + reason + "', not a reason this bridge writes"; + else if (payload[static_cast(required)] != kTail) + failure = "the copy wrote past the " + std::to_string(required) + " bytes it reported"; + } + + // Consumed now, and only now. + if (failure.empty()) { + cultmesh_quic_event emptied = poisoned; + int32_t required_emptied = 0; + const int32_t fourth = + cultmesh_quic_next_event(runtime, 0, &emptied, nullptr, 0, &required_emptied); + if (fourth != 0) + failure = "the event was still queued after being delivered: a further ask returned " + + std::to_string(fourth); + } + + cultmesh_quic_runtime_close(runtime); + return failure; +} + +int PayloadFit(int iterations) { + for (int iteration = 0; iteration < iterations; ++iteration) { + const std::string failure = PayloadFitOnce(); + if (!failure.empty()) + return Fail("payloadfit iteration " + std::to_string(iteration) + ": " + failure); + } + std::printf("payloadfit %dx: ok\n", iterations); + return 0; +} + #if defined(CULTMESH_QUIC_DEBUG_ASSERTS) // How long a poll that does not block is given to prove it by returning, after @@ -241,6 +466,98 @@ std::string HoldCloseOnce(int pollers) { return {}; } +// One iteration of the other half of section 4: a call made while a close is +// running is refused, and is not counted behind the close's wait. +// +// The prohibition is the host's — no call may begin after the close starts — and +// the bridge does not make it optional. What it does do is refuse the call that +// races the start, and that refusal is load-bearing rather than courteous: a +// call admitted after the closer has begun increments the in-flight count behind +// a wait that has already read it, and the wait can then be left waiting on a +// count that reaches zero only when the late caller happens to leave. +// +// The seam is what makes the window wide enough to aim at: a held poller keeps +// the close inside its wait for as long as this scenario wants it there. +std::string LateCallOnce() { + cultmesh_quic_debug_hold_calls(1); + void* runtime = nullptr; + const int32_t opened = cultmesh_quic_runtime_open("cultmesh-quic-native-tests", &runtime); + if (opened != 0 || runtime == nullptr) { + cultmesh_quic_debug_hold_calls(0); + return "cultmesh_quic_runtime_open returned " + std::to_string(opened); + } + + std::thread poller([runtime] { + std::vector payload(static_cast(kPayloadCapacity)); + cultmesh_quic_event event{}; + int32_t required = 0; + cultmesh_quic_next_event( + runtime, kPollTimeoutMs, &event, payload.data(), kPayloadCapacity, &required); + }); + + const auto counted_deadline = + std::chrono::steady_clock::now() + std::chrono::milliseconds(kCountedDeadlineMs); + while (cultmesh_quic_debug_peak_calls() < 1 && + std::chrono::steady_clock::now() < counted_deadline) + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + std::this_thread::sleep_for(std::chrono::milliseconds(kSettleMs)); + + std::thread closer([runtime] { cultmesh_quic_runtime_close(runtime); }); + // The close has begun its wait once it has read the count, and the held + // poller is what keeps it there. + const auto started_deadline = + std::chrono::steady_clock::now() + std::chrono::milliseconds(kCountedDeadlineMs); + while (cultmesh_quic_debug_calls_at_close() < 0 && + std::chrono::steady_clock::now() < started_deadline) + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + const int32_t at_close = cultmesh_quic_debug_calls_at_close(); + + // Two calls, from a thread of their own so that a bridge which admits them + // cannot park this one: one that does not block, and then the blocking + // crossing. That order, because a bridge that admits the blocking one parks + // it inside the library until the hold is released, and a call made after + // that would be racing the teardown rather than the wait. + std::atomic late_poll{0}; + std::atomic late_error{0}; + std::thread late([runtime, &late_poll, &late_error] { + std::array message{}; + late_error.store(cultmesh_quic_last_error(runtime, message.data(), + static_cast(message.size()))); + std::vector payload(static_cast(kPayloadCapacity)); + cultmesh_quic_event event{}; + int32_t required = 0; + late_poll.store(cultmesh_quic_next_event( + runtime, kPollTimeoutMs, &event, payload.data(), kPayloadCapacity, &required)); + }); + + std::this_thread::sleep_for(std::chrono::milliseconds(kSettleMs)); + cultmesh_quic_debug_hold_calls(0); + late.join(); + closer.join(); + poller.join(); + + if (at_close < 1) + return "the close did not begin its wait with the poller counted inside, so nothing held it " + "open for a call to race"; + if (late_poll.load() != -1) + return "cultmesh_quic_next_event returned " + std::to_string(late_poll.load()) + + " to a call made during the close, not the -1 that refuses it"; + if (late_error.load() != -1) + return "cultmesh_quic_last_error returned " + std::to_string(late_error.load()) + + " to a call made during the close, not the -1 that refuses it"; + return {}; +} + +int LateCall(int iterations) { + for (int iteration = 0; iteration < iterations; ++iteration) { + const std::string failure = LateCallOnce(); + if (!failure.empty()) + return Fail("latecall iteration " + std::to_string(iteration) + ": " + failure); + } + std::printf("latecall %dx: ok\n", iterations); + return 0; +} + int HoldClose(int iterations, int pollers) { for (int iteration = 0; iteration < iterations; ++iteration) { const std::string failure = HoldCloseOnce(pollers); @@ -267,7 +584,9 @@ int CloseRace(int iterations, int pollers) { int main(int argc, char** argv) { if (argc < 2) { - std::fprintf(stderr, "usage: %s closerace|holdclose [iterations] [pollers]\n", argv[0]); + std::fprintf(stderr, + "usage: %s closerace|holdclose|polltimeout|payloadfit|latecall [iterations] [pollers]\n", + argv[0]); return 2; } const std::string scenario = argv[1]; @@ -278,13 +597,16 @@ int main(int argc, char** argv) { return 2; } if (scenario == "closerace") return CloseRace(iterations, pollers); - if (scenario == "holdclose") { + if (scenario == "polltimeout") return PollTimeout(iterations); + if (scenario == "payloadfit") return PayloadFit(iterations); + if (scenario == "holdclose" || scenario == "latecall") { #if defined(CULTMESH_QUIC_DEBUG_ASSERTS) + if (scenario == "latecall") return LateCall(iterations); return HoldClose(iterations, pollers); #else std::fprintf(stderr, - "holdclose needs the development seam: configure with " - "-DCULTMESH_QUIC_DEBUG_ASSERTS=ON\n"); + "%s needs the development seam: configure with " + "-DCULTMESH_QUIC_DEBUG_ASSERTS=ON\n", scenario.c_str()); return 2; #endif } diff --git a/scripts/mutate-cultmesh.mjs b/scripts/mutate-cultmesh.mjs index c75a9041..ac0711b1 100644 --- a/scripts/mutate-cultmesh.mjs +++ b/scripts/mutate-cultmesh.mjs @@ -47,9 +47,12 @@ // dependencies: // // docker build -t cultlib-quic-native-dev -f scripts/quic-native-linux-dev.Dockerfile scripts -// docker run --rm --security-opt seccomp=unconfined -v F:\Projects\CultLib:/src -w /src \ +// docker run --rm --security-opt seccomp=unconfined -v "${PWD}:/src" -w /src ` // cultlib-quic-native-dev bash -lc "node scripts/mutate-cultmesh.mjs native" // +// Both from the repository root; the mount is that root wherever it is, as +// `${PWD}` in PowerShell or `$(pwd)` in a POSIX shell. +// // The seccomp flag is required, not cautious: the ThreadSanitizer configuration // is re-executed under `setarch -R`, and Docker's default profile denies the // personality call that needs. See `explainUnrunnable` below, and the bridge's @@ -455,11 +458,165 @@ const mutations = [ target: "native", honestOn: ["linux-x64", "win32-x64"], // Still a wait, still the host's timeout in the signature, and the poller is - // gone a millisecond later. A scenario that only sleeps cannot tell. + // gone a millisecond later. This used to die for the wrong reason — 1 ms is + // shorter than `closerace`'s 50 ms settle, so the poller left before the + // close began and the scenario complained about its own fixture. It dies on + // `polltimeout` now, which measures the wait against what was asked for. rule: "a positive timeout blocks until an event or the close (loosening: it waits a token 1 ms)", old: "std::chrono::milliseconds(timeout_ms),", new: "std::chrono::milliseconds(1),", }, + // And the rule underneath that one, which nothing reached until `polltimeout`: + // the duration waited on is the host's, not one of the bridge's own. Both + // scenarios that came before it end their waits with a close, so the timeout + // governed nothing either could see and any constant at all passed them. A + // 2000 ms constant survived the entire Linux matrix and Windows: a host asking + // for five seconds silently got two, and every consumer's poll loop spun at two + // and a half times the rate it asked for. + // + // `polltimeout` asks twice, 200 ms and 900 ms, and measures. That is what makes + // a constant unsurvivable rather than merely unlucky: no single value sits in + // both bands, so the entries below are two sides of one rule and not two + // numbers to be tuned against. + { + target: "native", + honestOn: ["linux-x64", "win32-x64"], + rule: "the wait is on the host's timeout (revert: a shorter constant of the bridge's own)", + old: "std::chrono::milliseconds(timeout_ms),", + new: "std::chrono::milliseconds(100),", + }, + { + target: "native", + honestOn: ["linux-x64", "win32-x64"], + // The lengthening, which is the half that looks harmless: every poll still + // blocks, every poll still returns, and nothing anywhere reports an error. + rule: "the wait is on the host's timeout (loosening: a longer constant of the bridge's own)", + old: "std::chrono::milliseconds(timeout_ms),", + new: "std::chrono::milliseconds(2000),", + }, + // The close's wake. It was defended by committed code and had no entry, so the + // table understated what `closerace` covers; these say it. + { + target: "native", + honestOn: ["linux-x64", "win32-x64"], + rule: "the close wakes every blocked poll (revert: the close marks and waits without waking)", + old: " runtime->signal.notify_all();\n" + + " CULTMESH_QUIC_DEBUG_AT_CLOSE(runtime->active_calls);\n", + new: " CULTMESH_QUIC_DEBUG_AT_CLOSE(runtime->active_calls);\n", + }, + { + target: "native", + honestOn: ["linux-x64", "win32-x64"], + // The plausible reading: the wait is for events, so wake it when there are + // events. The wake a close owes has nothing to do with the queue. + // + // Not listed beside it: narrowing `notify_all` to `notify_one`. It survived + // when tried, and it is equivalent rather than uncovered — the first poller + // out notifies all under the gate from its own CallScope destructor, so the + // wake propagates whatever the close asked for. + rule: "the close wakes every blocked poll (loosening: it wakes only when something is queued)", + old: " runtime->signal.notify_all();\n" + + " CULTMESH_QUIC_DEBUG_AT_CLOSE(runtime->active_calls);\n", + new: " if (!runtime->events.empty()) runtime->signal.notify_all();\n" + + " CULTMESH_QUIC_DEBUG_AT_CLOSE(runtime->active_calls);\n", + }, + // Section 4's other half: a call that races the start of a close is refused. + // The header has promised it since it was written and nothing checked it. A + // call admitted instead increments the in-flight count behind a wait that has + // already read it, and that wait can then be left on a count that only reaches + // zero if the late caller happens to leave. + { + target: "native", + honestOn: ["linux-x64", "win32-x64"], + rule: "a call racing the start of a close is refused (revert: the scope admits it)", + old: " if (runtime_->closing) return;\n", + new: "", + }, + { + target: "native", + honestOn: ["linux-x64", "win32-x64"], + // The reading that sounds conservative: if other calls are still inside, the + // close is waiting for them anyway, so one more can do no harm. It is the + // exact case the refusal exists for. + rule: "a call racing the start of a close is refused (loosening: only when nothing else is inside)", + old: " if (runtime_->closing) return;\n", + new: " if (runtime_->closing && runtime_->active_calls == 0) return;\n", + }, + // The two-phase poll, which is how every host sizes its buffer. Reaching it + // needs no listener, no credential and no established connection: a client + // opening to a closed loopback port is refused in about a millisecond and the + // refusal always carries a non-empty reason, which is an 87-byte payload. + // It was written off as unreachable and is neither unreachable nor safe — + // moving the pop above the copy crashes outright. + { + target: "native", + honestOn: ["linux-x64", "win32-x64"], + rule: "a payload that does not fit is refused without being consumed (revert: the refusal pops)", + old: " if (required > payload_capacity) {\n" + + " *out_required = required;\n" + + " return 2;\n" + + " }\n", + new: " if (required > payload_capacity) {\n" + + " *out_required = required;\n" + + " runtime->events.pop_front();\n" + + " return 2;\n" + + " }\n", + }, + { + target: "native", + honestOn: ["linux-x64", "win32-x64"], + // The helpful shape: give the caller what fits, tell it what it missed. It + // reports 2, so a host obeying the header asks again — for an event that is + // no longer there. + rule: "a payload that does not fit is refused without being consumed (loosening: it truncates)", + old: " if (required > payload_capacity) {\n" + + " *out_required = required;\n" + + " return 2;\n" + + " }\n", + new: " if (required > payload_capacity) {\n" + + " *out_required = required;\n" + + " *out_event = queued.header;\n" + + " if (payload_capacity > 0 && payload != nullptr)\n" + + " std::memcpy(payload, queued.payload.data(), static_cast(payload_capacity));\n" + + " runtime->events.pop_front();\n" + + " return 2;\n" + + " }\n", + }, + { + target: "native", + honestOn: ["linux-x64", "win32-x64"], + rule: "the event is copied before it is popped (revert: the pop moves above the copy)", + old: " *out_event = queued.header;\n" + + " if (required > 0 && payload != nullptr)\n" + + " std::memcpy(payload, queued.payload.data(), static_cast(required));\n" + + " *out_required = required;\n" + + " runtime->events.pop_front();\n", + new: " runtime->events.pop_front();\n" + + " *out_event = queued.header;\n" + + " if (required > 0 && payload != nullptr)\n" + + " std::memcpy(payload, queued.payload.data(), static_cast(required));\n" + + " *out_required = required;\n", + }, + { + target: "native", + honestOn: ["linux-x64", "win32-x64"], + // `queued` is a reference into the deque, and the pop destroys what it names. + // This is the spelling that hides that: the header is copied first, so only + // the payload — the part that is a pointer into a freed vector — is read + // afterwards. Both die; this one dies with the header already correct, which + // is why the payload bytes are checked and not only their length. + rule: "the event is copied before it is popped (loosening: only the payload copy moves after it)", + old: " *out_event = queued.header;\n" + + " if (required > 0 && payload != nullptr)\n" + + " std::memcpy(payload, queued.payload.data(), static_cast(required));\n" + + " *out_required = required;\n" + + " runtime->events.pop_front();\n", + new: " *out_event = queued.header;\n" + + " *out_required = required;\n" + + " runtime->events.pop_front();\n" + + " if (required > 0 && payload != nullptr)\n" + + " std::memcpy(payload, queued.payload.data(), static_cast(required));\n", + }, { target: "native", honestOn: ["linux-x64", "win32-x64"], @@ -516,30 +673,29 @@ function testsPass(workspace) { // ThreadSanitizer build is the only thing that sees a notify land on a destroyed // condition variable, and it exists on linux-x64 alone. // -// `holdclose` runs only where the development seam exists, which is the -// assertion build: it drives the quiesce through that seam rather than through a -// sleep, and it is what kills the rules about counting a host call at all. The -// ThreadSanitizer build deliberately keeps the shipped shape — no assertions, no -// seam — because what it is there for is the race in a library built like the -// one that ships. +// `holdclose` and `latecall` run only where the development seam exists, which +// is the assertion build: they drive the quiesce and the refusal through that +// seam rather than through a sleep, and they are what kill the rules about +// counting a host call at all. The ThreadSanitizer build deliberately keeps the +// shipped shape — no assertions, no seam — because what it is there for is the +// race in a library built like the one that ships. +// +// `polltimeout` measures a wait against the timeout it asked for, so it is run +// only where nothing distorts the clock: a sanitizer's slowdown would make its +// bands meaningless, which is the honest limit of that entry. +const assertScenarios = [ + ["holdclose", "3", "64"], + ["latecall", "5"], + ["polltimeout", "3"], + ["payloadfit", "3"], + ["closerace", "20", "256"], +]; const nativeConfigurations = platform === "linux-x64" ? [ - { - name: "asserts", - asserts: "ON", - sanitizer: null, - scenarios: [["holdclose", "3", "64"], ["closerace", "20", "256"]], - }, + { name: "asserts", asserts: "ON", sanitizer: null, scenarios: assertScenarios }, { name: "tsan", asserts: "OFF", sanitizer: "thread", scenarios: [["closerace", "20", "256"]] }, ] - : [ - { - name: "asserts", - asserts: "ON", - sanitizer: null, - scenarios: [["holdclose", "3", "64"], ["closerace", "20", "256"]], - }, - ]; + : [{ name: "asserts", asserts: "ON", sanitizer: null, scenarios: assertScenarios }]; function msquicArguments() { if (platform === "win32-x64") {