fix(audience): Json guards + DiskStore.Count cache + test-surface cleanup (SDK-239)#706
Merged
Merged
Conversation
ImmutableJeffrey
force-pushed
the
feat/audience-hardening-cleanup
branch
from
954cb4f to
75b0081
Compare
ImmutableJeffrey
changed the base branch from
main
to
feat/audience-diagnostic-surface
ImmutableJeffrey
force-pushed
the
feat/audience-diagnostic-surface
branch
from
85c7b38 to
73d3e46
Compare
ImmutableJeffrey
force-pushed
the
feat/audience-hardening-cleanup
branch
from
75b0081 to
3184bd5
Compare
nattb8
reviewed
Apr 24, 2026
ImmutableJeffrey
force-pushed
the
feat/audience-hardening-cleanup
branch
from
3184bd5 to
46c433e
Compare
…eep dicts Two defences against StackOverflowException (uncatchable in .NET). - MaxDepth = 64 — WriteObject/WriteArray throw FormatException past the cap rather than recursing. - ReferenceEquality visited-set — self-referential or shared- instance containers throw on re-entry. Sibling diamonds (same dict under two independent keys) are NOT cycles; enforced by visited.Remove after the recursive write returns. HashSet<object> is allocated lazily, so scalar-only payloads pay nothing. No behaviour change for any acyclic, shallow caller. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both guards exist because their absence yields StackOverflowException, which the .NET runtime does not catch — the process terminates. Neither guard had a test. Three new tests: - nesting one level past MaxDepth throws FormatException with "nesting exceeds" message. Sabotage: disabling GuardDepth fails the test. - self-referential dict throws FormatException with "cycle" message. Sabotage: disabling the visited.Add(container) check makes the test recurse into the depth guard and the assertion on "cycle" text fails. - shared-child diamond (same dict in two sibling keys) is not treated as a cycle. Sabotage: removing the post-recursion visited.Remove makes the second sibling throw "cycle". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sample diagnostics panels poll QueueSize on a UI tick — at 10 Hz with a non-trivial spool that's a continuous Directory.GetFiles scan for a value that barely changes. Cache the count instead. - Seed at construction from the existing spool so first poll is correct without a pre-mutation branch. Lazy-seeding on first bump had a double-count bug: the seed scan already saw the just-written file, then the +1 delta pushed the cache past the real on-disk count. - Write / Delete / DeleteAll / ApplyAnonymousDowngrade maintain the delta via BumpCount (Interlocked.Add) and TryDelete now returns a bool so "actually removed" vs "already gone" can drive the decrement. - TryDelete folds DirectoryNotFoundException into the true (idempotent) branch — per the MS docs it fires on invalid paths (example: "unmapped drive"), and an unreachable path can't point to a real file — and keeps IOException / UnauthorizedAccessException in the false branch so the cache stays honest when a file survived the delete attempt. FileNotFoundException is deliberately absent: File.Delete silently succeeds when the file is gone, so there is nothing to catch. - Count() reads through Volatile.Read so a caller on a different thread sees the latest published value. Cache invariant: on-disk-count-at-ctor + Σ tracked deltas. Events written outside the DiskStore API (direct File.* calls from tests that plant fixtures) are not tracked and will drift the cache; such tests assert on the post-op filesystem state, not Count(). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The "ForTesting" suffix signalled a test-only affordance, but the accessor is the canonical internal read of the consent level and is used the same way inside the SDK. Drop the suffix. Internal-only API — no SDK consumers break. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ImmutableJeffrey
force-pushed
the
feat/audience-hardening-cleanup
branch
from
46c433e to
ba5f256
Compare
nattb8
approved these changes
Apr 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Json.Serializeguards against cyclic / pathologically deep dicts. Both defences againstStackOverflowException(uncatchable in .NET):MaxDepth = 64cap and a ReferenceEquality visited-set carried through the recursive write. Sibling diamonds (same dict reachable via two independent keys) still serialise.DiskStore.Countcache: seeded eagerly at construction, maintained byWrite/Deletedeltas viaInterlocked.Add.Count()reads viaVolatile.Read. Avoids aDirectory.GetFilesscan per poll.CurrentConsentForTesting→CurrentConsent— the accessor is the canonical internal read of the consent level, not a test-only affordance. Internal-only API; no SDK consumers break.Closes SDK-239. Stacked on PR #705 — retarget this PR to main once #705 merges.
Test plan
dotnet test src/Packages/Audience/Tests/Audience.Tests.csprojpasses.Serialize_NestingExceedsMaxDepth_ThrowsFormatException,Serialize_SelfReferentialDict_ThrowsFormatException,Serialize_SharedChildInSiblingKeys_IsNotTreatedAsCycle.DiskStoreTestspass;Count()matches on-disk state after aWrite/Deletesequence.🤖 Generated with Claude Code