Skip to content

fix(json,json-cppagent): cache JsonSerializerOptions singletons — plug DynamicMethod LCG leak (+3.2-3.8 MB/h RSS) - #249

Draft
ottobolyos wants to merge 15 commits into
TrakHound:masterfrom
ottobolyos:fix/json-serializer-options-per-call-leak
Draft

fix(json,json-cppagent): cache JsonSerializerOptions singletons — plug DynamicMethod LCG leak (+3.2-3.8 MB/h RSS)#249
ottobolyos wants to merge 15 commits into
TrakHound:masterfrom
ottobolyos:fix/json-serializer-options-per-call-leak

Conversation

@ottobolyos

@ottobolyos ottobolyos commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Cache JsonSerializerOptions as static readonly singletons in both MTConnect.NET-JSON and MTConnect.NET-JSON-cppagent JsonFunctions.cs, plus every sibling site that re-allocated the option object per call. Prevents unbounded loader-heap growth from LCG-emitted property accessors, observed on two DIME-connector production hosts as a +3.2–3.8 MB/h RSS climb with a flat managed heap.

Root cause

A fresh JsonSerializerOptions instance owns its own serialization-metadata cache. Building that cache emits reflection-based property accessors (DynamicMethod in the runtime's LCG heap) for every property in the reachable type graph. Allocating one on every serialization call therefore re-emits those accessors on every call, and the emitted code accumulates in the runtime's loader heaps where the GC cannot reclaim it.

Peer diagnosis (tempco-investigation-server, 2026-08-21) on two hosts:

  • tempco-001 (amd64, server-GC) and tim-001 (arm64, workstation-GC) — completely different source/sink shapes, 28× Lua/Scriban usage delta between them. The only shared serialization surface was MTConnect JSON.
  • dotnet-counters: GC heap flat (76–87 MB sawtooth); working set climbed (445 → 460 MB); ~370 MB outside the managed heap.
  • Number of Methods Jitted: 17,651,641 in 71 h on tempco-001 (+67/s forever) — healthy service is typically 10 k–50 k lifetime.
  • IL Bytes Jitted: +67,083 B / 90 s = 2.7 MB/h, matching the 3.3 MB/h RSS climb.
  • dotnet-gcdump: System.Text.Json.Serialization.Metadata.ReflectionEmitCachingMemberAccessor+Cache+CacheEntry present.
  • dotnet-trace LCG DynamicMethods: 8946 hits on dynamicClass, 2465 on IObservationOutput accessor, 1371 on JsonEvents, 1161 on JsonSamples.

Why both JsonFunctions.cs files ship in one PR. DIME's MQTT sink at DIME/Connectors/MtConnectMqtt/Sink.cs:49 sets documentFormat: "JSON-cppagent-mqtt", so per-observation serialization on peer's production hosts flowed through MTConnect.NET-JSON-cppagent.JsonFunctions, not the plain one. An earlier peer hot-swap that patched only the plain-JSON assembly did not reduce JIT churn, because DIME's real workload never touched it. MTConnect.NET-JSON-cppagent.dll also defines 25 Streams.Json classes vs 10 in the plain assembly, so the LCG cost per serialization is proportionally larger. Both assemblies carry independent copies of the anti-pattern (five new JsonSerializerOptions sites each in DefaultOptions, IndentOptions, Convert, ConvertBytes, ConvertStream); both must ship together in this same PR, rather than split into a follow-up, to close the leak for cppagent-format consumers.

Fix

Both JsonFunctions.cs files gain:

private static readonly JsonSerializerOptions _defaultOptions = CreateOptions(false);
private static readonly JsonSerializerOptions _indentOptions  = CreateOptions(true);

private static JsonSerializerOptions CreateOptions(bool indented) => new JsonSerializerOptions { ... WriteIndented = indented, ... };

private static JsonSerializerOptions GetOptions(JsonConverter converter, bool indented)
{
    if (converter == null) return indented ? _indentOptions : _defaultOptions;
    var options = CreateOptions(indented);
    options.Converters.Add(converter);
    return options;
}

public static JsonSerializerOptions DefaultOptions => _defaultOptions;
public static JsonSerializerOptions IndentOptions  => _indentOptions;

Convert/ConvertBytes/ConvertStream now delegate through GetOptions(converter, indented). Every in-tree caller (verified via grep across libraries/, agent/, tests/) passes converter == null and does not mutate the returned instance, so every in-tree call reuses the singleton. The cold-path branch preserves the public API contract for external consumers that supply a per-call converter.

Sibling sweep (same static-readonly pattern, folded in atomically here rather than deferred to a follow-up PR):

  • libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs_readOptions (ReadCommentHandling = Skip), 2 call sites.
  • libraries/MTConnect.NET-Common/Configurations/AdapterApplicationConfiguration.cs_readOptions, 2 call sites.
  • libraries/MTConnect.NET-Common/Agents/MTConnectAgentInformation.cs_saveOptions (WriteIndented = true), 1 call site.
  • libraries/MTConnect.NET-Common/Clients/MTConnectClientInformation.cs_saveOptions, 1 call site.
  • libraries/MTConnect.NET-Common/Buffers/MTConnectAssetFileBuffer.cs_writeOptions, 1 call site.
  • libraries/MTConnect.NET-MQTT/MTConnectMqttMessage.cs_agentInformationOptions, 1 call site.

libraries/MTConnect.NET-Common/Buffers/MTConnectObservationFileBuffer.cs was in the peer's sweep list but already calls JsonSerializer.Serialize with no options argument — no anti-pattern present.

Regression tests

tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs and its cppagent mirror pin the singleton contract:

  • DefaultOptions_returns_the_same_instance_on_repeat_accessReferenceEquals(JsonFunctions.DefaultOptions, JsonFunctions.DefaultOptions) == true.
  • IndentOptions_returns_the_same_instance_on_repeat_access — same for the indented preset.
  • DefaultOptions_and_IndentOptions_are_distinct_instances.
  • DefaultOptions_WriteIndented_is_false and IndentOptions_WriteIndented_is_true — behavior guards.
  • JsonFunctions_holds_a_static_readonly_options_field — reflection guard that the shared field exists (blocks the anti-pattern regressing under any future refactor).

Verified RED on upstream/master (587a126): 3 fails + 3 passes per project (6 fails total across both suites). Verified GREEN after fix: 6 pass + 6 pass. Full-suite GREEN on bluefin/net8.0:

Project Pass / Total
MTConnect.NET-JSON-Tests 69 / 69
MTConnect.NET-JSON-cppagent-Tests 369 / 369
MTConnect.NET-Common-Tests 3987 / 3987

Total 4425 tests, zero failures.

Depends on

None. Standalone fix branched off upstream/master.

Efficacy validation

JIT / emit churn eliminated in production; end-to-end RSS-floor confirmation received 2026-08-21 from the same peer that filed the original diagnosis (tempco-investigation-server session, ≥ 22 h post-deploy on tim-001, matched-window controlled comparison against unpatched tempco-001). IL bytes / method transitioned from 22 (emitted accessor stubs) to 108 (ordinary real methods); working set 292–300 MB stable.

Same-host matched-process-age comparison on tim-001 (both DLLs patched, deployed 2026-08-20T23:05:21Z):

Metric Pre-fix (cppagent unpatched) Post-fix (both patched)
Methods/s at ~3 min uptime 198.2 14.2
Methods/s at ~6–10 min 160–205 (at 17–21 min) 0.6–1.3
Total jitted at 3 min 56,254 27,815
Total jitted at 10–20 min 231,709 at 20 min 28,669 at 10 min (plateaued)
Post-fix delta over 5 min +192 methods (28,477 → 28,669)

~225× reduction; curve is FLAT, not merely lower.

Supporting signals:

  • IL bytes / method: 22 (emitted accessor stubs) → 108 (ordinary real methods) — remaining churn is tiered-compilation warm-up, not emit churn.
  • dotnet-trace LCG DynamicMethod count over a 20 s window: 5 862 → 1 276 (post-fix sample still warming when captured; has since flatlined).
  • GC heap unchanged in character (26–28 MB); working set 292–300 MB stable across the window.

Assembly hygiene

Rebuilt cppagent DLL reports .ver 7:0:0:0 identical to the shipped assembly; informational version unchanged; no strong-name change. comm on the .assembly extern sets between pre-fix and post-fix returns empty — no new external references introduced on either assembly. Drop-in binary-compatible.

End-to-end RSS-floor confirmation (2026-08-21)

Controlled comparison, both hosts, identical wall-clock window (08:00–17:00 CDT, 13:00–22:00Z Chicago manufacturing shift), trough-based least squares on 15-min post-GC minima:

tim-001 (patched) tempco-001 (control, unpatched)
Working-day window (13–22Z) +0.96 ± 0.52 MB/h +4.47 ± 0.47 MB/h
Last 6 h +2.44 ± 0.29 MB/h +4.21 ± 0.82 MB/h
Full run +1.14 ± 0.13 MB/h (22.5 h) +3.19 ± 0.05 MB/h (95 h)

Working-day confidence intervals do not overlap. The control host resumed climbing when the shift started; the patched host did not.

Same-host before/after on tim-001 (cleanest measure — same machine, same config, same workload conditions):

Window RSS-floor growth
PRE-FIX (34 h, cppagent unpatched) +3.5 ± 0.4 MB/h
POST-FIX (22.5 h, both patched) +1.14 ± 0.13 MB/h

67 % reduction in RSS-floor growth. Container-limit cap projection moves from ~22 days to ~61 days (against the 2 GB container ceiling).

JIT side-by-side, workload-independent, taken minutes apart:

Metric tim-001 patched (11.4 h up) tempco-001 control (84 h up)
Methods jitted (total) 31 523 20 458 426
Rate 0.3 /s 30.4 /s
IL bytes jitted 0.06 MB/h 1.43 MB/h
GC heap 39 MB 124 MB
Working set 330 MB 597 MB

649× difference in total methods compiled. The patched host's total is essentially just its own startup JIT and has been flat for a day; the control added ~2.8 M methods in the 24 h between measurements.

Residual grower caveat. tim-001's last-6h window reads +2.44 ± 0.29 MB/h — above its own 22-hour average, and JIT cannot account for it (0.3 methods/s = 0.06 MB/h). The residual is entirely anonymous memory (+1.10 MB/h anon out of +1.11 MB/h rss, decomposed against /proc/self/status + smaps; file/shmem/threads/fds/sockets all flat over the 22.5 h window). With the managed GC heap at 39 MB and methods-jitted at 0.3/s, loader/code heap is not growing either — so the residual mechanism is distinct from the reflection-emit class this PR closes. Every sibling JsonSerializerOptions construction site on MTConnect.NET-Common was independently verified as already using the static readonly singleton pattern (either landed by this PR or pre-existing), consistent with the peer's finding. The residual is most likely native-side (glibc arena fragmentation, native allocator growth, or a native-side buffer) and is being characterised in a separate investigation. This PR eliminates the dominant reflection-emit leak (67 % reduction in RSS-floor growth, 649× reduction in cumulative JIT); the residual is out of scope + tracked separately.

Measurement discipline: RestartCount=0 throughout, no memory.events max/oom on either host, no container hit its limit, and tim-001 published normally the whole time (30 mtconnectMqttSink writes/60 s).

Dime review cycles

Cycle 1 — baseline pass on b2703730

  • code-review: [BLOCKER] H1 — the per-call new JsonSerializerOptions on every Convert / ConvertBytes / ConvertStream invocation re-emits LCG DynamicMethod property accessors on every serialization, matching the peer's tempco-001 / tim-001 heap-outside-GC diagnosis (+3.2–3.8 MB/h RSS climb). Fix: cache the options as static readonly singletons. → Fixed @ f1565222 (the initial cache-singletons commit).
  • code-review: [FINDING] M1 — the shared singleton must be frozen (MakeReadOnly on net8+) so a caller's stray Converters.Add fails fast rather than silently polluting the process-wide instance. → Fixed @ adfed47d.
  • security-audit: NO FINDINGS.
  • simplification: NO FINDINGS.
  • improvement: NO FINDINGS.
  • documentation-audit: NO FINDINGS.
  • test-coverage-audit: [TEST] FLOOR gap — thread-safety, cold-path converter path, and Convert overload parity lacked regression pins. → Fixed @ b2703730 (coverage-FLOOR pin batch, TDD RED-first before the H1 fix).

Cycle 2 — verify freeze + warm

  • code-review / security-audit / simplification / improvement: NO FINDINGS.
  • documentation-audit: [DOCS] L1 — the new CreateOptions / GetOptions helpers lacked XML /// blocks explaining the hot-path / cold-path invariant. → Fixed @ c212f6fc.
  • test-coverage-audit: NO FINDINGS.

Cycle 3 — verify L1 docs

  • code-review / security-audit / simplification / improvement / documentation-audit: NO FINDINGS.
  • test-coverage-audit: NO FINDINGS.
  • documentation-audit: [DOCS] L2 — AmE spelling drift in the freshly-added doc comments (serialise, initialise, synchronise). → Fixed @ 38cbeb07.

Cycle 4 — deeper warm-up + AmE re-sweep

  • improvement: [IMPROVE] M1-C3 — the initial warm-up used Serialize<object>(null, options), which only bootstrapped STJ's shared reflection resolver but never named a concrete MTConnect type; the cold LCG emit for the four top-level response envelopes still fell on the first user-facing request. → Fixed @ 432be963 (warm-up traverses MTConnect response graph via typed Serialize calls on JsonStreamsDocument / JsonAssetsDocument(null) / JsonDevicesDocument).
  • documentation-audit: [DOCS] M2-C3 — repo-wide grep surfaced additional AmE-token drift (amortise, synchronise, flavour). → Fixed @ e55badca.
  • test-coverage-audit: [TEST] FLOOR gap — no regression pin for the warm-up-before-MakeReadOnly static-ctor ordering invariant (a reorder would surface as NotSupportedException on the first real serialize). → Fixed @ 86b60a66.
  • Cycle-4 leftovers surfaced by verification pass: [IMPROVE] F-IMP-001 MEDIUM (WarmReachableGraph omits ErrorResponseDocument) + LOW (DefaultOptions / IndentOptions <remarks> state the invariant but not the WHY / LCG loader-heap link) — deferred to cycle 5.

Cycle 5 — cycle-4 leftovers + Error warm-up correctness

  • code-review / security-audit / documentation-audit: NO FINDINGS.
  • improvement: [IMPROVE] F-IMP-001 M-C4 (Error warm-up missing) → Fixed @ 014347b7; [IMPROVE] L-C4 (WHY-LCG remarks) → Fixed @ 014347b7.
  • improvement: [IMPROVE] F-IMP-C5-001 MEDIUM — cycle-5 Error warm-up populated only new ErrorResponseDocument() (null Header / Errors / Version), leaving the concrete MTConnectErrorHeader (9 accessors), Error (2), and System.Version (6) cold on the first real error response. → Fixed @ 6b081eac (populates Header = new MTConnectErrorHeader(), Errors = new[] { new Error() }, Version = new Version(2, 5) so STJ walks the runtime types the production /probe-error and parse-failure paths actually serialize).
  • improvement: [IMPROVE] F-IMP-C5-002 LOW — WarmReachableGraph depends on the null-tolerance of the JsonAssetsDocument(IAssetsResponseDocument) ctor; a future null-guard would turn first assembly load into TypeInitializationException. → Fixed @ 6b081eac (adds an <remarks> block on the ctor documenting the null-tolerance contract, the dependent warm-up site, and the requirement to update both atomically in the same commit before changing the ctor contract).
  • simplification: [SIMPLIFY] F-SIMP-001 LOW — the first <remarks> paragraph on IndentOptions is byte-identical to the paragraph on DefaultOptions in both files. → Closed with rationale: IntelliSense hover shows only the current member's remarks and does not chase <see cref/>; a consumer inspecting IndentOptions in isolation must see the invariant text self-contained, not as a pointer. Self-contained per-member docs are the maintainer preference.
  • test-coverage-audit: [TEST] MEDIUM — cycle-5's initial Frozen_singleton_can_serialize_ErrorResponseDocument_proving_Error_warm_up_ran pin was tautological (mutation-verified: removing the warm-up line left the pin green; STJ's DefaultJsonTypeInfoResolver lazily populates JsonTypeInfo on frozen options for arbitrary types once TypeInfoResolver is set). → Fixed @ 5a196588 (replaced with an IL-inspection walker WarmReachableGraph_IL_contains_newobj_for_ErrorResponseDocument_ctor + companion IL pins for the three Json* top-level surrogates per fixture). [TEST] LOW — the five sibling per-call → static-readonly refactors (AgentConfiguration, AdapterApplicationConfiguration, MTConnectAgentInformation, MTConnectClientInformation, MTConnectAssetFileBuffer, MTConnectMqttMessage) had no test-time guard. → Fixed @ 5a196588 (JsonSerializerOptionsSiblingSingletonTests + MTConnectMqttMessageSingletonTests structural pins, sibling-mutation-verified).

