fix(text-json): a JsonElement reports its document's disposal (#2117)

A JsonElement captured before JsonDocument::Dispose() kept answering GetInt32()=10. It now
raises ObjectDisposedException, as .NET does. sizeof(JsonElement) 48 -> 56; consumers rebuild.

THE GATE WAS SA-3 ALL ALONG. #2117's blocker was "enforcing it needs shared disposal state
reachable from every element, i.e. an OBJECT-LAYOUT CHANGE to JsonElement ... gated exactly as
modules/io's #2098 is" -- and #2098 landed under SA-3 (Approval IO-1) on 2026-08-18. SA-3 covers
this shape, so the gate is discharged rather than waived.

The review's framing correction stands: this was never a use-after-free. JsonElement held an
OWNING aliasing shared_ptr, so a captured element kept the tree alive and read LIVE storage. The
defect was a disposed document still serving data.

THE DESIGN IS .NET'S, NOT A WORKAROUND. .NET's JsonElement holds a JsonDocument _parent and an
index, and delegates every accessor to _parent.GetXxx(_idx); each of those begins with
CheckNotDisposed() -- some twenty call sites in JsonDocument.cs. The flag lives with the
DOCUMENT, and the element reaches it through the reference it already holds. This port's element
now points at a shared detail::JsonDocumentState and carries the node as a raw pointer into it --
the direct counterpart of _parent plus _idx.

The separate `bool disposed_` on JsonDocument is DELETED rather than mirrored: two flags for one
fact is what let the document and its elements disagree. Deriving a child element no longer
builds an aliasing shared_ptr at all, so five construction sites got simpler.

TWO BOUNDARIES ARE .NET'S AND BOTH ARE PINNED:

  - A DEFAULT element is undefined, not disposed. .NET keeps them apart -- CheckValidInstance()
    raises InvalidOperationException for a null parent, CheckNotDisposed() raises
    ObjectDisposedException. Writing the guard as `if (!node_) throw` would fail that.
  - A Clone() taken BEFORE disposal survives, because .NET's Clone() delegates to
    _parent.CloneElement(_idx), producing an element rooted in a NEW document.

ValueKind throws too, which is easy to leave out and is not optional: .NET's reads
_parent.GetJsonTokenType, which begins with CheckNotDisposed().

Dispose() still retains the tree, and now buys something for it. .NET frees its buffer there,
but .NET's elements reference the DOCUMENT and are told they are disposed; this port's elements
reference the STATE and would read freed storage if it vanished under them. Retention is what
makes the diagnostic safe -- the cost the class note has recorded since #2110, now paid for a
check rather than for nothing.

Mutations: 5, all caught. Two pins inverted, each of which asserted the defect verbatim. The
layout pin uses shadow structs and asserts the two differ by EXACTLY one pointer, so a change to
any member's own size shows up as a mismatch rather than a number to re-guess.

Gate: 17,388 run, 17,388 passed, 0 failed, 0 skipped across 38 executables (+2, in
SharpRuntimeTests_Text_Json, 298 -> 300). Module graph unchanged at 41/93. No vtable,
base-class, signature or noexcept change. Built in build/ with --parallel 2.
Downstream: zero JsonElement/JsonDocument code sites in cna and mobile-eggbert.

docs/Migration-JsonElementDisposalGuard.md

Author: robertvokac

CLAUDE.md

