Skip to content

Fix unfenced epoch announce in LightEpoch (x86-64) - #2015

Open
tiagonapoli wants to merge 6 commits into
microsoft:mainfrom
tiagonapoli:workstream/lightepoch-x86-minimal-v2
Open

Fix unfenced epoch announce in LightEpoch (x86-64)#2015
tiagonapoli wants to merge 6 commits into
microsoft:mainfrom
tiagonapoli:workstream/lightepoch-x86-minimal-v2

Conversation

@tiagonapoli

@tiagonapoli tiagonapoli commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

A thread entering a protected region announces its epoch with a plain store, which is not ordered against the reclaimer's later load of the same slot. A reclaimer can scan a live reader's slot, see it as free, raise SafeToReclaimEpoch past the reader's epoch, and free a page the reader is about to dereference.

The fix changes which word the claim CAS operates on. No barrier and no atomic is added — the lock cmpxchg was already there, it just lands on a different word.

Scoped to x86-64. Weaker architectures additionally need a release store in Release() and an acquire load in ProtectAndDrain(); that ordering work, along with herd7 models and TLA+ specs, is deliberately left out of this change.

The bug

ReserveEntryForThread(ref entry);                              // CAS: threadId 0 -> tid
(*(tableAligned + entry)).localCurrentEpoch = CurrentEpoch;    // plain store: the announce

Store-then-load-of-another-address is the one reordering TSO permits:

Reader Reclaimer
CAS threadId = tid
store localCurrentEpoch = 5 (buffered)
scan slot → reads 0, treats it as free
SafeToReclaimEpoch = 5, free the page
load page → use-after-free

The fix

The CAS writes localCurrentEpoch directly, so claiming the slot and announcing the epoch are one locked RMW — the announce is globally visible before any load in the protected region can issue:

if (Interlocked.CompareExchange(ref (tableAligned + entry)->localCurrentEpoch, epoch, 0) != 0)
    return false;

// The slot is now exclusively ours, so threadId needs no interlocked write.
(*(tableAligned + entry)).threadId = Metadata.threadId;

localCurrentEpoch doubles as the ownership word, sound because a protected thread never announces epoch 0 (asserted by a test). Release() correspondingly clears threadId before freeing the slot.

An mfence after the announce would also fix it on x86-64, but costs a full barrier on the hottest path in the store and is the kind of line a later refactor drops with no test failing. The relocated CAS makes the atomicity structural: the successful lock cmpxchg is the linearization point.

LightEpoch also moves to its own Garnet.LightEpoch project (git mv, history preserved) so it can be tested and disassembled in isolation.

Violation proof

8-hour hardware run

Two Azure VMs, quarantine litmus, baseline vs fixed (2026-07-30, westus2, x86-64):

impl violations sampled rounds
baseline 3,680 8.97 × 10⁷
this fix 0 5.60 × 10⁸

Local repro

--buggy points the harness at BuggyLightEpoch, a frozen copy of main's version, so both arms run back to back on one machine (20 logical processors, x86-64, 15 s runs):

arm run 1 run 2 run 3 run 4 exit
fixed 0 0 0 0 0 (pass)
buggy 34 77 131 245 1 (violation)

Benchmarks

BDN RawStringOperations, 3 interleaved runs per arm on a dedicated x86 VM (Xeon 8272CL, 16 vCPU): mean |Δ| 1.46%, 5 faster / 5 slower, 0 B allocated on every benchmark in both arms.

GetNotFound had the widest spread, so it was re-run alone with 8 interleaved runs per arm — the spread is host noise, and the fix arm is the steadier of the two:

arm mean median min max stdev CV
baseline 6.960 µs 6.848 6.519 7.761 0.395 5.68%
this fix 6.846 µs 6.826 6.628 7.104 0.140 2.04%

Δ = −1.65% (fix faster), Welch t = 0.77, df = 8.7 — not significant. Baseline's own run-to-run spread is an order of magnitude larger than the effect being chased. No measurable cost.