Cycle 6 — verify cycle-5 fixes + walker match against populated shape

  • security-audit / simplification / improvement / documentation-audit: NO FINDINGS.
  • code-review: [FINDING] F-CR-C6-001 LOW — the populated Error warm-up hard-codes new Version(2, 5) where the repo already exposes MTConnectVersions.Version25; reads scan-and-recognize vs. wonder why "2.5" appears amid a 2.7-max library. → Fixed @ ec86e338 (swap to the canonical constant; unused using System; removed from cppagent JsonFunctions.cs).
  • test-coverage-audit: [TEST] F-COV-C6-001 MEDIUM — cycle-5's IL walker only pinned the newobj for ErrorResponseDocument; a revert to new ErrorResponseDocument() (naked) would silently pass the pin while re-opening the exact LCG-emit-on-first-error class F-IMP-C5-001 closed. → Fixed @ e19674ec (adds WarmReachableGraph_IL_contains_newobj_for_concrete_Error_envelope_fields pin per fixture, asserting newobj producers for MTConnectErrorHeader + Errors.Error + System.Version; mutation-verified on bluefin).

Cycle 6 — chase-fix for F-CR-C6-001 side-effect

  • All six agents: NO cycle-7-eligible findings, one same-cycle fix landed:
    • [TEST] chase-fix @ 31668585 — the F-CR-C6-001 swap to MTConnectVersions.Version25 eliminated the newobj System.Version in WarmReachableGraph (an ldsfld now loads the canonical constant), which would have false-positived the cycle-6 walker on System.Version. Widened the IL walker helper AssertWarmReachableGraphNewsUp in both fixtures to accept EITHER newobj T::.ctor OR ldsfld <static-field-of-type-T> — both producers are semantically equivalent for warm-up because STJ walks the runtime type of the value fed into Serialize regardless of source. <summary> blocks updated in both fixtures to name the widened contract and cross-reference F-CR-C6-001. Mutation-verified: deleting Version = MTConnectVersions.Version25 fires the pin with the exact System.Version failure message; restoring returns to green.

Cycle 7 — full-diff re-sweep at HEAD 31668585

  • code-review: NO FINDINGS. Verified: no circular init risk from JsonFunctions.cctor referencing MTConnectVersions.Version25 (leaf static class, no MTConnect deps); widened walker's ldsfld branch endianness sound on every .NET-supported target; sibling singleton adopters follow the pattern with cross-referencing comments back to JsonFunctions.cs.
  • security-audit: NO FINDINGS. Verified: Module.ResolveField(token) operates on the test project's own compiled assembly (no untrusted metadata); no secret exposure in walker error strings; no dependency-manifest change.
  • simplification: NO FINDINGS. Verified: comment blocks on both WarmReachableGraph methods (~28 lines each) trace to distinct closed-finding rationales; the dual-opcode walker branch is clearer as-is than split into two helpers.
  • improvement: NO FINDINGS. Verified: MTConnectVersions.Version25 field-initializer chain has no cycle; widened walker adds zero production-side overhead; using System; correctly retained in plain-JSON (needed for DateTime / TimeSpan in GetTimestamp), correctly removed from cppagent (no unqualified System.* reference remains).
  • documentation-audit: NO FINDINGS. Verified: no docs page references the removed magic new Version(2, 5) literal; both updated <summary> blocks accurately name the widened walker contract and cross-reference F-CR-C6-001; no AmE-token drift in the cycle-6/7 prose.
  • test-coverage-audit: NO FINDINGS. Verified: mutation-tests confirm the widened walker catches every intended regression class (RED on delete of the Version producer, GREEN on restore); nested-field-of-same-type analysis returns no false-positive class.

