Conversation
RFC 7396 merge-patch application recursed into a nested target object via a
held `JsonElement.Mutable`, but the recursion mutated the document (bumping
its version) without refreshing that parent handle. Processing any further
property of the same non-root parent then failed its staleness check —
surfacing as `InvalidOperationException` during the merge, or
`ObjectDisposedException: 'JsonDocument'` when a frozen patch copied into the
merge was read afterwards. The root element is exempt from the staleness
check, so only nested ("real") merges with an object-valued property followed
by another property were affected; the existing RFC 7396 suite (top-level /
single-property-per-level only) missed it.
A recursive merge mutates only the child's own subtree, so the parent's start
index is unchanged and only its cached version is stale. `ApplyMergePatch`
now re-mints the parent handle after each nested merge via a new
`IMutableJsonDocument.RefreshElementUnsafe<TElement>(int)` seam, surfaced as a
public, deliberately unsafe high-performance helper
`JsonMarshal.RefreshUnsafe<T>(in T)`. No `InternalsVisibleTo` is used (it would
leak the internal netstandard `ObsoleteAttribute` polyfill into the Patch
package); the merge reaches the seam alloc-free through constrained generics.
Adds regression tests (nested object-then-sibling merges, frozen-mutable patch
readability, an RFC 7396 correctness fuzz, and direct `JsonMarshal.RefreshUnsafe`
tests), documents the helper under the version-tracking section of
JsonDocumentBuilder.md, and adds the V5.1.17 version history entry.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Contributor
Code Coverage Summary Report - Linux (net10.0)Summary
CoverageCorvus.Json.CodeGeneration - 80.3%
Corvus.Json.CodeGeneration.CSharp - 81.7%
Corvus.Json.ExtendedTypes - 74.9%
Corvus.Json.JsonReference - 76.6%
Corvus.Text.Json - 95%
Corvus.Text.Json.AsyncApi - 64.4%
Corvus.Text.Json.AsyncApi.Amqp - 0.6%
Corvus.Text.Json.AsyncApi.AzureServiceBus - 0%
Corvus.Text.Json.AsyncApi.CodeGeneration - 93.2%
Corvus.Text.Json.AsyncApi.Kafka - 0.6%
Corvus.Text.Json.AsyncApi.Mqtt - 0.9%
Corvus.Text.Json.AsyncApi.Nats - 0.4%
Corvus.Text.Json.AsyncApi.Polly - 100%
Corvus.Text.Json.AsyncApi.Testing - 96.6%
Corvus.Text.Json.AsyncApi.WebSocket - 0%
Corvus.Text.Json.CodeGeneration - 93.1%
Corvus.Text.Json.JMESPath - 93.8%
Corvus.Text.Json.JMESPath.CodeGeneration - 97.7%
Corvus.Text.Json.Jsonata - 91.6%
Corvus.Text.Json.Jsonata.CodeGeneration - 88.5%
Corvus.Text.Json.JsonLogic - 95.1%
Corvus.Text.Json.JsonLogic.CodeGeneration - 92.8%
Corvus.Text.Json.JsonPath - 95.2%
Corvus.Text.Json.JsonPath.CodeGeneration - 96.1%
Corvus.Text.Json.OpenApi - 95.7%
Corvus.Text.Json.OpenApi.CodeGeneration - 98.9%
|
Contributor
Code Coverage Summary Report - Windows (net10.0)Summary
CoverageCorvus.Json.CodeGeneration - 80.3%
Corvus.Json.CodeGeneration.CSharp - 81.7%
Corvus.Json.ExtendedTypes - 74.9%
Corvus.Json.JsonReference - 76.7%
Corvus.Text.Json - 94.8%
Corvus.Text.Json.AsyncApi - 64.4%
Corvus.Text.Json.AsyncApi.Amqp - 0.6%
Corvus.Text.Json.AsyncApi.AzureServiceBus - 0%
Corvus.Text.Json.AsyncApi.CodeGeneration - 93.2%
Corvus.Text.Json.AsyncApi.Kafka - 0.6%
Corvus.Text.Json.AsyncApi.Mqtt - 0.9%
Corvus.Text.Json.AsyncApi.Nats - 0.4%
Corvus.Text.Json.AsyncApi.Polly - 100%
Corvus.Text.Json.AsyncApi.Testing - 96.6%
Corvus.Text.Json.AsyncApi.WebSocket - 0%
Corvus.Text.Json.CodeGeneration - 93.1%
Corvus.Text.Json.JMESPath - 93.8%
Corvus.Text.Json.JMESPath.CodeGeneration - 97.7%
Corvus.Text.Json.Jsonata - 91.6%
Corvus.Text.Json.Jsonata.CodeGeneration - 88.5%
Corvus.Text.Json.JsonLogic - 95.1%
Corvus.Text.Json.JsonLogic.CodeGeneration - 92.8%
Corvus.Text.Json.JsonPath - 95.2%
Corvus.Text.Json.JsonPath.CodeGeneration - 96.1%
Corvus.Text.Json.OpenApi - 95.7%
|
Contributor
Code Coverage Summary Report - Windows (net481)Summary
CoverageCorvus.Json.CodeGeneration - 80.4%
Corvus.Json.CodeGeneration.CSharp - 81.4%
Corvus.Json.ExtendedTypes - 72.6%
Corvus.Json.JsonReference - 73.8%
Corvus.Text.Json - 93.5%
Corvus.Text.Json.CodeGeneration - 86%
Corvus.Text.Json.JMESPath - 93.8%
Corvus.Text.Json.JMESPath.CodeGeneration - 97.7%
Corvus.Text.Json.Jsonata - 91.8%
Corvus.Text.Json.Jsonata.CodeGeneration - 88.6%
Corvus.Text.Json.JsonLogic - 95.1%
Corvus.Text.Json.JsonLogic.CodeGeneration - 92.8%
Corvus.Text.Json.JsonPath - 95.2%
Corvus.Text.Json.JsonPath.CodeGeneration - 96.1%
Corvus.Text.Json.Patch - 97.5%
Corvus.Text.Json.Toon - 88.2%
Corvus.Text.Json.Validator - 95.5%
Corvus.Text.Json.Yaml - 90.1%
Corvus.Toon.SystemTextJson - 90%
Corvus.Yaml.SystemTextJson - 88.6%
|
Contributor
|
mwadams
deleted the
feature/820-using-applymergepatch-from-a-frozen-mutable-patch-document-causes-future-reads-to-fail-with-objectdisposedexception-jsondocument
branch
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
1 participant
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.
Fixes #820.
Problem
JsonMergePatchExtensions.ApplyMergePatchrecurses into a nested target object through a heldJsonElement.Mutable, but the recursion mutates the document (bumping its version) without refreshing that parent handle. The next sibling property of the same non-root parent then fails its staleness check — surfacing asInvalidOperationExceptionduring the merge, orObjectDisposedException: 'JsonDocument'when a value copied in from a frozen patch is read afterwards.The root element is exempt from the staleness check (always index 0, never relocated), so only nested ("real") merges with an object-valued property followed by another property were affected. The existing RFC 7396 suite is all top-level / single-property-per-level, so it never caught it.
See the issue comment for a minimal repro and the full analysis.
Fix
A recursive merge mutates only the child's own subtree, so the parent's start index is unchanged — only its cached version is stale.
ApplyMergePatchnow re-mints the parent handle after each nested merge.IMutableJsonDocument.RefreshElementUnsafe<TElement>(int index)seam (generic over the element type, mirroringFreezeElement<TElement>), implemented inJsonDocumentBuilderandDefaultValueJsonDocument.JsonMarshal.RefreshUnsafe<T>(in T element)— only valid where the caller knows their mutations cannot have changed the element's index in its parent document (i.e. mutations were confined to its own subtree).InternalsVisibleTo— an IVT from the core package to the Patch package leaked the internal netstandardObsoleteAttributepolyfill into Patch (CS0433onnetstandard2.0). The merge reaches the seam allocation-free through the existing constrained-generic pattern.Tests & docs
JsonMergePatchFrozenBuilderTests— nested object-then-sibling merges (the bug), frozen-mutable patch readability after apply, merge into a non-root target, realistic merge.JsonMergePatchFuzzTests— 2000 randomised merges checked against an independent RFC 7396 reference (catches silent corruption, not just throws).JsonMarshalRefreshUnsafeTests— direct coverage of the new helper (refresh after descendant mutation, root no-op, disposed-document throw).JsonMarshal.RefreshUnsafedocumented under the version-tracking / staleness section ofJsonDocumentBuilder.md, clearly flagged as an unsafe helper for high-performance code.VERSIONHISTORY.md→ V5.1.17.Full Patch suite (429) and touched core tests (Mutable/Freeze/DocumentBuilder/JsonMarshal, 4193) pass; core + Patch build clean for
netstandard2.0andnet10.0.🤖 Generated with Claude Code