Copilot AI review requested due to automatic review settings August 4, 2026 01:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a subtle x86-64 memory-ordering race in Tsavorite’s LightEpoch entry/announce path by making the slot-claim CAS operate on localCurrentEpoch (the epoch “announce” word) rather than threadId, preventing a reclaimer from transiently observing a live reader’s slot as free. It also factors LightEpoch into a standalone Garnet.LightEpoch project and adds an isolated hardware litmus harness plus focused unit tests.

Changes:

  • Rework epoch-table slot acquisition to CompareExchange on localCurrentEpoch (0 → announced epoch), then set threadId non-atomically once exclusive ownership is established.
  • Adjust Release() to clear threadId before publishing the slot free via localCurrentEpoch = 0.
  • Add Garnet.LightEpoch + Garnet.LightEpoch.test projects and a LightEpochLitmus playground harness for on-hardware validation.

Reviewed changes

Copilot reviewed 25 out of 26 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
playground/LightEpochLitmus/README.md Documents the on-hardware litmus harness and how to run it (native + Docker).
playground/LightEpochLitmus/QuarantineLitmus.cs Implements the quarantine-style litmus loop (reader/reclaimer/disturbers) and violation detection.
playground/LightEpochLitmus/Program.cs CLI, control run, stress iterations, JSON reporting, and exit codes.
playground/LightEpochLitmus/LightEpochLitmus.csproj Adds a signed standalone executable targeting net10.0 and referencing Garnet.LightEpoch.
playground/LightEpochLitmus/helpers/TwoThreadBarrier.cs Provides a two-thread lockstep barrier + shutdown protocol for the harness.
playground/LightEpochLitmus/helpers/Platform.cs Implements page mapping/unmapping and thread pinning (Windows/Linux).
playground/LightEpochLitmus/helpers/PagePool.cs Provides a fixed page pool mapping and poisoning mechanism for quarantine.
playground/LightEpochLitmus/helpers/EpochUnderTest.cs Wraps fixed vs buggy epoch implementations behind a generic, devirtualized harness-facing API.
playground/LightEpochLitmus/helpers/Emulation.cs Best-effort detection of emulator contexts where memory-ordering results are not meaningful.
playground/LightEpochLitmus/helpers/CoreLayout.cs Selects/publishes a core pinning layout intended to expose the reordering.
playground/LightEpochLitmus/helpers/BuggyLightEpoch.cs Frozen pre-fix LightEpoch copy used as a negative control for the harness.
playground/LightEpochLitmus/Dockerfile Builds and publishes the harness into a runtime image for longer runs.
libs/storage/Tsavorite/cs/test/test.epoch/ProtectionTests.cs Adds lifecycle tests validating slot ownership/cleanup and refresh behavior.
libs/storage/Tsavorite/cs/test/test.epoch/helpers/ParkedReaderThread.cs Adds a helper thread that holds epoch protection to test draining behavior.
libs/storage/Tsavorite/cs/test/test.epoch/helpers/EpochTestBase.cs Adds a shared test base integrating Garnet’s TestBase and epoch setup/teardown.
libs/storage/Tsavorite/cs/test/test.epoch/helpers/EpochProtection.cs Adds a using-scoped protection helper to ensure suspend on assertion failure.
libs/storage/Tsavorite/cs/test/test.epoch/Garnet.LightEpoch.test.csproj Introduces a dedicated LightEpoch test project wired into the solution.
libs/storage/Tsavorite/cs/test/test.epoch/DrainTests.cs Adds drain list correctness tests (ordering, blocking when full, exactly-once execution).
libs/storage/Tsavorite/cs/src/epoch/Murmur3.cs Adds a local Murmur3 hash helper for start-offset selection without relying on other core helpers.
libs/storage/Tsavorite/cs/src/epoch/LightEpoch.TestHooks.cs Adds internal test hooks for reading epoch-table state used by new tests/harness.
libs/storage/Tsavorite/cs/src/epoch/LightEpoch.EntryTable.cs Adds shared helpers for indexed entry access and debug assertions in the split partial class.
libs/storage/Tsavorite/cs/src/epoch/LightEpoch.cs Implements the core fix: slot claim CAS on localCurrentEpoch, adjusted release ordering, and refactoring to partial/IDisposable.
libs/storage/Tsavorite/cs/src/epoch/IEpochAccessor.cs Adds the IEpochAccessor interface in the new epoch project layout.
libs/storage/Tsavorite/cs/src/epoch/Garnet.LightEpoch.csproj Introduces the standalone LightEpoch project with IVT to tests and harness.
libs/storage/Tsavorite/cs/src/core/Tsavorite.core.csproj References the new epoch project and exposes internals to the new test assembly.
Garnet.slnx Wires in the new epoch project, litmus harness, and LightEpoch test project.
Suppressed comments (2)