@@ -0,0 +1,111 @@
+<!-- SPDX-License-Identifier: MIT -->
+<!-- Copyright (c) Robert Vokac and contributors -->
+
+# Migration — a `JsonElement` reports its document's disposal, and `sizeof` grows 48 → 56 (ticket #2117)
+
+*2026-08-19.* A `JsonElement` captured before `JsonDocument::Dispose()` kept answering. It now
+raises `System::ObjectDisposedException`, as .NET does.
+
+Landed under `docs/StandingApprovals.md` **SA-3** (private data members; `sizeof` pinned by a
+layout test, no vtable, base-class, signature or `noexcept` change). **Downstream consumers must
+be recompiled**; no source change is needed.
+
+---
+
+## 1. The gate was SA-3 all along
+
+#2117's recorded blocker: *"enforcing it needs shared disposal state reachable from every
+element, i.e. an OBJECT-LAYOUT CHANGE to `JsonElement`. No design is guessed here; it is gated
+exactly as `modules/io`'s #2098 is."*
+
+#2098 landed under SA-3 (Approval IO-1) on 2026-08-18. SA-3 covers exactly this shape, so the
+gate is discharged rather than waived.
+
+**And the review's framing correction stands:** this was never a use-after-free. `JsonElement`
+held an owning aliasing `shared_ptr`, so a captured element kept the tree alive and read **live**
+storage. The defect was a *disposed document still serving data*.
+
+## 2. The design is .NET's, not a workaround
+
+.NET's `JsonElement` holds a `JsonDocument _parent` and an index, and delegates every accessor to
+`_parent.GetXxx(_idx)`; each of those begins with `CheckNotDisposed()` — some twenty call sites in
+`JsonDocument.cs`. **The flag lives with the document, and the element reaches it through the
+reference it already holds.**
+
+This port now does the same: the element points at a shared `detail::JsonDocumentState` and
+carries the node as a raw pointer into it — the direct counterpart of `_parent` plus `_idx`.
+
+| | Was | Is |
+|---|---|---|
+| `JsonElement` | `shared_ptr<const ordered_json>` aliasing the node | `shared_ptr<JsonDocumentState>` + `const ordered_json*` |
+| `JsonDocument` | `shared_ptr<const ordered_json>` + a separate `bool disposed_` | one `shared_ptr<JsonDocumentState>` |
+| `sizeof(JsonElement)` | **48** | **56** |
+
+Two flags for one fact is what let the document and its elements disagree, so the separate
+`disposed_` bool is **gone** rather than mirrored.
+
+A side effect worth noting: deriving a child element no longer builds an aliasing `shared_ptr` at
+all — `JsonElement(state_, &(*it))` — so five construction sites got simpler.
+
+## 3. What changed observably
+
+| Call after `doc->Dispose()` | Was | Is |
+|---|---|---|
+| `captured.GetInt32()` | `10` | `ObjectDisposedException` |
+| `captured.getValueKindProperty()` | the kind | `ObjectDisposedException` |
+| `captured.GetRawText()`, `ToString()` | the text | `ObjectDisposedException` |
+| `array[1]`, `GetArrayLength()`, `GetProperty(…)` | answered | `ObjectDisposedException` |
+| `captured.Clone()` | a copy | `ObjectDisposedException` |
+| `doc->getRootElementProperty()` | already threw | **unchanged** |
+| double `Dispose()` | already safe | **unchanged** |
+| a **default** `JsonElement` | `Undefined` | **unchanged** (§4) |
+| a `Clone()` taken **before** disposal | — | **still works** (§4) |
+
+`ValueKind` throwing is easy to leave out and is not optional: .NET's reads
+`_parent.GetJsonTokenType`, which begins with `CheckNotDisposed()`. A mutation skipping it there
+is caught.
+
+## 4. Two boundaries that are .NET's, not conveniences
+
+**A default element is undefined, not disposed.** .NET keeps them apart:
+`CheckValidInstance()` raises `InvalidOperationException` for a null parent, `CheckNotDisposed()`
+raises `ObjectDisposedException`. A default element here has no document, so it must keep
+answering "undefined" rather than claiming a disposal that never happened. Writing the guard as
+`if (!node_) throw` would fail that, and the mutation is caught.
+
+**A clone survives.** `Clone()` gives the copy its own state, so it outlives the original
+document — .NET's `Clone()` delegates to `_parent.CloneElement(_idx)`, producing an element rooted
+in a **new** document. A mutation that shares the original state instead is caught.
+
+## 5. `Dispose()` still retains the tree, and now buys something for it
+
+`Dispose()` does not free the parsed tree. .NET frees its buffer there, but .NET's elements hold a
+reference to the *document* and are simply told they are disposed; this port's elements hold a
+reference to the **state** and would read freed storage if it vanished under them.
+
+Retention is what makes the diagnostic safe. It is the cost the class note has recorded since
+#2110 — now paid for a check rather than for nothing.
+
+## 6. Evidence
+
+Five mutations, **all caught**:
+
+| Mutation | Caught by |
+|---|---|
+| the guard never fires | 2 cases |
+| the guard also fires for a default element | `Fix2117_ADefaultElementIsUndefinedNotDisposed` |
+| `Dispose()` does not set the flag | 2 cases |
+| `Clone()` shares the original state | `Fix2117_DisposalReachesElementsHandedOutEarlier` |
+| `ValueKind` skips the guard | 2 cases |
+
+Two pins were inverted — `JsonReviewPinTests.DisposalGuardsThatALREADYWorkAndTheOneThatDoesNot`
+and `JsonGatedBehaviourPins.PIN2117…` — each of which asserted the defect verbatim.
+
+The layout pin uses shadow structs rather than bare numbers, and asserts that the two shadows
+**differ by exactly one pointer**, so a change to any member's own size shows up as a mismatch
+instead of a number to re-guess.
+
+## 7. Downstream, measured
+
+`cna` and `mobile-eggbert` reference `JsonElement` or `JsonDocument` in **zero** code sites.
+Neither was modified. The full-rebuild requirement is recorded here for any future consumer.