(Zero unfixed findings — Ready-eligible.)

Test results at HEAD 31668585 (bluefin 2026-08-21, dotnet test per project):

Project Pass / Total
MTConnect.NET-JSON-Tests 82 / 82
MTConnect.NET-JSON-cppagent-Tests 382 / 382
MTConnect.NET-Common-Tests 4085 / 4085
MTConnect.NET-AgentModule-MqttRelay-Tests 63 / 63

Total 4612 tests, zero failures. Net +11 pins over cycle 1 (Error warm-up IL walker + concrete-field pin + sibling singleton pins + MQTT singleton pin + widened walker <summary> updates). All 14 commits upstream/master..HEAD signed (%G? = G).

ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 20, 2026
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 20, 2026
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
…#249) into integration/up-to-pr-249

Cascade rebuild after integration/up-to-pr-241 refresh — PR TrakHound#241 cycle-3
closed 5 LOW dime findings and shifted its tip; the queue-head merge is
re-applied unchanged onto the new base so int-249 stays semantically
identical to the pre-cascade shape.

# Conflicts:
#	libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
… ordering (net8+) — coverage-FLOOR §1.0d-trigies-novodecies

Cycle-3 test-coverage-audit gap on PR TrakHound#249: the XML-doc invariant
"Order matters: the warm-up must run BEFORE MakeReadOnly" (JsonFunctions.cs
static ctor, both flavours) had no named regression pin. The existing
freeze-Add-throws tests prove the freeze happened, but they do not prove
the Serialize<object>(null, _defaultOptions) warm-up completed BEFORE
MakeReadOnly(populateMissingResolver: false). If a future refactor
reordered the two calls — or removed the warm-up entirely — the frozen,
resolver-less singleton would throw NotSupportedException on the first
real-payload Serialize.