libs/storage/Tsavorite/cs/src/epoch/LightEpoch.TestHooks.cs:34

  • TestHookThreadIdAt may be observed from another thread in tests/harness scenarios; using a non-volatile read can return stale values or be hoisted by the JIT. Use Volatile.Read for a proper cross-thread observation point.
        /// <summary>
        /// The thread id recorded in epoch table slot <paramref name="entry"/>, or 0 if the slot is free.
        /// </summary>
        internal int TestHookThreadIdAt(int entry) => EntryAt(entry).threadId;

libs/storage/Tsavorite/cs/src/epoch/LightEpoch.cs:549

  • PR description claims no barriers were added, but this change introduces Volatile.Read/Volatile.Write in the slot-claim and release paths. Even on x86 these are observable ordering primitives at the C# memory model level; please update the PR description to match (e.g., clarify that no hardware fence was added on x86-64).

Comment thread playground/LightEpochLitmus/helpers/BuggyLightEpoch.cs
Comment thread libs/storage/Tsavorite/cs/src/epoch/LightEpoch.TestHooks.cs
A thread entering a protected region announced its epoch with a plain store, which is not ordered against the reclaimer's later load of the same slot. A reclaimer could scan a live reader's slot, see it as free, raise SafeToReclaimEpoch past the reader's epoch, and free a page the reader was about to dereference.

The claim CAS now writes localCurrentEpoch directly, so claiming the slot and announcing the epoch are one locked RMW and the announce is globally visible before any load in the protected region can issue. No barrier and no atomic is added; the lock cmpxchg was already there. localCurrentEpoch doubles as the ownership word, sound because a protected thread never announces epoch 0. Release() correspondingly clears threadId before freeing the slot.

LightEpoch moves to its own Garnet.LightEpoch project so it can be tested and disassembled in isolation, with unit tests and a quarantine litmus harness under playground/LightEpochLitmus.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 38cd2f3a-d460-407c-8a96-a7330974ce99
@tiagonapoli
tiagonapoli force-pushed the workstream/lightepoch-x86-minimal-v2 branch from 30d3992 to 216b7b3 Compare August 4, 2026 01:23
Tiago Napoli and others added 5 commits August 3, 2026 18:35
CodeQL builds the whole solution with 'dotnet build -f net8.0', which failed
because the litmus project only targeted net10.0. Inherit the repo default
net8.0;net10.0 and pin the Dockerfile/README commands to net10.0.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b5f062b9-2d72-4cf0-b6f3-4c9beb98d068
Drop the Garnet prefix from the epoch library and its unit test project so
they match the Tsavorite.core / Tsavorite.test.* naming of the rest of the
storage engine.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d306b783-4a33-4673-9e29-790995df8179
Undo the split of LightEpoch into a standalone project: the sources return to
src/core/Epochs/ and the duplicated Murmur3 helper is dropped in favor of the
existing Utility.Murmur3. LightEpochLitmus now references Tsavorite.core.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d306b783-4a33-4673-9e29-790995df8179
LightEpoch already exposed a public Dispose(); declaring the interface adds
nothing and was not part of the fix.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d306b783-4a33-4673-9e29-790995df8179
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d306b783-4a33-4673-9e29-790995df8179
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants