diff --git a/src/GameCult.Caching/Contracts/cultcache-store-composition.md b/src/GameCult.Caching/Contracts/cultcache-store-composition.md index c25a36c4..1a03d4b1 100644 --- a/src/GameCult.Caching/Contracts/cultcache-store-composition.md +++ b/src/GameCult.Caching/Contracts/cultcache-store-composition.md @@ -146,15 +146,17 @@ which route wrote it. cannot take the two out of order; a cacheless store takes its own gate. The store reads which one applies, takes it, and checks the answer again under it: `Cache` is written only by the attach, holding both gates: it publishes `Cache` - before its pull and takes it back in the same hold if the pull fails, so no - other thread ever sees the cache come and go. Under the store's gate `Cache` is + before its pull and takes it back in the same hold if the pull fails. Another + thread's unlocked guess may read the tentative cache, but none runs a body + against it: the guess is checked under that cache's gate, which the attach + holds, and fails once the pull retracts it. Under the store's gate `Cache` is settled, and a settled cache never changes, because nothing detaches. A call takes at most three gates: the guessed cache's, the store's, the settled cache's. The pull reads the registry the attach lent, not `Cache`; a variant it loads carries no codec, and the cache resolves it with its own. During an attach the store is attached, so a journal the pull runs that calls the store - is refused as it would be afterwards; a cacheless write that finds a cache is - refused in that same hold. A + is refused as it would be afterwards; a cacheless write that finds a cache + published, during an attach or after it, is refused with the same text. A store must not call `Loaded` while holding its own lock outside the cache's hold. - A journal runs under the cache's gate, so a slow journal slows every writer of diff --git a/src/GameCult.Caching/CultCache.cs b/src/GameCult.Caching/CultCache.cs index ba9de683..9d942717 100644 --- a/src/GameCult.Caching/CultCache.cs +++ b/src/GameCult.Caching/CultCache.cs @@ -2264,7 +2264,8 @@ namespace GameCult.Caching // The only writer of store.Cache, holding the cache's gate and the store's. It publishes Cache before the pull, so the store is // attached for the whole of its hold and every entry point agrees: a journal the pull runs that calls the store is a journal // entering its cache and is refused, as it is after the attach; every other thread waits on the cache's or the store's gate. - // A failed pull takes Cache back in that same hold, so no other thread ever sees the cache come and go, and there is no detach. + // A failed pull takes Cache back in that same hold. Another thread's unlocked guess may read the tentative cache, but none runs a + // body against it: the guess is checked under that cache's gate, which the attach holds, and fails once the pull retracts it. Held(() => store.UnderStoreGate(() => { if (store.Cache != null) @@ -2824,15 +2825,19 @@ namespace GameCult.Caching // The one lock of a store's gate. A thread holds one store's gate at a time; a store call may be entered again from inside it, // except a cacheless write: it owns its store outright, so it is the outermost hold of the store's gate. - internal static T InStoreHold(object storeGate, Func body, bool cacheless = false) + // A cacheless write that re-enters its own store's hold is refused by whichever fact is true: Cache is the one attachment fact, so + // a store with a cache published (an attach's pull included) gets the attached refusal, and one without gets the re-entry refusal. + internal static T InStoreHold(CacheBackingStore store, Func body, bool cacheless = false) { + var storeGate = store._storeGate; if (_storeHeld != null) { if (ReferenceEquals(_storeHeld, storeGate)) { if (cacheless) - throw new InvalidOperationException( - "A cacheless write was made inside a call that already holds this store (an attach's pull, or a store call whose user code made it); write through the cache."); + throw new InvalidOperationException(store.Cache != null + ? CacheBackingStore.AttachedRefusal + : "A cacheless write was made inside a call that already holds this store (a store call whose user code made it); write through the cache."); return body(); } throw new InvalidOperationException( @@ -3586,7 +3591,13 @@ namespace GameCult.Caching // any of it; if the cache refuses a record the call throws and the store keeps its previous view. protected internal Action, IReadOnlyList>? Loaded; - private readonly object _storeGate = new(); + internal readonly object _storeGate = new(); + + // Gates the current thread has taken for store calls, read by tests (InternalsVisibleTo) to pin the bound of three per call. + [ThreadStatic] + internal static int GateTakes; + + internal const string AttachedRefusal = "The store is attached to a cache, write through the cache."; // Written only by CultCache.AddBackingStore, holding the cache's gate and this store's: set before the pull, and set back to null // in the same hold if the pull fails. Read under this store's gate it is settled, and a settled cache never changes, because // nothing detaches. @@ -3647,23 +3658,26 @@ namespace GameCult.Caching var guess = cacheless ? null : Volatile.Read(ref Cache); if (guess != null) { + GateTakes++; var (ran, result) = guess.Held(() => Cache == guess ? (true, body()) : (false, default(T)!)); if (ran) return result; } + GateTakes++; var (done, value, settled) = UnderStoreGate(() => { var cache = Cache; if (cache == null) return (true, body(), (CultCache?)null); if (cacheless) - throw new InvalidOperationException("The store is attached to a cache, write through the cache."); + throw new InvalidOperationException(AttachedRefusal); return (false, default(T)!, cache); }, cacheless); if (done) return value; + GateTakes++; return settled!.Held(() => Cache == settled ? body() : throw new InvalidOperationException( @@ -3692,7 +3706,7 @@ namespace GameCult.Caching }, cacheless: true); // The store's own gate, taken only here. Lock order: a cache's gate, then a store's; nothing takes a store's gate and then a cache's. - internal T UnderStoreGate(Func body, bool cacheless = false) => CultCache.InStoreHold(_storeGate, body, cacheless); + internal T UnderStoreGate(Func body, bool cacheless = false) => CultCache.InStoreHold(this, body, cacheless); // The one writer of the lent registry: a cacheless call for the call, an attach for as long as it holds the store, and a failed // attach puts it back, so a cacheless store holds none between calls and a pull on one loads nothing. Nothing else is lent, and the diff --git a/tests/GameCult.Caching.Tests/CachelessSingleFileTests.cs b/tests/GameCult.Caching.Tests/CachelessSingleFileTests.cs index 48f5a031..b23bed97 100644 --- a/tests/GameCult.Caching.Tests/CachelessSingleFileTests.cs +++ b/tests/GameCult.Caching.Tests/CachelessSingleFileTests.cs @@ -1186,6 +1186,8 @@ namespace GameCult.Caching.Tests private const string ReentryRefusal = "A cacheless write was made inside a call that already holds this store"; + private const string AttachedRefusal = "The store is attached to a cache, write through the cache."; + private const string JournalRefusal = "A journal entered the cache it journals"; // Every store entry point, called by a journal the attach's pull runs on the attaching thread, through a real attach. The store is @@ -1276,7 +1278,7 @@ namespace GameCult.Caching.Tests Assert.That(journalRuns, Is.GreaterThan(0), "the pull's admission ran the journal"); Assert.That(journalError, Is.InstanceOf()); - Assert.That(journalError!.Message, Does.StartWith(op is "writedoc" or "writerecord" ? ReentryRefusal : JournalRefusal)); + Assert.That(journalError!.Message, Does.StartWith(op is "writedoc" or "writerecord" ? AttachedRefusal : JournalRefusal)); Assert.That(store.IsDirty, Is.False, "nothing the cache never admitted is staged"); await cache.UpsertAsync(typeof(IdDeck), new IdDeck { Name = "later" }, new CultRecordKey("later")); @@ -1417,5 +1419,49 @@ namespace GameCult.Caching.Tests foreach (var c in caches) c.Dispose(); } + + // One call takes at most three gates (the guessed cache's, the store's, the settled cache's), however often failing attaches + // publish and retract Cache under it. A retry around the unlocked guess would take one per stale guess and exceed this. + [Test] + public void OneStoreCallTakesAtMostThreeGatesUnderFailingAttachChurn() + { + var registry = CultDocumentRegistry.ForTypes(new[] { typeof(IdDeck) }); + new SingleFileMessagePackBackingStore(_path).WriteDocument(new CultRecordKey("a"), new IdDeck { Name = "a" }, registry: registry); + var store = new RoleStore(_path) { FailAttachPull = true }; + store.ReleaseAttach.Set(); + var caches = Enumerable.Range(0, 4).Select(_ => new CultCache(registry)).ToArray(); + var stop = 0; + var mostTakes = 0; + var calls = 0; + var threads = caches.Select(c => OnThread("attach", () => + { + while (Volatile.Read(ref stop) == 0) + { + try { c.AddBackingStore(store); } + catch (InvalidOperationException e) when (e.Message.Contains("pull fails")) { } + } + })).Concat(Enumerable.Range(0, 4).Select(_ => OnThread("counting-reader", () => + { + while (Volatile.Read(ref stop) == 0) + { + CacheBackingStore.GateTakes = 0; + try { store.ReadStoreSnapshot(); } + catch (InvalidOperationException) { } + var takes = CacheBackingStore.GateTakes; + Interlocked.Increment(ref calls); + int seen; + while (takes > (seen = Volatile.Read(ref mostTakes)) && Interlocked.CompareExchange(ref mostTakes, takes, seen) != seen) { } + } + }))).ToArray(); + Thread.Sleep(TimeSpan.FromSeconds(3)); + Volatile.Write(ref stop, 1); + foreach (var t in threads) + Finishes(t.Thread, "a churn thread"); + + Assert.That(calls, Is.GreaterThan(100), "the readers made calls under churn"); + Assert.That(mostTakes, Is.LessThanOrEqualTo(3), "no call took more than three gates"); + foreach (var c in caches) + c.Dispose(); + } } } diff --git a/unity/org.gamecult.cultlib/CHANGELOG.md b/unity/org.gamecult.cultlib/CHANGELOG.md index c9530cbb..843d60ba 100644 --- a/unity/org.gamecult.cultlib/CHANGELOG.md +++ b/unity/org.gamecult.cultlib/CHANGELOG.md @@ -23,7 +23,7 @@ Releases as 3.0.0 (major). ### Added -- A cacheless `WriteRecord` or `WriteDocument`, and an attach, hold the store's gate while they run, and user code they run (a document's property getters, a constructor while a copied record is read) now throws `InvalidOperationException` if it enters a cache it does not hold or a second store; it used to wait forever against an attach of the same store. A store call that queued on an attach that then failed runs under the gate that owns the store, not beside a cacheless call or a second cache's attach. An attach that fails takes its registry back, so a `PullAll` on the store afterwards is refused as unattached instead of loading records the next cache never sees. A cacheless `WriteRecord` or `WriteDocument` made while the same thread already holds that store (from a journal an attach's pull runs, or from user code another cacheless write of the store runs) throws `InvalidOperationException`; it used to write the file behind the attaching cache and take that cache's registry, so the cache could never flush or pull the store again. A journal that calls its cache's store (`Push`, `Delete`, `PushAll`, `CommitBatch`, `PullAll`, `ReadStoreSnapshot`, `JudgeChecks`) while that cache's `AddBackingStore` runs the journal now throws "A journal entered the cache it journals", as it does after the attach; it used to stage or write records the cache never admitted, so the cache and the file disagreed. +- A cacheless `WriteRecord` or `WriteDocument`, and an attach, hold the store's gate while they run, and user code they run (a document's property getters, a constructor while a copied record is read) now throws `InvalidOperationException` if it enters a cache it does not hold or a second store; it used to wait forever against an attach of the same store. A store call that queued on an attach that then failed runs under the gate that owns the store, not beside a cacheless call or a second cache's attach. An attach that fails takes its registry back, so a `PullAll` on the store afterwards is refused as unattached instead of loading records the next cache never sees. A cacheless `WriteRecord` or `WriteDocument` made while the same thread already holds that store from user code another cacheless write of the store runs, or from a journal an attach's pull runs (then "The store is attached to a cache, write through the cache.", as after the attach) throws `InvalidOperationException`; it used to write the file behind the attaching cache and take that cache's registry, so the cache could never flush or pull the store again. A journal that calls its cache's store (`Push`, `Delete`, `PushAll`, `CommitBatch`, `PullAll`, `ReadStoreSnapshot`, `JudgeChecks`) while that cache's `AddBackingStore` runs the journal now throws "A journal entered the cache it journals", as it does after the attach; it used to stage or write records the cache never admitted, so the cache and the file disagreed. - `SingleFileBackingStore.ReadStoreSnapshot()` reads a store file once, under the store's in-process gate, with the reader a cache's store opens with, and returns the durable snapshot or null when nothing is at the path. `SingleFileBackingStore.WriteRecord(record, catalogEntry, holdsIds, registry)` writes one record without a cache: under the store's lock every other record and catalog entry is copied as stored, the file is replaced atomically, and the header is decided as a flush decides it. It refuses on a store attached to a cache. - `SingleFileBackingStore.WriteDocument(key, document, storedAt, registry)` writes one typed document without a cache, as a cache write does: it plans element ids, encodes the document as `T` (the type its schema id and ids are taken from, also for a derived instance) and marks the store when it holds an id; a failed write gives the minted ids back. It refuses on a store attached to a cache. The `registry` of `WriteRecord` and `WriteDocument` serves that call only; a store keeps none. - `CultCodec.DecodeDocument(key, payload)` decodes a stored payload as a cache load does (`CultCacheMessagePack.CreateCodec(registry)` makes the codec). It reads no file and holds no state. `CultElementIds` stays internal.