Convert's silent catch-all would then swallow the exception into a null
return, degrading any existing "Assert.That(s, Is.Not.Null)" failure into
an opaque "Expected: not null" with no diagnostic pointing at the
warm-up-order regression. The new test calls
JsonSerializer.Serialize(payload, JsonFunctions.DefaultOptions) DIRECTLY
(bypassing Convert's catch) so the failure surfaces as a diagnostic
NotSupportedException at the exact site, with a message naming the
warm-up-before-freeze invariant.

Guarded under #if NET8_0_OR_GREATER since MakeReadOnly + the freeze
pattern only exist on net8+. Mirror pins in both MTConnect.NET-JSON-Tests
and MTConnect.NET-JSON-cppagent-Tests fixtures — both assemblies ship
independent copies of the JsonFunctions surface and must keep the
invariant in lockstep.
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
… ordering (net8+) — coverage-FLOOR §1.0d-trigies-novodecies

Cycle-3 test-coverage-audit gap on PR TrakHound#249: the XML-doc invariant
"Order matters: the warm-up must run BEFORE MakeReadOnly" (JsonFunctions.cs
static ctor, both flavours) had no named regression pin. The existing
freeze-Add-throws tests prove the freeze happened, but they do not prove
the Serialize<object>(null, _defaultOptions) warm-up completed BEFORE
MakeReadOnly(populateMissingResolver: false). If a future refactor
reordered the two calls — or removed the warm-up entirely — the frozen,
resolver-less singleton would throw NotSupportedException on the first
real-payload Serialize.

Convert's silent catch-all would then swallow the exception into a null
return, degrading any existing "Assert.That(s, Is.Not.Null)" failure into
an opaque "Expected: not null" with no diagnostic pointing at the
warm-up-order regression. The new test calls
JsonSerializer.Serialize(payload, JsonFunctions.DefaultOptions) DIRECTLY
(bypassing Convert's catch) so the failure surfaces as a diagnostic
NotSupportedException at the exact site, with a message naming the
warm-up-before-freeze invariant.

Guarded under #if NET8_0_OR_GREATER since MakeReadOnly + the freeze
pattern only exist on net8+. Mirror pins in both MTConnect.NET-JSON-Tests
and MTConnect.NET-JSON-cppagent-Tests fixtures — both assemblies ship
independent copies of the JsonFunctions surface and must keep the
invariant in lockstep.
@ottobolyos
ottobolyos force-pushed the fix/json-serializer-options-per-call-leak branch from 639c986 to 4026ac6 Compare August 21, 2026 06:07
@PatrickRitchie PatrickRitchie moved this from Reviewing to Ready to Merge in MTConnect.NET-Development Aug 21, 2026
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
… ordering (net8+) — coverage-FLOOR §1.0d-trigies-novodecies

Cycle-3 test-coverage-audit gap on PR TrakHound#249: the XML-doc invariant
"Order matters: the warm-up must run BEFORE MakeReadOnly" (JsonFunctions.cs
static ctor, both flavours) had no named regression pin. The existing
freeze-Add-throws tests prove the freeze happened, but they do not prove
the Serialize<object>(null, _defaultOptions) warm-up completed BEFORE
MakeReadOnly(populateMissingResolver: false). If a future refactor
reordered the two calls — or removed the warm-up entirely — the frozen,
resolver-less singleton would throw NotSupportedException on the first
real-payload Serialize.

Convert's silent catch-all would then swallow the exception into a null
return, degrading any existing "Assert.That(s, Is.Not.Null)" failure into
an opaque "Expected: not null" with no diagnostic pointing at the
warm-up-order regression. The new test calls
JsonSerializer.Serialize(payload, JsonFunctions.DefaultOptions) DIRECTLY
(bypassing Convert's catch) so the failure surfaces as a diagnostic
NotSupportedException at the exact site, with a message naming the
warm-up-before-freeze invariant.

Guarded under #if NET8_0_OR_GREATER since MakeReadOnly + the freeze
pattern only exist on net8+. Mirror pins in both MTConnect.NET-JSON-Tests
and MTConnect.NET-JSON-cppagent-Tests fixtures — both assemblies ship
independent copies of the JsonFunctions surface and must keep the
invariant in lockstep.
@ottobolyos
ottobolyos force-pushed the fix/json-serializer-options-per-call-leak branch from 52d98a1 to e55badc Compare August 21, 2026 06:39
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
…on singletons — dime M+L-C4

Cycle-4 leftovers on the JsonSerializerOptions singleton campaign (TrakHound#249).

Fix (M-C4 — F-IMP-001):
Adds MTConnect.Errors.ErrorResponseDocument to the WarmReachableGraph
pass on BOTH JsonFunctions static ctors. ErrorResponseDocument is the
fourth top-level response envelope written directly by
Format(IErrorResponseDocument, ...) in both formatter families — no
Json* surrogate wrapper. Without the addition, the first /probe error,
/current parse failure, or unsupported device request would pay a
cold LCG DynamicMethod emit against the shared, frozen options, which
is the exact hot-path cost the singleton pattern exists to amortize.

Fix (L-C4):
Extends the <remarks> block on DefaultOptions + IndentOptions in both
files with a WHY sentence explaining, in operator-actionable terms,
that mutating a shared JsonSerializerOptions triggers System.Text.Json
to rebuild its metadata cache, which re-emits property accessors as
DynamicMethods into the runtime's LCG loader heaps. Those heaps are
never reclaimed by the GC — a peer measured this at +3.2-3.8 MB/h RSS
in production, which is what the frozen singleton eliminates.

Pin tests:
Adds Frozen_singleton_can_serialize_ErrorResponseDocument_proving_Error_warm_up_ran
to both JsonSerializerOptionsSingletonTests fixtures. Mirrors the
existing SamplePayload warm-up-before-freeze pin — a regression that
dropped ErrorResponseDocument from WarmReachableGraph would surface
here as NotSupportedException on the frozen, resolver-less options.
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
…ResponseDocument pin with IL-inspection; add sibling structural pins — coverage-FLOOR §1.0d-trigies-novodecies

The cycle-4 Frozen_singleton_can_serialize_ErrorResponseDocument_proving_Error_warm_up_ran
pin was tautological — verified by mutation on bluefin 2026-08-21: removing
the ErrorResponseDocument warm-up line from WarmReachableGraph left the pin
green. STJ's DefaultJsonTypeInfoResolver lazily populates JsonTypeInfo on
frozen options for arbitrary types once TypeInfoResolver is set (which any
earlier warm-up call does); MakeReadOnly(populateMissingResolver: false)
only locks the configuration surface, not the internal metadata cache.

Rewrite the pin as WarmReachableGraph_IL_contains_newobj_for_ErrorResponseDocument_ctor:
walk the method body's IL for a newobj instruction whose ResolveMethod
resolves to a ctor of the expected type. Mutation-verified: removing the
warm-up line now cleanly fails the pin. Add companion pin covering the
three Json* top-level response surrogates in each fixture.

Also add sibling-site structural pins for the four MTConnect.NET-Common
per-call `new JsonSerializerOptions(...)` sites flipped to shared static
readonly fields in PR TrakHound#249 (MTConnectAgentInformation, MTConnectClientInformation,
MTConnectAssetFileBuffer, AdapterApplicationConfiguration, AgentConfiguration),
plus the MTConnect.NET-MQTT sibling (MTConnectMqttMessage) in the MqttRelay
test project (which transitively references MTConnect.NET-MQTT). Each pin
asserts at least one private static readonly JsonSerializerOptions field
exists on the type — sibling-mutation-verified on MTConnectAgentInformation.

Net +8 pins across four projects; all fixtures green on bluefin
(net8.0, 4,530 passing).
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
…Document null-tolerance — dime M+L-C5

Cycle-5 leftovers on the JsonSerializerOptions singleton campaign (TrakHound#249).

Fix (M-C5 — F-IMP-C5-001):
Populates the WarmReachableGraph Error envelope with a concrete
MTConnectErrorHeader + a single Error entry + a Version(2, 5), so
STJ walks the RUNTIME types the production /probe-error and
/parse-failure paths actually serialize. STJ resolves accessors for
interface-typed properties (IMTConnectErrorHeader Header,
IEnumerable<IError> Errors) only when the value is non-null; the
prior naked `new ErrorResponseDocument()` warmed the envelope's own
accessors but left the concrete MTConnectErrorHeader (9 properties),
Error (2), and System.Version (6) cold — exactly the LCG-emit class
the singleton pattern exists to eliminate. Populating a
representative graph completes the fix.

Fix (L-C5 — F-IMP-C5-002):
Documents the null-tolerance contract on
`JsonAssetsDocument(IAssetsResponseDocument)` in a <remarks> block.
The plain-JSON WarmReachableGraph calls
`new JsonAssetsDocument(null)` because the surrogate has no public
parameterless ctor. If a future refactor added
`ArgumentNullException.ThrowIfNull(assetsDocument)` there, first
assembly load would fail with TypeInitializationException on any
JSON serialization. The <remarks> names the coupling and directs
future contributors to update the warm-up site atomically per
§1.0d-trigies-bis before changing the ctor contract.

Cycle-5 dispositions:
- code-review: NO FINDINGS
- security-audit: NO FINDINGS
- documentation-audit: NO FINDINGS
- simplification: F-SIMP-001 LOW (dup first-para on IndentOptions
  remarks) — Closed-with-rationale: IntelliSense hover shows only the
  current member's remarks and does not chase <see cref/>; a consumer
  inspecting IndentOptions in isolation must see the invariant text
  self-contained, not as a pointer.
- improvement: F-IMP-C5-001 MEDIUM (this commit); F-IMP-C5-002 LOW
  (this commit).
- test-coverage-audit: 2 findings resolved atomically in 5a19658
  (tautological Error-warm-up pin replaced with IL-inspection walker;
  sibling structural pins added for the five per-call → static-readonly
  refactors).
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
…ion25 — dime L-C6

Cycle-6 leftover on the JsonSerializerOptions singleton campaign (TrakHound#249).

Fix (L-C6 — F-CR-C6-001):
Replaces the magic `new Version(2, 5)` literal in both
`WarmReachableGraph` Error envelopes with the repo's canonical
`MTConnectVersions.Version25` constant. The value is functionally
identical (same `new Version(2, 5)` allocation under the hood) but
removes a hand-typed tuple in favour of the named constant every
other error-doc construction site in the codebase uses
(`MTConnectAgentBroker.GetErrorHeader` sources Version from
`MTConnectVersion` rather than a literal). Reads scan-and-recognise
instead of raising "why 2.5 in a 2.7-max library" questions.

Removes now-unused `using System;` from the cppagent JsonFunctions.cs
(the only unqualified `Version` reference was the magic literal, now
gone; the property type is resolved through the receiver's declared
type).

Cycle-6 dispositions:
- code-review: F-CR-C6-001 LOW (this commit).
- security-audit: NO FINDINGS.
- simplification: NO FINDINGS (cycle-5 F-SIMP-001 duplication class
  intentionally preserved per prior Closed-with-rationale).
- improvement: NO FINDINGS.
- documentation-audit: NO FINDINGS.
- test-coverage-audit: F-COV-C6-001 MEDIUM (concrete Error-envelope
  field pins) — Fixed atomically in e19674e (IL walker now asserts
  newobj for MTConnectErrorHeader + Error + Version, catching a
  revert-to-unpopulated regression that cycle-5's ErrorResponseDocument-only
  pin missed).
@ottobolyos ottobolyos changed the title fix(json,json-cppagent): cache JsonSerializerOptions singletons — plug DynamicMethod LCG leak (+3.2-3.8 MB/h RSS) fix(json,json-cppagent): cache JsonSerializerOptions singletons Aug 21, 2026
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
… ordering (net8+) — coverage-FLOOR

Cycle-3 test-coverage-audit gap on PR TrakHound#249: the XML-doc invariant
"Order matters: the warm-up must run BEFORE MakeReadOnly" (JsonFunctions.cs
static ctor, both flavors) had no named regression pin. The existing
freeze-Add-throws tests prove the freeze happened, but they do not prove
the Serialize<object>(null, _defaultOptions) warm-up completed BEFORE
MakeReadOnly(populateMissingResolver: false). If a future refactor
reordered the two calls — or removed the warm-up entirely — the frozen,
resolver-less singleton would throw NotSupportedException on the first
real-payload Serialize.

Convert's silent catch-all would then swallow the exception into a null
return, degrading any existing "Assert.That(s, Is.Not.Null)" failure into
an opaque "Expected: not null" with no diagnostic pointing at the
warm-up-order regression. The new test calls
JsonSerializer.Serialize(payload, JsonFunctions.DefaultOptions) DIRECTLY
(bypassing Convert's catch) so the failure surfaces as a diagnostic
NotSupportedException at the exact site, with a message naming the
warm-up-before-freeze invariant.

Guarded under #if NET8_0_OR_GREATER since MakeReadOnly + the freeze
pattern only exist on net8+. Mirror pins in both MTConnect.NET-JSON-Tests
and MTConnect.NET-JSON-cppagent-Tests fixtures — both assemblies ship
independent copies of the JsonFunctions surface and must keep the
invariant in lockstep.
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
…on singletons — dime M+L-C4

Cycle-4 leftovers on the JsonSerializerOptions singleton campaign (TrakHound#249).

Fix (M-C4 — F-IMP-001):
Adds MTConnect.Errors.ErrorResponseDocument to the WarmReachableGraph
pass on BOTH JsonFunctions static ctors. ErrorResponseDocument is the
fourth top-level response envelope written directly by
Format(IErrorResponseDocument, ...) in both formatter families — no
Json* surrogate wrapper. Without the addition, the first /probe error,
/current parse failure, or unsupported device request would pay a
cold LCG DynamicMethod emit against the shared, frozen options, which
is the exact hot-path cost the singleton pattern exists to amortize.

Fix (L-C4):
Extends the <remarks> block on DefaultOptions + IndentOptions in both
files with a WHY sentence explaining, in operator-actionable terms,
that mutating a shared JsonSerializerOptions triggers System.Text.Json
to rebuild its metadata cache, which re-emits property accessors as
DynamicMethods into the runtime's LCG loader heaps. Those heaps are
never reclaimed by the GC — a peer measured this at +3.2–3.8 MB/h RSS
in production, which is what the frozen singleton eliminates.

Pin tests:
Adds Frozen_singleton_can_serialize_ErrorResponseDocument_proving_Error_warm_up_ran
to both JsonSerializerOptionsSingletonTests fixtures. Mirrors the
existing SamplePayload warm-up-before-freeze pin — a regression that
dropped ErrorResponseDocument from WarmReachableGraph would surface
here as NotSupportedException on the frozen, resolver-less options.
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
…ResponseDocument pin with IL-inspection; add sibling structural pins — coverage-FLOOR

The cycle-4 Frozen_singleton_can_serialize_ErrorResponseDocument_proving_Error_warm_up_ran
pin was tautological — verified by mutation on bluefin 2026-08-21: removing
the ErrorResponseDocument warm-up line from WarmReachableGraph left the pin
green. STJ's DefaultJsonTypeInfoResolver lazily populates JsonTypeInfo on
frozen options for arbitrary types once TypeInfoResolver is set (which any
earlier warm-up call does); MakeReadOnly(populateMissingResolver: false)
only locks the configuration surface, not the internal metadata cache.

Rewrite the pin as WarmReachableGraph_IL_contains_newobj_for_ErrorResponseDocument_ctor:
walk the method body's IL for a newobj instruction whose ResolveMethod
resolves to a ctor of the expected type. Mutation-verified: removing the
warm-up line now cleanly fails the pin. Add companion pin covering the
three Json* top-level response surrogates in each fixture.

Also add sibling-site structural pins for the four MTConnect.NET-Common
per-call `new JsonSerializerOptions(...)` sites flipped to shared static
readonly fields in PR TrakHound#249 (MTConnectAgentInformation, MTConnectClientInformation,
MTConnectAssetFileBuffer, AdapterApplicationConfiguration, AgentConfiguration),
plus the MTConnect.NET-MQTT sibling (MTConnectMqttMessage) in the MqttRelay
test project (which transitively references MTConnect.NET-MQTT). Each pin
asserts at least one private static readonly JsonSerializerOptions field
exists on the type — sibling-mutation-verified on MTConnectAgentInformation.

Net +8 pins across four projects; all fixtures green on bluefin
(net8.0, 4,530 passing).
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
…Document null-tolerance — dime M+L-C5

Cycle-5 leftovers on the JsonSerializerOptions singleton campaign (TrakHound#249).

Fix (M-C5 — F-IMP-C5-001):
Populates the WarmReachableGraph Error envelope with a concrete
MTConnectErrorHeader + a single Error entry + a Version(2, 5), so
STJ walks the RUNTIME types the production /probe-error and
/parse-failure paths actually serialize. STJ resolves accessors for
interface-typed properties (IMTConnectErrorHeader Header,
IEnumerable<IError> Errors) only when the value is non-null; the
prior naked `new ErrorResponseDocument()` warmed the envelope's own
accessors but left the concrete MTConnectErrorHeader (9 properties),
Error (2), and System.Version (6) cold — exactly the LCG-emit class
the singleton pattern exists to eliminate. Populating a
representative graph completes the fix.

Fix (L-C5 — F-IMP-C5-002):
Documents the null-tolerance contract on
`JsonAssetsDocument(IAssetsResponseDocument)` in a <remarks> block.
The plain-JSON WarmReachableGraph calls
`new JsonAssetsDocument(null)` because the surrogate has no public
parameterless ctor. If a future refactor added
`ArgumentNullException.ThrowIfNull(assetsDocument)` there, first
assembly load would fail with TypeInitializationException on any
JSON serialization. The <remarks> names the coupling and directs
future contributors to update the warm-up site atomically, in the
same commit, before changing the ctor contract.

Cycle-5 dispositions:
- code-review: NO FINDINGS
- security-audit: NO FINDINGS
- documentation-audit: NO FINDINGS
- simplification: F-SIMP-001 LOW (dup first-para on IndentOptions
  remarks) — Closed-with-rationale: IntelliSense hover shows only the
  current member's remarks and does not chase <see cref/>; a consumer
  inspecting IndentOptions in isolation must see the invariant text
  self-contained, not as a pointer.
- improvement: F-IMP-C5-001 MEDIUM (this commit); F-IMP-C5-002 LOW
  (this commit).
- test-coverage-audit: 2 findings resolved atomically in 5a19658
  (tautological Error-warm-up pin replaced with IL-inspection walker;
  sibling structural pins added for the five per-call → static-readonly
  refactors).
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
…ion25 — dime L-C6

Cycle-6 leftover on the JsonSerializerOptions singleton campaign (TrakHound#249).

Fix (L-C6 — F-CR-C6-001):
Replaces the magic `new Version(2, 5)` literal in both
`WarmReachableGraph` Error envelopes with the repo's canonical
`MTConnectVersions.Version25` constant. The value is functionally
identical (same `new Version(2, 5)` allocation under the hood) but
removes a hand-typed tuple in favor of the named constant every
other error-doc construction site in the codebase uses
(`MTConnectAgentBroker.GetErrorHeader` sources Version from
`MTConnectVersion` rather than a literal). Reads scan-and-recognize
instead of raising "why 2.5 in a 2.7-max library" questions.

Removes now-unused `using System;` from the cppagent JsonFunctions.cs
(the only unqualified `Version` reference was the magic literal, now
gone; the property type is resolved through the receiver's declared
type).

Cycle-6 dispositions:
- code-review: F-CR-C6-001 LOW (this commit).
- security-audit: NO FINDINGS.
- simplification: NO FINDINGS (cycle-5 F-SIMP-001 duplication class
  intentionally preserved per prior Closed-with-rationale).
- improvement: NO FINDINGS.
- documentation-audit: NO FINDINGS.
- test-coverage-audit: F-COV-C6-001 MEDIUM (concrete Error-envelope
  field pins) — Fixed atomically in e19674e (IL walker now asserts
  newobj for MTConnectErrorHeader + Error + Version, catching a
  revert-to-unpopulated regression that cycle-5's ErrorResponseDocument-only
  pin missed).
@ottobolyos
ottobolyos force-pushed the fix/json-serializer-options-per-call-leak branch from 3166858 to af2f0ca Compare August 21, 2026 14:13
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
…call-leak) at af2f0ca

# Conflicts:
#	libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
… ordering (net8+) — coverage-FLOOR

Cycle-3 test-coverage-audit gap on PR TrakHound#249: the XML-doc invariant
"Order matters: the warm-up must run BEFORE MakeReadOnly" (JsonFunctions.cs
static ctor, both flavors) had no named regression pin. The existing
freeze-Add-throws tests prove the freeze happened, but they do not prove
the Serialize<object>(null, _defaultOptions) warm-up completed BEFORE
MakeReadOnly(populateMissingResolver: false). If a future refactor
reordered the two calls — or removed the warm-up entirely — the frozen,
resolver-less singleton would throw NotSupportedException on the first
real-payload Serialize.

Convert's silent catch-all would then swallow the exception into a null
return, degrading any existing "Assert.That(s, Is.Not.Null)" failure into
an opaque "Expected: not null" with no diagnostic pointing at the
warm-up-order regression. The new test calls
JsonSerializer.Serialize(payload, JsonFunctions.DefaultOptions) DIRECTLY
(bypassing Convert's catch) so the failure surfaces as a diagnostic
NotSupportedException at the exact site, with a message naming the
warm-up-before-freeze invariant.

Guarded under #if NET8_0_OR_GREATER since MakeReadOnly + the freeze
pattern only exist on net8+. Mirror pins in both MTConnect.NET-JSON-Tests
and MTConnect.NET-JSON-cppagent-Tests fixtures — both assemblies ship
independent copies of the JsonFunctions surface and must keep the
invariant in lockstep.
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
…on singletons — dime M+L-C4

Cycle-4 leftovers on the JsonSerializerOptions singleton campaign (TrakHound#249).

Fix (M-C4 — F-IMP-001):
Adds MTConnect.Errors.ErrorResponseDocument to the WarmReachableGraph
pass on BOTH JsonFunctions static ctors. ErrorResponseDocument is the
fourth top-level response envelope written directly by
Format(IErrorResponseDocument, ...) in both formatter families — no
Json* surrogate wrapper. Without the addition, the first /probe error,
/current parse failure, or unsupported device request would pay a
cold LCG DynamicMethod emit against the shared, frozen options, which
is the exact hot-path cost the singleton pattern exists to amortize.

Fix (L-C4):
Extends the <remarks> block on DefaultOptions + IndentOptions in both
files with a WHY sentence explaining, in operator-actionable terms,
that mutating a shared JsonSerializerOptions triggers System.Text.Json
to rebuild its metadata cache, which re-emits property accessors as
DynamicMethods into the runtime's LCG loader heaps. Those heaps are
never reclaimed by the GC — a peer measured this at +3.2–3.8 MB/h RSS
in production, which is what the frozen singleton eliminates.

Pin tests:
Adds Frozen_singleton_can_serialize_ErrorResponseDocument_proving_Error_warm_up_ran
to both JsonSerializerOptionsSingletonTests fixtures. Mirrors the
existing SamplePayload warm-up-before-freeze pin — a regression that
dropped ErrorResponseDocument from WarmReachableGraph would surface
here as NotSupportedException on the frozen, resolver-less options.
@ottobolyos ottobolyos changed the title fix(json,json-cppagent): cache JsonSerializerOptions singletons fix(json,json-cppagent): cache JsonSerializerOptions singletons — plug DynamicMethod LCG leak (+3.2-3.8 MB/h RSS) Aug 22, 2026
@ottobolyos
ottobolyos force-pushed the fix/json-serializer-options-per-call-leak branch from c0dea3d to c8406d5 Compare August 22, 2026 08:58
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 22, 2026
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 22, 2026
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 23, 2026
… ordering (net8+) — coverage-FLOOR

Cycle-3 test-coverage-audit gap on PR TrakHound#249: the XML-doc invariant
"Order matters: the warm-up must run BEFORE MakeReadOnly" (JsonFunctions.cs
static ctor, both flavors) had no named regression pin. The existing
freeze-Add-throws tests prove the freeze happened, but they do not prove
the Serialize<object>(null, _defaultOptions) warm-up completed BEFORE
MakeReadOnly(populateMissingResolver: false). If a future refactor
reordered the two calls — or removed the warm-up entirely — the frozen,
resolver-less singleton would throw NotSupportedException on the first
real-payload Serialize.

Convert's silent catch-all would then swallow the exception into a null
return, degrading any existing "Assert.That(s, Is.Not.Null)" failure into
an opaque "Expected: not null" with no diagnostic pointing at the
warm-up-order regression. The new test calls
JsonSerializer.Serialize(payload, JsonFunctions.DefaultOptions) DIRECTLY
(bypassing Convert's catch) so the failure surfaces as a diagnostic
NotSupportedException at the exact site, with a message naming the
warm-up-before-freeze invariant.

Guarded under #if NET8_0_OR_GREATER since MakeReadOnly + the freeze
pattern only exist on net8+. Mirror pins in both MTConnect.NET-JSON-Tests
and MTConnect.NET-JSON-cppagent-Tests fixtures — both assemblies ship
independent copies of the JsonFunctions surface and must keep the
invariant in lockstep.
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 23, 2026
…on singletons — dime M+L-C4

Cycle-4 leftovers on the JsonSerializerOptions singleton campaign (TrakHound#249).

Fix (M-C4 — F-IMP-001):
Adds MTConnect.Errors.ErrorResponseDocument to the WarmReachableGraph
pass on BOTH JsonFunctions static ctors. ErrorResponseDocument is the
fourth top-level response envelope written directly by
Format(IErrorResponseDocument, ...) in both formatter families — no
Json* surrogate wrapper. Without the addition, the first /probe error,
/current parse failure, or unsupported device request would pay a
cold LCG DynamicMethod emit against the shared, frozen options, which
is the exact hot-path cost the singleton pattern exists to amortize.

Fix (L-C4):
Extends the <remarks> block on DefaultOptions + IndentOptions in both
files with a WHY sentence explaining, in operator-actionable terms,
that mutating a shared JsonSerializerOptions triggers System.Text.Json
to rebuild its metadata cache, which re-emits property accessors as
DynamicMethods into the runtime's LCG loader heaps. Those heaps are
never reclaimed by the GC — a peer measured this at +3.2–3.8 MB/h RSS
in production, which is what the frozen singleton eliminates.

Pin tests:
Adds Frozen_singleton_can_serialize_ErrorResponseDocument_proving_Error_warm_up_ran
to both JsonSerializerOptionsSingletonTests fixtures. Mirrors the
existing SamplePayload warm-up-before-freeze pin — a regression that
dropped ErrorResponseDocument from WarmReachableGraph would surface
here as NotSupportedException on the frozen, resolver-less options.
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 23, 2026
…ResponseDocument pin with IL-inspection; add sibling structural pins — coverage-FLOOR

The cycle-4 Frozen_singleton_can_serialize_ErrorResponseDocument_proving_Error_warm_up_ran
pin was tautological — verified by mutation on bluefin 2026-08-21: removing
the ErrorResponseDocument warm-up line from WarmReachableGraph left the pin
green. STJ's DefaultJsonTypeInfoResolver lazily populates JsonTypeInfo on
frozen options for arbitrary types once TypeInfoResolver is set (which any
earlier warm-up call does); MakeReadOnly(populateMissingResolver: false)
only locks the configuration surface, not the internal metadata cache.

Rewrite the pin as WarmReachableGraph_IL_contains_newobj_for_ErrorResponseDocument_ctor:
walk the method body's IL for a newobj instruction whose ResolveMethod
resolves to a ctor of the expected type. Mutation-verified: removing the
warm-up line now cleanly fails the pin. Add companion pin covering the
three Json* top-level response surrogates in each fixture.

Also add sibling-site structural pins for the four MTConnect.NET-Common
per-call `new JsonSerializerOptions(...)` sites flipped to shared static
readonly fields in PR TrakHound#249 (MTConnectAgentInformation, MTConnectClientInformation,
MTConnectAssetFileBuffer, AdapterApplicationConfiguration, AgentConfiguration),
plus the MTConnect.NET-MQTT sibling (MTConnectMqttMessage) in the MqttRelay
test project (which transitively references MTConnect.NET-MQTT). Each pin
asserts at least one private static readonly JsonSerializerOptions field
exists on the type — sibling-mutation-verified on MTConnectAgentInformation.

Net +8 pins across four projects; all fixtures green on bluefin
(net8.0, 4,530 passing).
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 23, 2026
…Document null-tolerance — dime M+L-C5

Cycle-5 leftovers on the JsonSerializerOptions singleton campaign (TrakHound#249).

Fix (M-C5 — F-IMP-C5-001):
Populates the WarmReachableGraph Error envelope with a concrete
MTConnectErrorHeader + a single Error entry + a Version(2, 5), so
STJ walks the RUNTIME types the production /probe-error and
/parse-failure paths actually serialize. STJ resolves accessors for
interface-typed properties (IMTConnectErrorHeader Header,
IEnumerable<IError> Errors) only when the value is non-null; the
prior naked `new ErrorResponseDocument()` warmed the envelope's own
accessors but left the concrete MTConnectErrorHeader (9 properties),
Error (2), and System.Version (6) cold — exactly the LCG-emit class
the singleton pattern exists to eliminate. Populating a
representative graph completes the fix.

Fix (L-C5 — F-IMP-C5-002):
Documents the null-tolerance contract on
`JsonAssetsDocument(IAssetsResponseDocument)` in a <remarks> block.
The plain-JSON WarmReachableGraph calls
`new JsonAssetsDocument(null)` because the surrogate has no public
parameterless ctor. If a future refactor added
`ArgumentNullException.ThrowIfNull(assetsDocument)` there, first
assembly load would fail with TypeInitializationException on any
JSON serialization. The <remarks> names the coupling and directs
future contributors to update the warm-up site atomically, in the
same commit, before changing the ctor contract.

Cycle-5 dispositions:
- code-review: NO FINDINGS
- security-audit: NO FINDINGS
- documentation-audit: NO FINDINGS
- simplification: F-SIMP-001 LOW (dup first-para on IndentOptions
  remarks) — Closed-with-rationale: IntelliSense hover shows only the
  current member's remarks and does not chase <see cref/>; a consumer
  inspecting IndentOptions in isolation must see the invariant text
  self-contained, not as a pointer.
- improvement: F-IMP-C5-001 MEDIUM (this commit); F-IMP-C5-002 LOW
  (this commit).
- test-coverage-audit: 2 findings resolved atomically in 5a19658
  (tautological Error-warm-up pin replaced with IL-inspection walker;
  sibling structural pins added for the five per-call → static-readonly
  refactors).
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 23, 2026
…ion25 — dime L-C6

Cycle-6 leftover on the JsonSerializerOptions singleton campaign (TrakHound#249).

Fix (L-C6 — F-CR-C6-001):
Replaces the magic `new Version(2, 5)` literal in both
`WarmReachableGraph` Error envelopes with the repo's canonical
`MTConnectVersions.Version25` constant. The value is functionally
identical (same `new Version(2, 5)` allocation under the hood) but
removes a hand-typed tuple in favor of the named constant every
other error-doc construction site in the codebase uses
(`MTConnectAgentBroker.GetErrorHeader` sources Version from
`MTConnectVersion` rather than a literal). Reads scan-and-recognize
instead of raising "why 2.5 in a 2.7-max library" questions.

Removes now-unused `using System;` from the cppagent JsonFunctions.cs
(the only unqualified `Version` reference was the magic literal, now
gone; the property type is resolved through the receiver's declared
type).

Cycle-6 dispositions:
- code-review: F-CR-C6-001 LOW (this commit).
- security-audit: NO FINDINGS.
- simplification: NO FINDINGS (cycle-5 F-SIMP-001 duplication class
  intentionally preserved per prior Closed-with-rationale).
- improvement: NO FINDINGS.
- documentation-audit: NO FINDINGS.
- test-coverage-audit: F-COV-C6-001 MEDIUM (concrete Error-envelope
  field pins) — Fixed atomically in e19674e (IL walker now asserts
  newobj for MTConnectErrorHeader + Error + Version, catching a
  revert-to-unpopulated regression that cycle-5's ErrorResponseDocument-only
  pin missed).
@ottobolyos
ottobolyos force-pushed the fix/json-serializer-options-per-call-leak branch from bf58c88 to d0f770e Compare August 23, 2026 02:59
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 23, 2026
…g DynamicMethod LCG leak

A fresh JsonSerializerOptions owns its own serialization-metadata cache,
and building that cache emits LCG DynamicMethod property accessors for
every property in the reachable type graph. Allocating one per call
therefore re-emits those accessors on every serialization, and the
emitted code accumulates in the runtime's loader heaps where the GC
cannot reclaim it. Peer diagnosis on two DIME-connector production
hosts (tempco-001 amd64 server-GC, tim-001 arm64 workstation-GC)
observed +3.2–3.8 MB/h RSS climb with a flat managed heap, ~370 MB
outside the managed heap, 9.4–17.6 M methods jitted lifetime, the
ReflectionEmitCachingMemberAccessor cache present in gcdump, and
`dynamicClass` accessor names in the JIT trace — the mechanism is
confirmed at IL level in both shipped JSON assemblies.

Fix: hoist DefaultOptions and IndentOptions to static readonly fields
on both JsonFunctions classes (`MTConnect.NET-JSON` and
`MTConnect.NET-JSON-cppagent`) so a single instance backs every
serialization call. Convert/ConvertBytes/ConvertStream now route
through a shared GetOptions(converter, indented) helper: hot path
(no per-call converter, which is every in-tree caller) returns the
singleton; cold path builds a private instance only when a
caller-supplied converter forces per-call mutation. No in-tree caller
passes a converter or mutates the returned options — assumptions
verified via grep across libraries/, agent/, and tests/.

Sibling sweep: the same static-readonly pattern applied to
AgentConfiguration.ReadJson (2 sites), AdapterApplicationConfiguration.ReadJson
(2 sites), MTConnectAgentInformation.Save, MTConnectClientInformation.Save,
MTConnectAssetFileBuffer.WriteAssetFile, and
MTConnectMqttMessage.CreateAgentInformationMessage — lower-frequency
call sites but same anti-pattern, folded in atomically into this same
commit rather than deferred to a follow-up PR.
MTConnectObservationFileBuffer.cs was in the peer's sweep list but
already uses default options (no `new JsonSerializerOptions` in-method).

RED-first: `tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs`
and its cppagent mirror pin ReferenceEquals across repeat access,
distinct-instance guard on Default vs Indent, WriteIndented shape
guards, and a reflection guard that the static readonly options fields
exist. Verified RED on upstream/master (587a126): 3+3 failing per
project. Verified GREEN after fix: 6+6 passing per project. Full-suite
GREEN on bluefin/net8.0: MTConnect.NET-JSON-Tests 69/69,
MTConnect.NET-JSON-cppagent-Tests 369/369, MTConnect.NET-Common-Tests
3987/3987. Total 4425 tests, zero failures.

Root-cause mechanism is peer-validated at IL level; empirical fix-efficacy
measurement (RSS-floor slope before/after against untouched control)
pending peer's fresh before/after run.
…onvert overload parity — coverage-FLOOR

Extends the JsonSerializerOptionsSingletonTests regression fixture in
both MTConnect.NET-JSON-Tests and MTConnect.NET-JSON-cppagent-Tests
with seven additional pins beyond the ReferenceEquals guards that
already ship on the PR head:

- Convert_is_thread_safe_across_100_concurrent_callers — spins up 100
  gated threads that all call JsonFunctions.Convert against the shared
  DefaultOptions; every output must equal a single-threaded canonical
  and no exception may escape. Pins the STJ thread-safe-for-read
  contract we now rely on.
- Convert_is_thread_safe_under_ParallelFor — same guarantee via the
  ThreadPool scheduling path DIME's MQTT sink actually uses.
- Convert_with_custom_converter_does_not_mutate_DefaultOptions_converters
  — cold-path pin. Freezes the singleton, then calls Convert with a
  caller-supplied JsonConverter and asserts DefaultOptions.Converters
  and IndentOptions.Converters counts are unchanged. Guards the
  observable side of the GetOptions cold-path branch — a regression
  that appended to the singleton would both pollute it and throw once
  the converter list froze.
- Convert_with_custom_converter_uses_converter_without_affecting_singleton_output
  — pins that the fresh cold-path options object actually applies the
  caller's converter (via a NoopConverter that drops the Child field),
  and that a following converter-less call is byte-identical to the
  original singleton output.
- Convert_ConvertBytes_ConvertStream_produce_identical_output_for_compact
  and _for_indented — smoke tests each Convert / ConvertBytes /
  ConvertStream overload for the compact and indented presets and
  asserts UTF-8 decode / stream read match Convert exactly. Guards
  against a future refactor routing one overload through a divergent
  options instance.
- Convert_overloads_return_null_for_null_input — pins the documented
  null-swallow behavior on the singleton refactor.

Verified GREEN on bluefin (dotnet 10.0.302, net8.0):
- MTConnect.NET-JSON-Tests: Passed 76 / Failed 0 (was 69 / 0).
- MTConnect.NET-JSON-cppagent-Tests: Passed 376 / Failed 0 (was 369 / 0).
Exit 0 both. Total +14 new tests across the two mirrored fixtures.

Local sandbox lacks the net9.0 SDK, so the dotnet-test run was
dispatched to the bluefin runner (ts_p15g2-bluefin) per the repo's
policy that resource-intensive test runs happen there rather than
in the local sandbox.
…ime H1+M1

The shared JsonSerializerOptions singletons in MTConnect.NET-JSON and
MTConnect.NET-JSON-cppagent were documented as reusable but not
enforced as immutable — a careless caller could still mutate the
Converters collection (or any other writable property) and silently
corrupt every other in-process serializer that shares the instance.
Ultrareview cycle 1 (2026-08-21) flagged this as a 3-agent convergent
HIGH finding across docs + code-review + improvement.

Fix — freeze the singletons (H1):
- Add a static constructor on both JsonFunctions classes that calls
  MakeReadOnly(populateMissingResolver: false) on _defaultOptions and
  _indentOptions under #if NET8_0_OR_GREATER. Attempted mutation on
  net8+ now throws InvalidOperationException at the point of the
  offending Add, rather than silently polluting the shared instance.
- On older TFMs (netstandard2.0, net4.6.1–net4.8, net6.0, net7.0)
  MakeReadOnly is not available on the STJ surface those runtimes
  ship, so the guard is compile-time gated. The <remarks> block on
  DefaultOptions / IndentOptions calls out that the immutability
  contract still holds by convention on those TFMs.
- Add matching <remarks> XML docs on both DefaultOptions and
  IndentOptions properties in both files, stating the singleton +
  do-not-mutate + net8+ throws contract explicitly. Consumers reading
  IntelliSense now see the constraint at the call site.
- Extend the JsonSerializerOptionsSingletonTests regression fixture
  in both MTConnect.NET-JSON-Tests and MTConnect.NET-JSON-cppagent-Tests
  with two net8+-guarded freeze pins per side:
  * DefaultOptions_Converters_Add_throws_InvalidOperationException_when_frozen
  * IndentOptions_Converters_Add_throws_InvalidOperationException_when_frozen
  Each asserts Assert.Throws<InvalidOperationException> on
  Converters.Add(new NoopConverter()) so a future regression that
  drops MakeReadOnly (or accidentally rebuilds one of the singletons
  as a fresh writable instance) fails loudly.

Warm-up at load (M1):
- The static ctor also runs JsonSerializer.Serialize<object>(null,
  _defaultOptions) and the same for _indentOptions BEFORE the
  MakeReadOnly calls. The warm-up pays the reflection-resolver
  bootstrap cost at assembly-load time rather than on the first
  production /current or /sample request under load.
- Order matters and is called out in the comment: MakeReadOnly(false)
  freezes the options WITHOUT choosing a TypeInfoResolver, so a
  subsequent Serialize on a resolver-less, frozen options would throw
  NotSupportedException. Running Serialize first lets STJ auto-populate
  the resolver via its normal lazy path, after which MakeReadOnly(false)
  is a pure lock with no side effect on serialization.

Verified locally (dotnet 8.0.104, net8.0):
- MTConnect.NET-JSON: build 0/0, JsonSerializerOptionsSingletonTests
  filter 15 passed / 0 failed (was 13/0 before the two freeze pins).
- MTConnect.NET-JSON-cppagent: build 0/0, filter 15 passed / 0 failed
  (was 13/0).

Full-suite validation runs on bluefin next.
… dime L1

The private CreateOptions(bool indented) and GetOptions(JsonConverter,
bool) helpers on JsonFunctions carried inline comments in the method
body but no /// <summary> XML doc blocks. Ultrareview cycle 1 flagged
this as a LOW documentation gap — the two helpers embody the
singleton-vs-fresh contract at the heart of the leak fix, so their
role deserves first-class IntelliSense-visible documentation rather
than implicit knowledge in the method body.

Fix: add <summary> + <remarks> XML docs to both helpers in both files
(MTConnect.NET-JSON and MTConnect.NET-JSON-cppagent), naming the
contract explicitly:

- CreateOptions: returns a fresh instance; intended only for
  static-init and the cold-path branch of GetOptions; every call
  allocates + re-emits the STJ reflection metadata cache and must
  therefore never sit on a hot-path serialization site.
- GetOptions: hot path returns the shared frozen singleton (every
  in-tree caller hits this branch); cold path allocates a fresh
  instance per call and appends the caller's converter. The
  cold-path branch is not shared between callers — per-call
  concurrent use is safe (each call owns its options); sharing a
  converter across cold callers is safe iff the converter itself
  is thread-safe.

Build verified locally on net8.0: both projects 0 warnings, 0 errors.
Cycle-1 landings on JsonFunctions.cs (cppagent flavor) and the two
JsonSerializerOptionsSingletonTests fixtures used the BrE spellings
`serialisation` / `behaviour` in comments and Assert.That failure
messages. MTConnect.NET's canonical spelling rule is AmE inside
committed source — including code comments and XML docs — with BrE
reserved for user-authored prose only. Cycle-1 Ultrareview flagged
the drift as a LOW code-review finding.

Fix: normalize the BrE tokens to AmE across all three affected files:
- libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs — one
  occurrence in the LCG-heap comment (`serialisation` → `serialization`).
- tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs
  — 3× `serialisation` → `serialization`, 8× `behaviour` → `behavior`.
- tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs
  — 2× `serialisation` → `serialization`, 7× `behaviour` → `behavior`.

Verified no assertion value depends on the BrE spelling before the
rewrite: every match landed in a code comment, a `/// <summary>` block,
or the optional failure-message argument of `Assert.That` — none in
the value under test. Regression fixture reruns green (15/15 on both
sides) after normalization.
… ordering (net8+) — coverage-FLOOR

Cycle-3 test-coverage-audit gap on PR TrakHound#249: the XML-doc invariant
"Order matters: the warm-up must run BEFORE MakeReadOnly" (JsonFunctions.cs
static ctor, both flavors) had no named regression pin. The existing
freeze-Add-throws tests prove the freeze happened, but they do not prove
the Serialize<object>(null, _defaultOptions) warm-up completed BEFORE
MakeReadOnly(populateMissingResolver: false). If a future refactor
reordered the two calls — or removed the warm-up entirely — the frozen,
resolver-less singleton would throw NotSupportedException on the first
real-payload Serialize.

Convert's silent catch-all would then swallow the exception into a null
return, degrading any existing "Assert.That(s, Is.Not.Null)" failure into
an opaque "Expected: not null" with no diagnostic pointing at the
warm-up-order regression. The new test calls
JsonSerializer.Serialize(payload, JsonFunctions.DefaultOptions) DIRECTLY
(bypassing Convert's catch) so the failure surfaces as a diagnostic
NotSupportedException at the exact site, with a message naming the
warm-up-before-freeze invariant.

Guarded under #if NET8_0_OR_GREATER since MakeReadOnly + the freeze
pattern only exist on net8+. Mirror pins in both MTConnect.NET-JSON-Tests
and MTConnect.NET-JSON-cppagent-Tests fixtures — both assemblies ship
independent copies of the JsonFunctions surface and must keep the
invariant in lockstep.
…— dime M1-C3

Cycle-2's static-ctor warm-up used JsonSerializer.Serialize<object>(null,
options), which bootstrapped the shared STJ reflection resolver but
never named a concrete MTConnect type. The reachable-graph LCG
DynamicMethod emit for JsonStreamsDocument, JsonAssetsDocument, and
JsonDevicesDocument (and their cppagent counterparts) therefore still
fell on the first user-facing /current | /sample | /assets request —
the exact cold-first-request cost the warm-up was supposed to amortize.

Replace the null-typed Serialize<object>(null, …) calls with typed
Serialize calls against instances of each MTConnect top-level response
surrogate, so STJ configures JsonTypeInfo (and emits the LCG accessors)
for the whole reachable graph rooted at each type. Extracted to a
private WarmReachableGraph(options) helper called once per singleton
(compact + indented) from each assembly's static constructor.

  - MTConnect.NET-JSON: warms JsonStreamsDocument (parameterless ctor),
    JsonAssetsDocument (single (IAssetsResponseDocument) ctor tolerates
    null), JsonDevicesDocument (parameterless ctor).
  - MTConnect.NET-JSON-cppagent: warms JsonStreamsResponseDocument,
    JsonAssetsResponseDocument, JsonDevicesResponseDocument (all
    expose public parameterless ctors for JSON deserialization).

The warm-up ordering pin from cycle-3's test-coverage audit
(Frozen_singleton_can_serialize_a_real_payload_proving_warm_up_ran_before_MakeReadOnly)
still holds — the typed Serialize calls run BEFORE MakeReadOnly, so
the resolver is populated before the freeze; test comments updated
to name the new WarmReachableGraph mechanism.
…or — dime M2-C3

Sweep-through of BrE tokens missed by cycle-2's L2 sweep, converting
each to its AmE spelling per the repo's canonical rule: AmE inside
committed source, including code and code comments.

  - libraries/MTConnect.NET-JSON/JsonFunctions.cs:
    * amortise → amortize (CreateOptions <remarks>)
    * synchronised → synchronized (GetOptions <remarks>)
  - libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs:
    same two tokens in the mirror surface.
  - tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs:
    * cppagent-flavoured → cppagent-flavored (fixture summary)
    * cppagent flavour → cppagent flavor (thread-safety pin summary)

Doc-comment-only change; no test assertion string depends on any of
these tokens (verified via grep against the tests directory before
rewriting).
…on singletons — dime M+L-C4

Cycle-4 leftovers on the JsonSerializerOptions singleton campaign (TrakHound#249).

Fix (M-C4 — F-IMP-001):
Adds MTConnect.Errors.ErrorResponseDocument to the WarmReachableGraph
pass on BOTH JsonFunctions static ctors. ErrorResponseDocument is the
fourth top-level response envelope written directly by
Format(IErrorResponseDocument, ...) in both formatter families — no
Json* surrogate wrapper. Without the addition, the first /probe error,
/current parse failure, or unsupported device request would pay a
cold LCG DynamicMethod emit against the shared, frozen options, which
is the exact hot-path cost the singleton pattern exists to amortize.

Fix (L-C4):
Extends the <remarks> block on DefaultOptions + IndentOptions in both
files with a WHY sentence explaining, in operator-actionable terms,
that mutating a shared JsonSerializerOptions triggers System.Text.Json
to rebuild its metadata cache, which re-emits property accessors as
DynamicMethods into the runtime's LCG loader heaps. Those heaps are
never reclaimed by the GC — a peer measured this at +3.2–3.8 MB/h RSS
in production, which is what the frozen singleton eliminates.

Pin tests:
Adds Frozen_singleton_can_serialize_ErrorResponseDocument_proving_Error_warm_up_ran
to both JsonSerializerOptionsSingletonTests fixtures. Mirrors the
existing SamplePayload warm-up-before-freeze pin — a regression that
dropped ErrorResponseDocument from WarmReachableGraph would surface
here as NotSupportedException on the frozen, resolver-less options.
…ResponseDocument pin with IL-inspection; add sibling structural pins — coverage-FLOOR

The cycle-4 Frozen_singleton_can_serialize_ErrorResponseDocument_proving_Error_warm_up_ran
pin was tautological — verified by mutation on bluefin 2026-08-21: removing
the ErrorResponseDocument warm-up line from WarmReachableGraph left the pin
green. STJ's DefaultJsonTypeInfoResolver lazily populates JsonTypeInfo on
frozen options for arbitrary types once TypeInfoResolver is set (which any
earlier warm-up call does); MakeReadOnly(populateMissingResolver: false)
only locks the configuration surface, not the internal metadata cache.

Rewrite the pin as WarmReachableGraph_IL_contains_newobj_for_ErrorResponseDocument_ctor:
walk the method body's IL for a newobj instruction whose ResolveMethod
resolves to a ctor of the expected type. Mutation-verified: removing the
warm-up line now cleanly fails the pin. Add companion pin covering the
three Json* top-level response surrogates in each fixture.

Also add sibling-site structural pins for the four MTConnect.NET-Common
per-call `new JsonSerializerOptions(...)` sites flipped to shared static
readonly fields in PR TrakHound#249 (MTConnectAgentInformation, MTConnectClientInformation,
MTConnectAssetFileBuffer, AdapterApplicationConfiguration, AgentConfiguration),
plus the MTConnect.NET-MQTT sibling (MTConnectMqttMessage) in the MqttRelay
test project (which transitively references MTConnect.NET-MQTT). Each pin
asserts at least one private static readonly JsonSerializerOptions field
exists on the type — sibling-mutation-verified on MTConnectAgentInformation.

Net +8 pins across four projects; all fixtures green on bluefin
(net8.0, 4,530 passing).
…Document null-tolerance — dime M+L-C5

Cycle-5 leftovers on the JsonSerializerOptions singleton campaign (TrakHound#249).

Fix (M-C5 — F-IMP-C5-001):
Populates the WarmReachableGraph Error envelope with a concrete
MTConnectErrorHeader + a single Error entry + a Version(2, 5), so
STJ walks the RUNTIME types the production /probe-error and
/parse-failure paths actually serialize. STJ resolves accessors for
interface-typed properties (IMTConnectErrorHeader Header,
IEnumerable<IError> Errors) only when the value is non-null; the
prior naked `new ErrorResponseDocument()` warmed the envelope's own
accessors but left the concrete MTConnectErrorHeader (9 properties),
Error (2), and System.Version (6) cold — exactly the LCG-emit class
the singleton pattern exists to eliminate. Populating a
representative graph completes the fix.

Fix (L-C5 — F-IMP-C5-002):
Documents the null-tolerance contract on
`JsonAssetsDocument(IAssetsResponseDocument)` in a <remarks> block.
The plain-JSON WarmReachableGraph calls
`new JsonAssetsDocument(null)` because the surrogate has no public
parameterless ctor. If a future refactor added
`ArgumentNullException.ThrowIfNull(assetsDocument)` there, first
assembly load would fail with TypeInitializationException on any
JSON serialization. The <remarks> names the coupling and directs
future contributors to update the warm-up site atomically, in the
same commit, before changing the ctor contract.

Cycle-5 dispositions:
- code-review: NO FINDINGS
- security-audit: NO FINDINGS
- documentation-audit: NO FINDINGS
- simplification: F-SIMP-001 LOW (dup first-para on IndentOptions
  remarks) — Closed-with-rationale: IntelliSense hover shows only the
  current member's remarks and does not chase <see cref/>; a consumer
  inspecting IndentOptions in isolation must see the invariant text
  self-contained, not as a pointer.
- improvement: F-IMP-C5-001 MEDIUM (this commit); F-IMP-C5-002 LOW
  (this commit).
- test-coverage-audit: 2 findings resolved atomically in 5a19658
  (tautological Error-warm-up pin replaced with IL-inspection walker;
  sibling structural pins added for the five per-call → static-readonly
  refactors).
…rage-FLOOR

Cycle-6 gap: the F-IMP-C5-001 fix (6b081ea) populated the Error
envelope in WarmReachableGraph with a concrete MTConnectErrorHeader
+ Errors.Error + Version(2, 5) so STJ walks the runtime types the
production /probe error and parse-failure paths actually serialize.
The cycle-5 IL walker only pinned newobj ErrorResponseDocument, so
a revert to `new ErrorResponseDocument()` (unpopulated) would
silently pass — re-opening the exact LCG-emit-on-first-error class
F-IMP-C5-001 closed. Adds a per-fixture pin
WarmReachableGraph_IL_contains_newobj_for_concrete_Error_envelope_fields
that asserts newobj instructions for the three concrete types.
Mutation-verified on bluefin 2026-08-21: reverting the Error init
to `new ErrorResponseDocument()` fires the new pin (MTConnectErrorHeader
newobj gone) while the ErrorResponseDocument-only pin still passes.
System.Version is not news-up-ed by the three top-level surrogates,
so a walker match on System.Version uniquely fingerprints the
Error-envelope initializer surviving.

Cycle-6 also verified the existing IL walker still catches deletion
of the whole Error Serialize call: mutation on bluefin confirmed
WarmReachableGraph_IL_contains_newobj_for_ErrorResponseDocument_ctor
fails cleanly after the object-initializer form replaces `new
ErrorResponseDocument()` — the newobj for the envelope ctor is the
first opcode the initializer emits (before dup + property setters).

Test totals bluefin 2026-08-21 (net +2 pins over cycle-5):
- MTConnect.NET-JSON-Tests: 82/82
- MTConnect.NET-JSON-cppagent-Tests: 382/382
- MTConnect.NET-Common-Tests: 4085/4085
- MTConnect.NET-AgentModule-MqttRelay-Tests: 63/63
…ion25 — dime L-C6

Cycle-6 leftover on the JsonSerializerOptions singleton campaign (TrakHound#249).

Fix (L-C6 — F-CR-C6-001):
Replaces the magic `new Version(2, 5)` literal in both
`WarmReachableGraph` Error envelopes with the repo's canonical
`MTConnectVersions.Version25` constant. The value is functionally
identical (same `new Version(2, 5)` allocation under the hood) but
removes a hand-typed tuple in favor of the named constant every
other error-doc construction site in the codebase uses
(`MTConnectAgentBroker.GetErrorHeader` sources Version from
`MTConnectVersion` rather than a literal). Reads scan-and-recognize
instead of raising "why 2.5 in a 2.7-max library" questions.

Removes now-unused `using System;` from the cppagent JsonFunctions.cs
(the only unqualified `Version` reference was the magic literal, now
gone; the property type is resolved through the receiver's declared
type).

Cycle-6 dispositions:
- code-review: F-CR-C6-001 LOW (this commit).
- security-audit: NO FINDINGS.
- simplification: NO FINDINGS (cycle-5 F-SIMP-001 duplication class
  intentionally preserved per prior Closed-with-rationale).
- improvement: NO FINDINGS.
- documentation-audit: NO FINDINGS.
- test-coverage-audit: F-COV-C6-001 MEDIUM (concrete Error-envelope
  field pins) — Fixed atomically in e19674e (IL walker now asserts
  newobj for MTConnectErrorHeader + Error + Version, catching a
  revert-to-unpopulated regression that cycle-5's ErrorResponseDocument-only
  pin missed).
…F-CR-C6-001

The F-CR-C6-001 refactor (ec86e33) switched the Error-envelope
Version producer from `new Version(2, 5)` to
`MTConnectVersions.Version25`. That eliminates the `newobj System.Version`
IL instruction the cycle-6 walker
(`WarmReachableGraph_IL_contains_newobj_for_concrete_Error_envelope_fields`)
required, so the pin would false-positive on the current source.

Widen the walker to accept EITHER `newobj T::.ctor` OR
`ldsfld <static-field-of-type-T>`. Both producers are semantically
equivalent for warm-up: STJ walks the runtime type of the value fed
into Serialize and cannot tell whether the instance came from a fresh
allocation or a static-field load. Companion `<summary>` blocks
updated in both fixtures to name the widened contract and reference
F-CR-C6-001.

Mutation-verified (bluefin 2026-08-21):
- Delete the Error `Serialize(...)` call → pin fails on
  `MTConnectErrorHeader must produce a concrete instance…`.
- Revert to naked `new Errors.ErrorResponseDocument()` → pin fails
  on `MTConnectErrorHeader must produce a concrete instance…`.
- Revert `Version = MTConnectVersions.Version25` to `Version = null`
  → pin fails on `System.Version must produce a concrete instance…`.
- Restore all three → green.

The widened walker still rejects the pathological regression class
(any change that removes both the newobj AND the ldsfld producer of
a target type).
@ottobolyos
ottobolyos force-pushed the fix/json-serializer-options-per-call-leak branch from d0f770e to 6db4057 Compare August 26, 2026 04:53
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 26, 2026
…to integration/up-to-pr-249

Cascade rebuild after upstream/master advanced to 0ddc3c6 (PR TrakHound#220 merge).

# Conflicts:
#	libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 26, 2026
…d#249's new test files

integration-branch-only glue fix, same class as the TrakHound#239/TrakHound#241 fixes
already on this branch. TrakHound#249 builds clean standalone against current
upstream/master without NUnit 4 present.
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 26, 2026
…#261's new test file

integration-branch-only glue fix, same class as the TrakHound#239/TrakHound#241/TrakHound#249
fixes already on this branch. TrakHound#261 builds clean standalone against
current upstream/master without NUnit 4 present.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Ready to Merge

Development

Successfully merging this pull request may close these issues.

2 participants