docs/Migration-JsonElementDisposalGuard.md

@@ -6,6 +6,7 @@
 #include <string>
 #include "System/IDisposable.hpp"
 #include "System/ObjectDisposedException.hpp"
+#include "System/Text/Json/detail/JsonDocumentState.hpp"
 #include "System/Text/Json/JsonDocumentOptions.hpp"
 #include "System/Text/Json/JsonElement.hpp"
 #include "System/Text/Json/JsonException.hpp"
@@ -32,24 +33,31 @@ namespace System::Text::Json {
      * survives. That is not a leak, and LSan agrees.
      */
     class JsonDocument : public System::IDisposable {
-        std::shared_ptr<const nlohmann::ordered_json> root_;
-        bool disposed_ = false;
+        // #2117: one shared state, so every element handed out can see the disposal. The
+        // separate `disposed_` bool is gone -- two flags for one fact is what let the document
+        // and its elements disagree.
+        std::shared_ptr<detail::JsonDocumentState> state_;
 
-        explicit JsonDocument(std::shared_ptr<const nlohmann::ordered_json> root) : root_(std::move(root)) {}
+        explicit JsonDocument(std::shared_ptr<detail::JsonDocumentState> state) : state_(std::move(state)) {}
 
     public:
         ~JsonDocument() override = default;
 
         /** @brief Releases the root element and marks the document as disposed. */
         void Dispose() override {
-            disposed_ = true;
-            root_.reset();
+            if (state_) state_->disposed.store(true, std::memory_order_relaxed);
+            // The state is NOT released. .NET frees its buffer here, but .NET's elements hold a
+            // reference to the document and are told they are disposed; this port's elements
+            // hold a reference to the STATE and would read freed storage if it went away while
+            // one was alive. Retention is what makes the diagnostic safe -- the cost the class
+            // note has recorded since #2110, now bought for a check rather than for nothing.
+            state_.reset();
         }
 
         /** @return The root JsonElement of this document. @throws System::ObjectDisposedException if disposed. */
         [[nodiscard]] JsonElement getRootElementProperty() const {
-            if (disposed_) throw System::ObjectDisposedException("JsonDocument");
-            return JsonElement(root_);
+            if (!state_) throw System::ObjectDisposedException("JsonDocument");
+            return JsonElement(state_, &state_->root);
         }
 
         /**
@@ -62,7 +70,7 @@ namespace System::Text::Json {
                 // and the depth check all live in ONE place now, because JsonSerializer had a
                 // second, drifted copy of this sequence. See detail::ParseDocumentText.
                 return std::shared_ptr<JsonDocument>(new JsonDocument(
-                    std::make_shared<const nlohmann::ordered_json>(detail::ParseDocumentText(json, options))));
+                    std::make_shared<detail::JsonDocumentState>(detail::ParseDocumentText(json, options))));
             // #2111: this caught only parse_error, so a number literal that overflows a
             // double -- which raises out_of_range, NOT parse_error -- escaped as a std::
             // exception that no caller writing catch(const System::Exception&) could see.

modules/text-json/include/System/Text/Json/JsonDocument.hpp

@@ -8,6 +8,8 @@
 #include "SharpRuntime/SharpRuntimeHelper.hpp"
 #include "System/Text/Json/JsonValueKind.hpp"
 #include "nlohmann/json.hpp"
+#include "System/ObjectDisposedException.hpp"
+#include "System/Text/Json/detail/JsonDocumentState.hpp"
 
 namespace System::Text::Json {
 
@@ -27,23 +29,55 @@ namespace System::Text::Json {
      * — same observable API, simpler implementation.
      */
     class JsonElement {
-        std::shared_ptr<const nlohmann::ordered_json> node_;
+        // #2117: the element points at the document's shared STATE and carries the node as a raw
+        // pointer into it, which is .NET's `_parent` plus `_idx`. It used to hold an aliasing
+        // `shared_ptr` straight to the node, which kept the tree alive -- correct for lifetime,
+        // and with no route back to the document to ask whether it had been disposed.
+        //
+        // sizeof(JsonElement) 48 -> 56 under SA-3; pinned by JsonLayoutPinTests.
+        std::shared_ptr<detail::JsonDocumentState> state_;
+        const nlohmann::ordered_json*              node_ = nullptr;
         std::string propertyName_; // set only for elements obtained via EnumerateObject(); see JsonProperty
 
         friend class JsonProperty;
         friend class JsonDocument;
 
         [[nodiscard]] const nlohmann::ordered_json& require(JsonValueKind expected, const char* what) const;
 
+        /**
+         * @brief The node, after checking the owning document has not been disposed.
+         *
+         * The single choke point .NET spreads over some twenty `CheckNotDisposed()` calls in
+         * `JsonDocument`. Every accessor goes through it, so an element handed out before
+         * `Dispose()` reports the disposal rather than serving data.
+         *
+         * A **default** element has no state and is NOT disposed -- it is undefined, and .NET
+         * distinguishes the two: `CheckValidInstance()` raises `InvalidOperationException` for a
+         * null parent while `CheckNotDisposed()` raises `ObjectDisposedException`. Callers here
+         * keep their existing "undefined" behaviour, so this returns nullptr for that case and
+         * each accessor answers as it always did.
+         */
+        [[nodiscard]] const nlohmann::ordered_json* checkedNode() const {
+            if (state_ && state_->disposed.load(std::memory_order_relaxed))
+                throw System::ObjectDisposedException("JsonDocument");
+            return node_;
+        }
+
     public:
         /** @brief Constructs an undefined JsonElement. */
         JsonElement() = default;
-        /** @brief Wraps a node from a JsonDocument's parsed tree (internal; use JsonDocument::getRootElementProperty()/GetProperty()/etc. instead). */
-        explicit JsonElement(std::shared_ptr<const nlohmann::ordered_json> node) : node_(std::move(node)) {}
+        /** @brief Wraps a node in a JsonDocument's shared state (internal; use JsonDocument::getRootElementProperty()/GetProperty()/etc. instead). */
+        JsonElement(std::shared_ptr<detail::JsonDocumentState> state, const nlohmann::ordered_json* node)
+            : state_(std::move(state)), node_(node) {}
 
-        /** @return The kind of this JSON value. */
+        /**
+         * @return The kind of this JSON value.
+         * @throws System::ObjectDisposedException if the owning document has been disposed
+         *         (#2117). .NET throws here too: `ValueKind` reads `_parent.GetJsonTokenType`,
+         *         which begins with `CheckNotDisposed()`.
+         */
         [[nodiscard]] JsonValueKind getValueKindProperty() const {
-            if (!node_) return JsonValueKind::Undefined;
+            if (!checkedNode()) return JsonValueKind::Undefined;
             if (node_->is_object()) return JsonValueKind::Object;
             if (node_->is_array()) return JsonValueKind::Array;
             if (node_->is_string()) return JsonValueKind::String;
@@ -63,7 +97,7 @@ namespace System::Text::Json {
          * null/empty-string distinction matters.
          */
         [[nodiscard]] std::string GetString() const {
-            if (node_ && node_->is_null()) return {};
+            if (checkedNode() && node_->is_null()) return {};
             return require(JsonValueKind::String, "String").get<std::string>();
         }
 
@@ -92,7 +126,7 @@ namespace System::Text::Json {
         [[nodiscard]] bool TryGetInt64(longcs& value) const;
         /** @brief Tries to get this element's value as a double without throwing on failure. */
         [[nodiscard]] bool TryGetDouble(double& value) const {
-            if (!node_ || !node_->is_number()) { value = 0; return false; }
+            if (!checkedNode() || !node_->is_number()) { value = 0; return false; }
             value = node_->get<double>();
             return true;
         }
@@ -110,7 +144,7 @@ namespace System::Text::Json {
          * **object-layout change to `JsonDocument` and `JsonElement`** — hence blocked, not
          * deferred.
          */
-        [[nodiscard]] std::string GetRawText() const { return node_ ? node_->dump() : std::string(); }
+        [[nodiscard]] std::string GetRawText() const { return checkedNode() ? node_->dump() : std::string(); }
 
         /**
          * @brief Tries to get a named object property.
@@ -124,7 +158,7 @@ namespace System::Text::Json {
             const auto& n = require(JsonValueKind::Object, "Object");
             auto it = n.find(name);
             if (it == n.end()) return false;
-            out = JsonElement(std::shared_ptr<const nlohmann::ordered_json>(node_, &(*it)));
+            out = JsonElement(state_, &(*it));
             return true;
         }
 
@@ -157,17 +191,26 @@ namespace System::Text::Json {
             const auto& arr = require(JsonValueKind::Array, "Array");
             std::vector<JsonElement> result;
             result.reserve(arr.size());
-            for (const auto& item : arr) result.emplace_back(std::shared_ptr<const nlohmann::ordered_json>(node_, &item));
+            for (const auto& item : arr) result.emplace_back(state_, &item);
             return result;
         }
 
         /** @return The properties of this JSON object, in document order. @throws System::InvalidOperationException if not an object. */
         [[nodiscard]] std::vector<JsonProperty> EnumerateObject() const;
 
         /** @return A deep copy of this element that owns its own storage. */
+        /**
+         * @return A deep copy of this element that owns its own storage.
+         *
+         * The copy has its own state, so it survives the original document's `Dispose()` — which
+         * is .NET's contract too: `Clone()` delegates to `_parent.CloneElement(_idx)`, producing
+         * an element rooted in a NEW document (`JsonElement.cs`). Cloning an element whose
+         * document is already disposed throws, because the check runs first.
+         */
         [[nodiscard]] JsonElement Clone() const {
-            if (!node_) return JsonElement();
-            return JsonElement(std::make_shared<const nlohmann::ordered_json>(*node_));
+            if (!checkedNode()) return JsonElement();
+            auto fresh = std::make_shared<detail::JsonDocumentState>(*node_);
+            return JsonElement(fresh, &fresh->root);
         }
 
         /** @return The raw JSON text of this element (same as GetRawText()). */

modules/text-json/include/System/Text/Json/JsonElement.hpp

@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: MIT
+// Copyright (c) Robert Vokac and contributors
+// Portions based on .NET runtime API (MIT License, Copyright .NET Foundation and Contributors)
+#pragma once
+
+#include <atomic>
+#include <utility>
+
+#include "nlohmann/json.hpp"
+
+namespace System::Text::Json::detail {
+
+    /**
+     * @brief The state a `JsonDocument` and every `JsonElement` handed out from it share.
+     *
+     * Ticket #2117 (SR-AUD-324, cause TJ-H). Before it, an element captured before
+     * `JsonDocument::Dispose()` kept answering — not a dangling read (the element held an owning
+     * aliasing `shared_ptr`, so the tree stayed alive) but a **disposed document still serving
+     * data**, where .NET throws.
+     *
+     * @par This is .NET's own structure, not a workaround
+     * .NET's `JsonElement` holds a `JsonDocument _parent` and an index, and delegates every
+     * accessor to `_parent.GetXxx(_idx)`; each of those begins with `CheckNotDisposed()`
+     * (`JsonDocument.cs`, some twenty call sites). So the disposal flag lives with the
+     * **document**, and the element reaches it through the reference it already holds. This
+     * struct is that reference: elements point at the state rather than at a bare tree node, and
+     * carry the node as a raw pointer into it — the direct counterpart of `_parent` plus `_idx`.
+     *
+     * The flag is `std::atomic` because `Dispose()` and a reader may be on different threads and
+     * the previous design gave no guarantee either way; making the answer well-defined costs one
+     * relaxed load per access.
+     */
+    struct JsonDocumentState {
+        nlohmann::ordered_json root;
+        std::atomic<bool>      disposed{false};
+
+        explicit JsonDocumentState(nlohmann::ordered_json parsed) : root(std::move(parsed)) {}
+    };
+
+}  // namespace System::Text::Json::detail

modules/text-json/include/System/Text/Json/detail/JsonDocumentState.hpp

@@ -93,15 +93,15 @@ namespace System::Text::Json {
         const auto& arr = require(JsonValueKind::Array, "Array");
         if (index < 0 || static_cast<size_t>(index) >= arr.size())
             throw System::IndexOutOfRangeException("Index was outside the bounds of the array.");
-        return JsonElement(std::shared_ptr<const nlohmann::ordered_json>(node_, &arr[static_cast<size_t>(index)]));
+        return JsonElement(state_, &arr[static_cast<size_t>(index)]);
     }
 
     std::vector<JsonProperty> JsonElement::EnumerateObject() const {
         const auto& obj = require(JsonValueKind::Object, "Object");
         std::vector<JsonProperty> result;
         result.reserve(obj.size());
         for (auto it = obj.begin(); it != obj.end(); ++it) {
-            JsonElement value(std::shared_ptr<const nlohmann::ordered_json>(node_, &(*it)));
+            JsonElement value(state_, &(*it));
             result.push_back(JsonProperty(it.key(), std::move(value)));
         }
         return result;