fix(s2n-quic-core): use raw waker vtable for contract assertions

[camshaft]

Description of changes:

The waker contract checker used Arc::strong_count to determine whether a waker had been cloned (stored for later waking). On ARM/aarch64, Arc::strong_count uses a Relaxed load internally. Under weak memory ordering, this allows the checker to observe the strong count decrement from a concurrent wake() + drop without yet observing the wake_called store (Release). This caused false contract violations that manifested as panics with debug assertions.

The fix replaces the Arc::strong_count + AtomicBool approach with explicit clone_count/drop_count/wake_count atomics tracked via a custom RawWaker vtable. The checker loads all counters with Acquire ordering, which establishes a proper happens-before relationship with the Release stores in the vtable functions. This eliminates the reordering window that ARM exposed.

Additionally, check_outcome now accepts an optional debug context parameter (assert_contract_with_context / debug_assert_contract_with_context) so callers can attach diagnostic state to contract violations.

Call-outs:

• sync::primitive visibility was changed from mod to pub(crate) so the contract can use loom-instrumented Arc/AtomicU64 for testing.
• Loom cannot reproduce the original bug because loom's Arc::strong_count uses SeqCst internally, which is stronger than real hardware. The loom test verifies correctness of the new implementation under all the interleavings it's able to, but doesn't guarantee full correctness.

Testing:

• Existing correct_test and incorrect_test pass (basic contract semantics preserved).
• New loom_concurrent_wake_and_check test verifies the fix under all thread interleavings explored by loom (RUSTFLAGS='--cfg loom'). The test models the exact scenario: one thread stores a waker clone, another thread takes and wakes it, while the first thread checks the contract concurrently.
• Confirmed on Graviton hardware: the old code panics with strong_count = 2; wake_called = false; the new code with custom vtable confirmed our Acquire counters see the waker is alive while strong_count disagrees, and the final fix eliminates both false positives.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[Copilot]

Pull request overview

This PR fixes false-positive waker contract violations on weakly-ordered architectures (notably aarch64) by replacing an Arc::strong_count-based heuristic with explicit atomic counters maintained via a custom RawWaker vtable. It also adds optional debug context to improve diagnostics when the contract is violated.

Changes:

• Implement waker contract tracking using a custom RawWakerVTable and clone/drop/wake counters with Acquire/Release synchronization.
• Extend the contract API with assert_contract_with_context / debug_assert_contract_with_context to attach optional diagnostic context to assertion failures.
• Add a loom-modeled concurrent wake-and-check test and adjust sync::primitive visibility to support loom-instrumented primitives.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
quic/s2n-quic-core/src/task/waker/contract.rs	Reworks contract checking to use a custom RawWaker vtable + atomic counters; adds context-aware assertion APIs and a loom concurrency test.
quic/s2n-quic-core/src/sync.rs	Makes sync::primitive pub(crate) so internal modules (like the contract checker) can use loom-instrumented primitives in tests.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[camshaft]

won't the compiler eliminate the computation of the context if it sees that it's never used anywhere?

[Mark-Simulacrum]

Depends on how the context is computed -- the compiler can't eliminate code that might have side effects for sure, but even side-effect-free code that it can't prove is side effect free (e.g. insufficient inlining won't get deleted).

[camshaft]

Fair - I can push a modification to disable it

[Mark-Simulacrum]

nit: I don't think you actually need a second thread, this can just happen in the 'main' thread? That seems like it might cut down runtimes within loom since there's less reordering possible (no need to consider differences between main thread which doesn't do any work and each of the child threads)?