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") {