Close lost-cancel window: match under the DashMap entry lock (#81)

[joaquinbejar]

Summary

Closes the lost-cancel race: match_order consumed a maker via pop_entry
(index.pop_front + orders.remove(id)) then re-inserted the partial residual.
A concurrent cancel(id) of the order being matched hit orders.remove(id) →
None → silently no-op'd (no counter decrement); the matcher then re-rested the
residual, so the user's cancel was lost. (Finding #3, the torn snapshot, was
already fixed by #62.)

Changes

• New OrderQueue::match_front: keeps the maker resident in the DashMap and
  runs the match decision + commit under the per-entry shard lock via entry(id),
  committing a FrontAction:
  • Remove (full consume), KeepInPlace (partial, same seq — keeps price-time
    priority), ReplaceAtTail (replenishment: swap value+seq in place, re-key
    index — never leaves orders), SetAside (no-progress park, preserves the
    Decide and document taker TIF / PostOnly / MarketToLimit responsibility at the level #65 zero-visible guard).
• A concurrent cancel/remove serializes on the same shard lock, so it either
  fully wins (removes original + decrements full counters) or fully loses (match
  commits, cancel removes the residual) — never lost, never double-counted.
  ReplaceAtTail re-sequences in place rather than remove+push, so the guarantee
  holds for replenishment too. Stale index→id entries self-heal on the next
  sweep's Vacant branch (next_seq is monotonic, so no live entry is ever
  mistaken).

Verification

• loom protocol model (tests/loom/cancel_match.rs, #![cfg(loom)]): models
  the per-entry-lock protocol (loom can't instrument dashmap/skiplist, documented)
  and asserts conservation, counter↔queue lockstep, and no-lost-cancel. Verified
  non-vacuous (the old remove-then-reinsert shape makes loom find the bug).
  Run: RUSTFLAGS="--cfg loom" cargo test --test loom_cancel_match.
• std-thread stress tests on the real impl (Barrier, deterministic, no
  sleep): match_order ∥ cancel(same id) (2000 iters; ~67% hit the exact
  window) and ∥ cancel(different ids).
• concurrency-auditor: SOUND — linearizable, counters lockstep across all 4
  actions, stale-index self-heal idempotent, no new race, FIFO + FOK dry-run
  preserved.

Scope

match_order ∥ cancel is now safe (rustdoc updated). Two concurrent matchers
on one level still require caller serialization (not expanded here). The
pre-existing update_order quantity-increase remove-then-push window is
unchanged (out of scope; tracked separately).

Testing

cargo test: 408 (lib) + 1 (integration) + 9 (doctests), 0 failed; loom: 1
passed under --cfg loom. cargo clippy --all-targets --all-features -- -D warnings, cargo fmt --all --check, cargo build --release: clean. loom is a
dev-dependency under cfg(loom) only (no runtime dep).

Checklist

• Lost-cancel window closed for all FrontActions; cancel∥match linearizable
• Counter↔queue lockstep; no double-decrement; reviewed SOUND
• FIFO/price-time (Partial fill demotes the resting maker to the back of its price-level queue (price-time priority violation) #39), snapshot (Make snapshot() internally consistent (no torn read between counters and order list) #62), in-place update (Preserve queue position on update_order (UpdateQuantity remove+push loses price-time priority) #64), taker-TIF/FOK (Decide and document taker TIF / PostOnly / MarketToLimit responsibility at the level #65) preserved
• loom model (non-vacuous) + std stress tests; no hang
• loom dev-only (cfg(loom)); no new runtime deps; no production .unwrap()

Closes #81

[Copilot]

Pull request overview

This PR closes the lost-cancel race between match_order and cancel on the same maker by introducing an in-place front-matching protocol: the matcher keeps the maker resident in the DashMap and commits the match outcome while holding the maker’s per-entry lock, so a concurrent cancel serializes on the same lock and cannot “slip through” a pop→reinsert gap.

Changes:

• Add OrderQueue::match_front plus FrontAction/FrontOutcome to atomically decide+commit front-maker mutations under the DashMap entry lock.
• Refactor PriceLevel::match_order to use match_front (supporting remove / keep-in-place / replace-at-tail / set-aside) while keeping counter updates outside the entry lock.
• Add verification via a loom protocol model test (cfg(loom)) and real-implementation std::thread stress tests; wire loom as a cfg-gated dev dependency.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/loom/cancel_match.rs	Adds a loom model that exhaustively checks the cancel-vs-match protocol linearization under cfg(loom).
src/price_level/tests/level.rs	Adds real-implementation concurrent stress tests for match_order vs cancel (same id and different ids).
src/price_level/order_queue.rs	Introduces match_front and related enums to keep makers resident and commit under the per-entry lock.
src/price_level/level.rs	Refactors match_order to drive matching via match_front and updates concurrency rustdoc accordingly.
Cargo.toml	Registers cfg(loom) for linting and adds loom as a cfg-gated dev dependency + explicit loom test target.

---

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

[codecov-commenter]

Codecov Report

❌ Patch coverage is 87.64045% with 11 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/price_level/level.rs	88.70%	7 Missing ⚠️
src/price_level/order_queue.rs	85.18%	4 Missing ⚠️

❌ Your project status has failed because the head coverage (73.70%) is below the target coverage (75.00%). You can increase the head coverage or adjust the target coverage.

Files with missing lines	Coverage Δ
src/price_level/order_queue.rs	92.68% <85.18%> (-2.11%)	⬇️
src/price_level/level.rs	92.51% <88.70%> (+0.02%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[joaquinbejar]

Thanks, all three addressed (no semantic change): removed the hot-path Arc clone via a borrow, set_aside is now a HashSet (no O(n^2)), and the parked maker id/seq are logged as structured fields outside the lock. loom + stress tests still green.