From f4e8624220ed5fe90e1bb835a66d3f665889553b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 01:06:54 +0200 Subject: [PATCH 01/15] =?UTF-8?q?fix(json,json-cppagent):=20cache=20JsonSe?= =?UTF-8?q?rializerOptions=20singletons=20=E2=80=94=20plug=20DynamicMethod?= =?UTF-8?q?=20LCG=20leak?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 (587a1265): 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. --- .../Agents/MTConnectAgentInformation.cs | 16 ++- .../Buffers/MTConnectAssetFileBuffer.cs | 16 ++- .../Clients/MTConnectClientInformation.cs | 16 ++- .../AdapterApplicationConfiguration.cs | 23 ++-- .../Configurations/AgentConfiguration.cs | 23 ++-- .../JsonFunctions.cs | 124 ++++++++---------- libraries/MTConnect.NET-JSON/JsonFunctions.cs | 122 ++++++++--------- .../MTConnectMqttMessage.cs | 8 +- .../JsonSerializerOptionsSingletonTests.cs | 84 ++++++++++++ .../JsonSerializerOptionsSingletonTests.cs | 75 +++++++++++ 10 files changed, 326 insertions(+), 181 deletions(-) create mode 100644 tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs create mode 100644 tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs diff --git a/libraries/MTConnect.NET-Common/Agents/MTConnectAgentInformation.cs b/libraries/MTConnect.NET-Common/Agents/MTConnectAgentInformation.cs index 1670879c1..a5310a23c 100644 --- a/libraries/MTConnect.NET-Common/Agents/MTConnectAgentInformation.cs +++ b/libraries/MTConnect.NET-Common/Agents/MTConnectAgentInformation.cs @@ -19,6 +19,15 @@ public class MTConnectAgentInformation /// public const string Filename = "agent.information.json"; + // Shared across every Save call. See JsonFunctions.cs for the + // rationale — a fresh JsonSerializerOptions per call re-emits + // LCG DynamicMethods into the loader heap, and the GC cannot + // reclaim them. + private static readonly JsonSerializerOptions _saveOptions = new JsonSerializerOptions + { + WriteIndented = true + }; + /// /// A token regenerated on every save, used to detect when the persisted information has changed. @@ -127,12 +136,7 @@ public void Save(string path = null) try { - var options = new JsonSerializerOptions - { - WriteIndented = true - }; - - var json = JsonSerializer.Serialize(this, options); + var json = JsonSerializer.Serialize(this, _saveOptions); File.WriteAllText(configurationPath, json); } catch { } diff --git a/libraries/MTConnect.NET-Common/Buffers/MTConnectAssetFileBuffer.cs b/libraries/MTConnect.NET-Common/Buffers/MTConnectAssetFileBuffer.cs index b89d1c84e..73f131bfe 100644 --- a/libraries/MTConnect.NET-Common/Buffers/MTConnectAssetFileBuffer.cs +++ b/libraries/MTConnect.NET-Common/Buffers/MTConnectAssetFileBuffer.cs @@ -36,6 +36,15 @@ public class MTConnectAssetFileBuffer : MTConnectAssetBuffer, IDisposable /// public const string DirectoryAssets = "assets"; + // Shared across every asset persistence call. See + // JsonFunctions.cs for the rationale — a fresh + // JsonSerializerOptions per call re-emits LCG DynamicMethods + // into the loader heap, and the GC cannot reclaim them. + private static readonly JsonSerializerOptions _writeOptions = new JsonSerializerOptions + { + WriteIndented = true + }; + private readonly string _basePath; private readonly MTConnectAssetQueue _items; private readonly Regex _regex = new Regex("([0-9]*)_(.*)"); @@ -432,14 +441,9 @@ private async Task WriteToFile(uint index, IAsset asset, uint originalInde } } - var options = new JsonSerializerOptions - { - WriteIndented = true - }; - var assetType = Asset.GetAssetType(asset.Type); - var json = JsonSerializer.Serialize(asset, assetType, options); + var json = JsonSerializer.Serialize(asset, assetType, _writeOptions); if (!string.IsNullOrEmpty(json)) { if (UseCompression) diff --git a/libraries/MTConnect.NET-Common/Clients/MTConnectClientInformation.cs b/libraries/MTConnect.NET-Common/Clients/MTConnectClientInformation.cs index 81bfe9e50..4e8bbad87 100644 --- a/libraries/MTConnect.NET-Common/Clients/MTConnectClientInformation.cs +++ b/libraries/MTConnect.NET-Common/Clients/MTConnectClientInformation.cs @@ -23,6 +23,15 @@ public class MTConnectClientInformation /// public const string FilenameExtension = ".json"; + // Shared across every Save call. See JsonFunctions.cs for the + // rationale — a fresh JsonSerializerOptions per call re-emits + // LCG DynamicMethods into the loader heap, and the GC cannot + // reclaim them. + private static readonly JsonSerializerOptions _saveOptions = new JsonSerializerOptions + { + WriteIndented = true + }; + /// /// A token regenerated on every save, used to detect that the persisted state has changed since it was last loaded. @@ -126,12 +135,7 @@ public void Save(string path = null) var configurationPath = Path.Combine(dir, GenerateFilename(DeviceKey)); if (path != null) configurationPath = path; - var options = new JsonSerializerOptions - { - WriteIndented = true - }; - - var json = JsonSerializer.Serialize(this, options); + var json = JsonSerializer.Serialize(this, _saveOptions); File.WriteAllText(configurationPath, json); } catch { } diff --git a/libraries/MTConnect.NET-Common/Configurations/AdapterApplicationConfiguration.cs b/libraries/MTConnect.NET-Common/Configurations/AdapterApplicationConfiguration.cs index 647873396..5f97a6972 100644 --- a/libraries/MTConnect.NET-Common/Configurations/AdapterApplicationConfiguration.cs +++ b/libraries/MTConnect.NET-Common/Configurations/AdapterApplicationConfiguration.cs @@ -40,6 +40,15 @@ public class AdapterApplicationConfiguration : IAdapterApplicationConfiguration /// public const string DefaultYamlFilename = "adapter.config.default.yaml"; + // Shared across every ReadJson call. See JsonFunctions.cs for + // the rationale — a fresh JsonSerializerOptions per call + // re-emits LCG DynamicMethods into the loader heap, and the GC + // cannot reclaim them. + private static readonly JsonSerializerOptions _readOptions = new JsonSerializerOptions() + { + ReadCommentHandling = JsonCommentHandling.Skip + }; + /// /// An opaque token regenerated each time the configuration is saved, allowing consumers to detect that the configuration has changed. @@ -371,12 +380,7 @@ public static T ReadJson(string path = null) where T : AdapterApplicationConf var text = File.ReadAllText(configurationPath); if (!string.IsNullOrEmpty(text)) { - var options = new JsonSerializerOptions() - { - ReadCommentHandling = JsonCommentHandling.Skip - }; - - var configuration = JsonSerializer.Deserialize(text, options); + var configuration = JsonSerializer.Deserialize(text, _readOptions); configuration.Path = configurationPath; return configuration; } @@ -411,12 +415,7 @@ public static AdapterApplicationConfiguration ReadJson(Type type, string path = var text = File.ReadAllText(configurationPath); if (!string.IsNullOrEmpty(text)) { - var options = new JsonSerializerOptions() - { - ReadCommentHandling = JsonCommentHandling.Skip - }; - - var configuration = (AdapterApplicationConfiguration)JsonSerializer.Deserialize(text, type, options); + var configuration = (AdapterApplicationConfiguration)JsonSerializer.Deserialize(text, type, _readOptions); configuration.Path = configurationPath; return configuration; } diff --git a/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs b/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs index fd873c6c7..ce979aeac 100644 --- a/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs +++ b/libraries/MTConnect.NET-Common/Configurations/AgentConfiguration.cs @@ -40,6 +40,15 @@ public class AgentConfiguration : IAgentConfiguration /// public const string DefaultYamlFilename = "agent.config.default.yaml"; + // Shared across every ReadJson call. See JsonFunctions.cs for + // the rationale — a fresh JsonSerializerOptions per call + // re-emits LCG DynamicMethods into the loader heap, and the GC + // cannot reclaim them. + private static readonly JsonSerializerOptions _readOptions = new JsonSerializerOptions() + { + ReadCommentHandling = JsonCommentHandling.Skip + }; + /// /// An opaque token regenerated each time the configuration is saved, allowing consumers to detect that the configuration has changed. @@ -277,12 +286,7 @@ public static T ReadJson(string path = null) where T : AgentConfiguration var text = File.ReadAllText(configurationPath); if (!string.IsNullOrEmpty(text)) { - var options = new JsonSerializerOptions() - { - ReadCommentHandling = JsonCommentHandling.Skip - }; - - var configuration = JsonSerializer.Deserialize(text, options); + var configuration = JsonSerializer.Deserialize(text, _readOptions); configuration.Path = configurationPath; return configuration; } @@ -317,12 +321,7 @@ public static AgentConfiguration ReadJson(Type type, string path = null) var text = File.ReadAllText(configurationPath); if (!string.IsNullOrEmpty(text)) { - var options = new JsonSerializerOptions() - { - ReadCommentHandling = JsonCommentHandling.Skip - }; - - var configuration = (AgentConfiguration)JsonSerializer.Deserialize(text, type, options); + var configuration = (AgentConfiguration)JsonSerializer.Deserialize(text, type, _readOptions); configuration.Path = configurationPath; return configuration; } diff --git a/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs b/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs index 046fd4f47..4ea77bbb8 100644 --- a/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs +++ b/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs @@ -18,50 +18,69 @@ namespace MTConnect /// public static class JsonFunctions { + // A JsonSerializerOptions instance owns its own serialization + // metadata cache, and building that cache emits reflection-based + // property accessors (LCG DynamicMethods) for every property in + // the reachable type graph. Allocating a fresh instance 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 — this is the mechanism behind + // the ~3.3 MB/h RSS climb observed on DIME production hosts in + // 2026-08. The cppagent-format assembly ships 25 Streams.Json + // classes (2.5× the plain-JSON count), so the LCG cost per + // serialisation is proportionally larger. The instances below + // are created once and reused; JsonSerializerOptions is + // thread-safe for read after its first (de)serialization, and + // nothing in this file mutates them after construction. + private static readonly JsonSerializerOptions _defaultOptions = CreateOptions(false); + private static readonly JsonSerializerOptions _indentOptions = CreateOptions(true); + + private static JsonSerializerOptions CreateOptions(bool indented) + { + return new JsonSerializerOptions + { + WriteIndented = indented, +#if NET5_0_OR_GREATER + DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingDefault, + NumberHandling = JsonNumberHandling.AllowReadingFromString, +#endif + PropertyNameCaseInsensitive = true, + MaxDepth = 1000 + }; + } + + private static JsonSerializerOptions GetOptions(JsonConverter converter, bool indented) + { + // Hot path: no per-call converter, hand back the shared + // instance and let System.Text.Json reuse its metadata cache + // instead of re-emitting property accessors for the whole + // type graph on every call. + if (converter == null) return indented ? _indentOptions : _defaultOptions; + + // Cold path: a caller-supplied converter cannot be added to + // a shared instance once it has been used, so build a + // private one. Callers that mint many one-off converters + // will still pay the LCG cost — this fallback preserves the + // public API contract for external consumers. + var options = CreateOptions(indented); + options.Converters.Add(converter); + return options; + } + /// /// Default serializer options used when no indentOutput /// option is requested. Produces compact JSON, omits properties /// at their default value, allows numbers to be read from /// strings, and ignores property-name casing. /// - public static JsonSerializerOptions DefaultOptions - { - get - { - return new JsonSerializerOptions - { - WriteIndented = false, -#if NET5_0_OR_GREATER - DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingDefault, - NumberHandling = JsonNumberHandling.AllowReadingFromString, -#endif - PropertyNameCaseInsensitive = true, - MaxDepth = 1000 - }; - } - } + public static JsonSerializerOptions DefaultOptions => _defaultOptions; /// /// Pretty-printed serializer options used when the /// indentOutput formatter option is enabled; otherwise /// identical to . /// - public static JsonSerializerOptions IndentOptions - { - get - { - return new JsonSerializerOptions - { - WriteIndented = true, -#if NET5_0_OR_GREATER - DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingDefault, - NumberHandling = JsonNumberHandling.AllowReadingFromString, -#endif - PropertyNameCaseInsensitive = true, - MaxDepth = 1000 - }; - } - } + public static JsonSerializerOptions IndentOptions => _indentOptions; /// @@ -76,18 +95,7 @@ public static string Convert(object obj, JsonConverter converter = null, bool in { try { - var options = new JsonSerializerOptions - { - WriteIndented = indented, -#if NET5_0_OR_GREATER - DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingDefault, - NumberHandling = JsonNumberHandling.AllowReadingFromString, -#endif - PropertyNameCaseInsensitive = true, - MaxDepth = 1000 - }; - - if (converter != null) options.Converters.Add(converter); + var options = GetOptions(converter, indented); return JsonSerializer.Serialize(obj, options); } @@ -109,18 +117,7 @@ public static byte[] ConvertBytes(object obj, JsonConverter converter = null, bo { try { - var options = new JsonSerializerOptions - { - WriteIndented = indented, -#if NET5_0_OR_GREATER - DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingDefault, - NumberHandling = JsonNumberHandling.AllowReadingFromString, -#endif - PropertyNameCaseInsensitive = true, - MaxDepth = 1000 - }; - - if (converter != null) options.Converters.Add(converter); + var options = GetOptions(converter, indented); return JsonSerializer.SerializeToUtf8Bytes(obj, options); } @@ -143,18 +140,7 @@ public static Stream ConvertStream(object obj, JsonConverter converter = null, b { try { - var options = new JsonSerializerOptions - { - WriteIndented = indented, -#if NET5_0_OR_GREATER - DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingDefault, - NumberHandling = JsonNumberHandling.AllowReadingFromString, -#endif - PropertyNameCaseInsensitive = true, - MaxDepth = 1000 - }; - - if (converter != null) options.Converters.Add(converter); + var options = GetOptions(converter, indented); var outputStream = new MemoryStream(); JsonSerializer.Serialize(outputStream, obj, options); @@ -166,4 +152,4 @@ public static Stream ConvertStream(object obj, JsonConverter converter = null, b return null; } } -} \ No newline at end of file +} diff --git a/libraries/MTConnect.NET-JSON/JsonFunctions.cs b/libraries/MTConnect.NET-JSON/JsonFunctions.cs index 908940911..a9be64b4c 100644 --- a/libraries/MTConnect.NET-JSON/JsonFunctions.cs +++ b/libraries/MTConnect.NET-JSON/JsonFunctions.cs @@ -16,49 +16,66 @@ namespace MTConnect /// public static class JsonFunctions { + // A JsonSerializerOptions instance owns its own serialization + // metadata cache, and building that cache emits reflection-based + // property accessors (LCG DynamicMethods) for every property in + // the reachable type graph. Allocating a fresh instance 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 — this is the mechanism behind + // the ~3.3 MB/h RSS climb observed on DIME production hosts in + // 2026-08. The instances below are created once and reused; + // JsonSerializerOptions is thread-safe for read after its first + // (de)serialization, and nothing in this file mutates them after + // construction. + private static readonly JsonSerializerOptions _defaultOptions = CreateOptions(false); + private static readonly JsonSerializerOptions _indentOptions = CreateOptions(true); + + private static JsonSerializerOptions CreateOptions(bool indented) + { + return new JsonSerializerOptions + { + WriteIndented = indented, +#if NET5_0_OR_GREATER + DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingDefault, + NumberHandling = JsonNumberHandling.AllowReadingFromString, +#endif + PropertyNameCaseInsensitive = true, + MaxDepth = 1000 + }; + } + + private static JsonSerializerOptions GetOptions(JsonConverter converter, bool indented) + { + // Hot path: no per-call converter, hand back the shared + // instance and let System.Text.Json reuse its metadata cache + // instead of re-emitting property accessors for the whole + // type graph on every call. + if (converter == null) return indented ? _indentOptions : _defaultOptions; + + // Cold path: a caller-supplied converter cannot be added to + // a shared instance once it has been used, so build a + // private one. Callers that mint many one-off converters + // will still pay the LCG cost — this fallback preserves the + // public API contract for external consumers. + var options = CreateOptions(indented); + options.Converters.Add(converter); + return options; + } + /// /// The default used by MTConnect /// JSON serialization: compact output, default-valued properties /// omitted on write (on net5+), numbers read from strings (on net5+), /// case-insensitive property names, and a depth limit of 1000. /// - public static JsonSerializerOptions DefaultOptions - { - get - { - return new JsonSerializerOptions - { - WriteIndented = false, -#if NET5_0_OR_GREATER - DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingDefault, - NumberHandling = JsonNumberHandling.AllowReadingFromString, -#endif - PropertyNameCaseInsensitive = true, - MaxDepth = 1000 - }; - } - } + public static JsonSerializerOptions DefaultOptions => _defaultOptions; /// /// The indented variant of , used when the /// formatter's indentOutput option is set. /// - public static JsonSerializerOptions IndentOptions - { - get - { - return new JsonSerializerOptions - { - WriteIndented = true, -#if NET5_0_OR_GREATER - DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingDefault, - NumberHandling = JsonNumberHandling.AllowReadingFromString, -#endif - PropertyNameCaseInsensitive = true, - MaxDepth = 1000 - }; - } - } + public static JsonSerializerOptions IndentOptions => _indentOptions; /// @@ -97,18 +114,7 @@ public static string Convert(object obj, JsonConverter converter = null, bool in { try { - var options = new JsonSerializerOptions - { - WriteIndented = indented, -#if NET5_0_OR_GREATER - DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingDefault, - NumberHandling = JsonNumberHandling.AllowReadingFromString, -#endif - PropertyNameCaseInsensitive = true, - MaxDepth = 1000 - }; - - if (converter != null) options.Converters.Add(converter); + var options = GetOptions(converter, indented); return JsonSerializer.Serialize(obj, options); } @@ -130,18 +136,7 @@ public static byte[] ConvertBytes(object obj, JsonConverter converter = null, bo { try { - var options = new JsonSerializerOptions - { - WriteIndented = indented, -#if NET5_0_OR_GREATER - DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingDefault, - NumberHandling = JsonNumberHandling.AllowReadingFromString, -#endif - PropertyNameCaseInsensitive = true, - MaxDepth = 1000 - }; - - if (converter != null) options.Converters.Add(converter); + var options = GetOptions(converter, indented); return JsonSerializer.SerializeToUtf8Bytes(obj, options); } @@ -163,18 +158,7 @@ public static Stream ConvertStream(object obj, JsonConverter converter = null, b { try { - var options = new JsonSerializerOptions - { - WriteIndented = indented, -#if NET5_0_OR_GREATER - DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingDefault, - NumberHandling = JsonNumberHandling.AllowReadingFromString, -#endif - PropertyNameCaseInsensitive = true, - MaxDepth = 1000 - }; - - if (converter != null) options.Converters.Add(converter); + var options = GetOptions(converter, indented); var outputStream = new MemoryStream(); JsonSerializer.Serialize(outputStream, obj, options); @@ -186,4 +170,4 @@ public static Stream ConvertStream(object obj, JsonConverter converter = null, b return null; } } -} \ No newline at end of file +} diff --git a/libraries/MTConnect.NET-MQTT/MTConnectMqttMessage.cs b/libraries/MTConnect.NET-MQTT/MTConnectMqttMessage.cs index feb833e98..9007412ba 100644 --- a/libraries/MTConnect.NET-MQTT/MTConnectMqttMessage.cs +++ b/libraries/MTConnect.NET-MQTT/MTConnectMqttMessage.cs @@ -23,6 +23,12 @@ namespace MTConnect.Mqtt /// public static class MTConnectMqttMessage { + // Shared across every agent-information publish. See + // JsonFunctions.cs for the rationale — a fresh + // JsonSerializerOptions per call re-emits LCG DynamicMethods + // into the loader heap, and the GC cannot reclaim them. + private static readonly JsonSerializerOptions _agentInformationOptions = new JsonSerializerOptions { WriteIndented = true }; + private static MqttApplicationMessage CreateMessage(string topic, string payload, bool retain = false) { try @@ -91,7 +97,7 @@ public static IEnumerable Create(IMTConnectAgent agent, } var topic = $"MTConnect/Agents/{agent.Uuid}/Information"; - var json = JsonSerializer.Serialize(information, new JsonSerializerOptions { WriteIndented = true }); + var json = JsonSerializer.Serialize(information, _agentInformationOptions); messages.Add(CreateMessage(topic, json, retain)); } diff --git a/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs b/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs new file mode 100644 index 000000000..eb18c48a3 --- /dev/null +++ b/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs @@ -0,0 +1,84 @@ +// Copyright (c) 2026 TrakHound Inc., All Rights Reserved. +// TrakHound Inc. licenses this file to you under the MIT license. + +using System.Reflection; +using System.Text.Json; +using NUnit.Framework; + +namespace MTConnect.NET_JSON_Tests.Regressions +{ + /// + /// Regression pin for the DIME-connector native-heap leak (peer + /// diagnosis dated 2026-08-21). A fresh + /// instance owns its own serialisation-metadata cache; building that + /// cache emits reflection-based property accessors for the entire type + /// graph, and the emitted code accumulates in the runtime's loader + /// heaps where the GC cannot reclaim it. Allocating one on every + /// serialisation call therefore leaked ~3.3 MB/h RSS in production + /// (tempco-001, tim-001). + /// + /// The and + /// properties must therefore + /// return the same instance on every access, and the + /// Convert/ConvertBytes/ConvertStream hot paths + /// must reuse those instances when no per-call converter is supplied. + /// + [TestFixture] + public class JsonSerializerOptionsSingletonTests + { + /// Pins the behaviour expressed by the test name: default options returns the same instance on repeat access. + [Test] + public void DefaultOptions_returns_the_same_instance_on_repeat_access() + { + var a = JsonFunctions.DefaultOptions; + var b = JsonFunctions.DefaultOptions; + Assert.That(ReferenceEquals(a, b), Is.True, + "JsonFunctions.DefaultOptions must be a shared singleton — a fresh JsonSerializerOptions per call leaks LCG-emitted metadata into the loader heap."); + } + + /// Pins the behaviour expressed by the test name: indent options returns the same instance on repeat access. + [Test] + public void IndentOptions_returns_the_same_instance_on_repeat_access() + { + var a = JsonFunctions.IndentOptions; + var b = JsonFunctions.IndentOptions; + Assert.That(ReferenceEquals(a, b), Is.True, + "JsonFunctions.IndentOptions must be a shared singleton — a fresh JsonSerializerOptions per call leaks LCG-emitted metadata into the loader heap."); + } + + /// Pins the behaviour expressed by the test name: default options and indent options are distinct instances. + [Test] + public void DefaultOptions_and_IndentOptions_are_distinct_instances() + { + Assert.That(ReferenceEquals(JsonFunctions.DefaultOptions, JsonFunctions.IndentOptions), Is.False, + "DefaultOptions (compact) and IndentOptions (pretty-printed) must be separate instances so WriteIndented differs."); + } + + /// Pins the behaviour expressed by the test name: default options write indented is false. + [Test] + public void DefaultOptions_WriteIndented_is_false() + { + Assert.That(JsonFunctions.DefaultOptions.WriteIndented, Is.False); + } + + /// Pins the behaviour expressed by the test name: indent options write indented is true. + [Test] + public void IndentOptions_WriteIndented_is_true() + { + Assert.That(JsonFunctions.IndentOptions.WriteIndented, Is.True); + } + + /// Pins the behaviour expressed by the test name: json functions holds a static readonly options field. + [Test] + public void JsonFunctions_holds_a_static_readonly_options_field() + { + // Structural guard: at least one static readonly field of type + // JsonSerializerOptions must exist on JsonFunctions so the + // shared instance survives across serialisation calls. + var fields = typeof(JsonFunctions).GetFields(BindingFlags.NonPublic | BindingFlags.Static); + var optionsFields = System.Array.FindAll(fields, f => f.FieldType == typeof(JsonSerializerOptions) && f.IsInitOnly); + Assert.That(optionsFields.Length, Is.GreaterThanOrEqualTo(2), + "JsonFunctions must declare shared static readonly JsonSerializerOptions fields (compact + indented) so the instances outlive each serialisation call."); + } + } +} diff --git a/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs b/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs new file mode 100644 index 000000000..4b26c5661 --- /dev/null +++ b/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs @@ -0,0 +1,75 @@ +// Copyright (c) 2026 TrakHound Inc., All Rights Reserved. +// TrakHound Inc. licenses this file to you under the MIT license. + +using System.Reflection; +using System.Text.Json; +using NUnit.Framework; + +namespace MTConnect.NET_JSON_cppagent_Tests.Regressions +{ + /// + /// Regression pin for the DIME-connector native-heap leak (peer + /// diagnosis dated 2026-08-21). This is the cppagent-flavoured + /// mirror of the same guard applied to + /// MTConnect.NET-JSON/JsonFunctions.cs. The two files ship + /// independent copies of the same option-preset surface, so both + /// must singleton their to keep + /// the runtime's loader heap from accumulating LCG-emitted property + /// accessors on every serialisation call. + /// + [TestFixture] + public class JsonSerializerOptionsSingletonTests + { + /// Pins the behaviour expressed by the test name: default options returns the same instance on repeat access. + [Test] + public void DefaultOptions_returns_the_same_instance_on_repeat_access() + { + var a = JsonFunctions.DefaultOptions; + var b = JsonFunctions.DefaultOptions; + Assert.That(ReferenceEquals(a, b), Is.True, + "JsonFunctions.DefaultOptions must be a shared singleton — a fresh JsonSerializerOptions per call leaks LCG-emitted metadata into the loader heap."); + } + + /// Pins the behaviour expressed by the test name: indent options returns the same instance on repeat access. + [Test] + public void IndentOptions_returns_the_same_instance_on_repeat_access() + { + var a = JsonFunctions.IndentOptions; + var b = JsonFunctions.IndentOptions; + Assert.That(ReferenceEquals(a, b), Is.True, + "JsonFunctions.IndentOptions must be a shared singleton — a fresh JsonSerializerOptions per call leaks LCG-emitted metadata into the loader heap."); + } + + /// Pins the behaviour expressed by the test name: default options and indent options are distinct instances. + [Test] + public void DefaultOptions_and_IndentOptions_are_distinct_instances() + { + Assert.That(ReferenceEquals(JsonFunctions.DefaultOptions, JsonFunctions.IndentOptions), Is.False, + "DefaultOptions (compact) and IndentOptions (pretty-printed) must be separate instances so WriteIndented differs."); + } + + /// Pins the behaviour expressed by the test name: default options write indented is false. + [Test] + public void DefaultOptions_WriteIndented_is_false() + { + Assert.That(JsonFunctions.DefaultOptions.WriteIndented, Is.False); + } + + /// Pins the behaviour expressed by the test name: indent options write indented is true. + [Test] + public void IndentOptions_WriteIndented_is_true() + { + Assert.That(JsonFunctions.IndentOptions.WriteIndented, Is.True); + } + + /// Pins the behaviour expressed by the test name: json functions holds a static readonly options field. + [Test] + public void JsonFunctions_holds_a_static_readonly_options_field() + { + var fields = typeof(JsonFunctions).GetFields(BindingFlags.NonPublic | BindingFlags.Static); + var optionsFields = System.Array.FindAll(fields, f => f.FieldType == typeof(JsonSerializerOptions) && f.IsInitOnly); + Assert.That(optionsFields.Length, Is.GreaterThanOrEqualTo(2), + "JsonFunctions must declare shared static readonly JsonSerializerOptions fields (compact + indented) so the instances outlive each serialisation call."); + } + } +} From 3dd320daff1efdb6c5dbc32c90f8e33e9ec479be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 07:44:15 +0200 Subject: [PATCH 02/15] =?UTF-8?q?test(json,json-cppagent):=20pin=20thread-?= =?UTF-8?q?safety=20+=20cold-path=20converter=20+=20Convert=20overload=20p?= =?UTF-8?q?arity=20=E2=80=94=20coverage-FLOOR?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../JsonSerializerOptionsSingletonTests.cs | 266 ++++++++++++++++++ .../JsonSerializerOptionsSingletonTests.cs | 238 ++++++++++++++++ 2 files changed, 504 insertions(+) diff --git a/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs b/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs index eb18c48a3..684d968b5 100644 --- a/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs +++ b/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs @@ -1,8 +1,13 @@ // Copyright (c) 2026 TrakHound Inc., All Rights Reserved. // TrakHound Inc. licenses this file to you under the MIT license. +using System.Collections.Concurrent; using System.Reflection; +using System.Text; using System.Text.Json; +using System.Text.Json.Serialization; +using System.Threading; +using System.Threading.Tasks; using NUnit.Framework; namespace MTConnect.NET_JSON_Tests.Regressions @@ -26,6 +31,38 @@ namespace MTConnect.NET_JSON_Tests.Regressions [TestFixture] public class JsonSerializerOptionsSingletonTests { + // Small POCO used by the Convert/ConvertBytes/ConvertStream overload + // smoke tests and the thread-safety pin. Kept tiny on purpose so + // the metadata cache warms in a single JIT pass and each thread's + // work is bounded, but with a nested type so the reachable-graph + // JIT emit still runs on cold DefaultOptions instances. + internal class SamplePayload + { + public string Name { get; set; } = "sample"; + public int Count { get; set; } = 42; + public SampleChild Child { get; set; } = new SampleChild(); + } + + internal class SampleChild + { + public string Label { get; set; } = "child"; + public double Value { get; set; } = 3.14; + } + + private sealed class NoopConverter : JsonConverter + { + public override SamplePayload Read(ref Utf8JsonReader reader, System.Type typeToConvert, JsonSerializerOptions options) + => new SamplePayload(); + + public override void Write(Utf8JsonWriter writer, SamplePayload value, JsonSerializerOptions options) + { + writer.WriteStartObject(); + writer.WriteString("Name", value.Name); + writer.WriteNumber("Count", value.Count); + writer.WriteEndObject(); + } + } + /// Pins the behaviour expressed by the test name: default options returns the same instance on repeat access. [Test] public void DefaultOptions_returns_the_same_instance_on_repeat_access() @@ -80,5 +117,234 @@ public void JsonFunctions_holds_a_static_readonly_options_field() Assert.That(optionsFields.Length, Is.GreaterThanOrEqualTo(2), "JsonFunctions must declare shared static readonly JsonSerializerOptions fields (compact + indented) so the instances outlive each serialisation call."); } + + /// + /// Thread-safety pin: 100 concurrent Convert calls against the shared + /// DefaultOptions must all succeed, must not throw, and must produce + /// byte-identical output. JsonSerializerOptions is documented as + /// thread-safe for read after its first (de)serialization call, so + /// concurrent Serialize calls sharing one instance is legal by + /// contract — this test pins that contract against any future + /// refactor that would put a mutation on the hot path. + /// + [Test] + public void Convert_is_thread_safe_across_100_concurrent_callers() + { + const int threadCount = 100; + var payload = new SamplePayload(); + var outputs = new ConcurrentBag(); + var exceptions = new ConcurrentBag(); + var startGate = new ManualResetEventSlim(false); + + var threads = new Thread[threadCount]; + for (int i = 0; i < threadCount; i++) + { + threads[i] = new Thread(() => + { + try + { + startGate.Wait(); + var s = JsonFunctions.Convert(payload); + outputs.Add(s); + } + catch (System.Exception ex) + { + exceptions.Add(ex); + } + }); + threads[i].Start(); + } + + // Warm the singleton once on the driver thread so the first-use + // metadata build is not itself concurrent with the race window. + // The point of the pin is steady-state hot-path concurrency, not + // first-touch racing (STJ handles that internally). + _ = JsonFunctions.Convert(payload); + + startGate.Set(); + foreach (var t in threads) t.Join(); + + Assert.That(exceptions, Is.Empty, + "Concurrent Convert calls against the shared DefaultOptions must not throw."); + Assert.That(outputs.Count, Is.EqualTo(threadCount)); + + var canonical = JsonFunctions.Convert(payload); + foreach (var s in outputs) + { + Assert.That(s, Is.EqualTo(canonical), + "Every concurrent Convert output must be byte-identical to the single-threaded canonical output."); + } + } + + /// + /// Cold-path pin: when a caller supplies a per-call converter, the + /// implementation must NOT append that converter to the shared + /// DefaultOptions.Converters collection. Doing so would (a) permanently + /// pollute the singleton and (b) throw InvalidOperationException + /// because JsonSerializerOptions freezes its converter list after + /// first use. The pin observes the singleton before and after a + /// converter-supplied Convert call and asserts the count is unchanged. + /// + [Test] + public void Convert_with_custom_converter_does_not_mutate_DefaultOptions_converters() + { + // Warm the singleton so its converters list is frozen — if the + // implementation ever tried to append to it, the test would + // observe an exception path rather than silent pollution. + _ = JsonFunctions.Convert(new SamplePayload()); + + var beforeCount = JsonFunctions.DefaultOptions.Converters.Count; + var beforeIndentCount = JsonFunctions.IndentOptions.Converters.Count; + + var converter = new NoopConverter(); + var result = JsonFunctions.Convert(new SamplePayload(), converter); + var resultIndented = JsonFunctions.Convert(new SamplePayload(), converter, indented: true); + + Assert.That(result, Is.Not.Null, + "Convert with a custom converter must succeed via the cold-path fresh-options branch."); + Assert.That(resultIndented, Is.Not.Null, + "Convert(indented:true) with a custom converter must succeed via the cold-path fresh-options branch."); + + Assert.That(JsonFunctions.DefaultOptions.Converters.Count, Is.EqualTo(beforeCount), + "The cold-path branch must build a fresh JsonSerializerOptions — a caller-supplied converter must never leak into DefaultOptions.Converters."); + Assert.That(JsonFunctions.IndentOptions.Converters.Count, Is.EqualTo(beforeIndentCount), + "The cold-path branch must build a fresh JsonSerializerOptions — a caller-supplied converter must never leak into IndentOptions.Converters."); + } + + /// + /// Cold-path pin: the custom-converter Convert output must reflect + /// the caller's converter (proving the fresh options object used + /// it), while a following converter-less Convert on the same input + /// must NOT reflect the converter (proving the singleton was untouched). + /// + [Test] + public void Convert_with_custom_converter_uses_converter_without_affecting_singleton_output() + { + var payload = new SamplePayload(); + var canonicalWithoutConverter = JsonFunctions.Convert(payload); + + var converter = new NoopConverter(); + var withConverter = JsonFunctions.Convert(payload, converter); + + // The custom converter drops the Child property, so its output + // must not equal the canonical singleton-emitted output. + Assert.That(withConverter, Is.Not.EqualTo(canonicalWithoutConverter), + "The cold-path fresh options must apply the caller's converter — otherwise the singleton was reused, defeating the branch."); + Assert.That(withConverter, Does.Not.Contain("Child"), + "NoopConverter drops the Child field; the cold-path output should reflect that."); + + // Following singleton-path call must be untouched. + var afterCanonical = JsonFunctions.Convert(payload); + Assert.That(afterCanonical, Is.EqualTo(canonicalWithoutConverter), + "After a converter-supplied call, the singleton-path output must be byte-identical to before."); + Assert.That(afterCanonical, Does.Contain("Child"), + "The singleton must still emit the Child field — the cold-path converter must not have polluted it."); + } + + /// + /// Overload smoke pin: Convert/ConvertBytes/ConvertStream must all + /// produce the same JSON for the same input under the compact + /// preset, and the indented variants must be internally consistent. + /// This guards against a future refactor that accidentally routes + /// one overload through a divergent options instance. + /// + [Test] + public void Convert_ConvertBytes_ConvertStream_produce_identical_output_for_compact() + { + var payload = new SamplePayload(); + + var s = JsonFunctions.Convert(payload); + var bytes = JsonFunctions.ConvertBytes(payload); + var stream = JsonFunctions.ConvertStream(payload); + + Assert.That(s, Is.Not.Null); + Assert.That(bytes, Is.Not.Null); + Assert.That(stream, Is.Not.Null); + + var bytesString = Encoding.UTF8.GetString(bytes!); + Assert.That(bytesString, Is.EqualTo(s), + "ConvertBytes(UTF-8) must decode to the same string ConvertBytes emits."); + + stream!.Position = 0; + using var reader = new System.IO.StreamReader(stream); + var streamString = reader.ReadToEnd(); + Assert.That(streamString, Is.EqualTo(s), + "ConvertStream must contain the same JSON Convert returns."); + } + + /// Overload smoke pin — indented variant of the same guard. + [Test] + public void Convert_ConvertBytes_ConvertStream_produce_identical_output_for_indented() + { + var payload = new SamplePayload(); + + var s = JsonFunctions.Convert(payload, indented: true); + var bytes = JsonFunctions.ConvertBytes(payload, indented: true); + var stream = JsonFunctions.ConvertStream(payload, indented: true); + + Assert.That(s, Is.Not.Null); + Assert.That(bytes, Is.Not.Null); + Assert.That(stream, Is.Not.Null); + + var bytesString = Encoding.UTF8.GetString(bytes!); + Assert.That(bytesString, Is.EqualTo(s)); + + stream!.Position = 0; + using var reader = new System.IO.StreamReader(stream); + var streamString = reader.ReadToEnd(); + Assert.That(streamString, Is.EqualTo(s)); + + // Cross-check indentation is present (crude, but sufficient + // to pin that IndentOptions actually took effect on all + // three overloads). + Assert.That(s, Does.Contain("\n"), + "Indented Convert output must contain newlines."); + Assert.That(bytesString, Does.Contain("\n"), + "Indented ConvertBytes output must contain newlines."); + Assert.That(streamString, Does.Contain("\n"), + "Indented ConvertStream output must contain newlines."); + } + + /// + /// Null-input pin: every overload must swallow a null input and + /// return null / null / null — this is documented behaviour and + /// the singleton refactor must preserve it. + /// + [Test] + public void Convert_overloads_return_null_for_null_input() + { + Assert.That(JsonFunctions.Convert(null), Is.Null); + Assert.That(JsonFunctions.ConvertBytes(null), Is.Null); + Assert.That(JsonFunctions.ConvertStream(null), Is.Null); + } + + /// + /// Async-throughput pin: 100 parallel Convert calls via + /// Parallel.For to complement the Thread-based pin above. The + /// ThreadPool scheduling differs from raw Thread scheduling, and + /// System.Text.Json's internal metadata-lock behaviour has + /// historically been sensitive to the difference; both paths are + /// pinned so future STJ upgrades are covered. + /// + [Test] + public void Convert_is_thread_safe_under_ParallelFor() + { + _ = JsonFunctions.Convert(new SamplePayload()); // warm + + var canonical = JsonFunctions.Convert(new SamplePayload()); + var results = new ConcurrentBag(); + + Parallel.For(0, 100, _ => + { + var s = JsonFunctions.Convert(new SamplePayload()); + results.Add(s); + }); + + Assert.That(results.Count, Is.EqualTo(100)); + foreach (var r in results) + { + Assert.That(r, Is.EqualTo(canonical)); + } + } } } diff --git a/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs b/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs index 4b26c5661..f463420c8 100644 --- a/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs +++ b/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs @@ -1,8 +1,13 @@ // Copyright (c) 2026 TrakHound Inc., All Rights Reserved. // TrakHound Inc. licenses this file to you under the MIT license. +using System.Collections.Concurrent; using System.Reflection; +using System.Text; using System.Text.Json; +using System.Text.Json.Serialization; +using System.Threading; +using System.Threading.Tasks; using NUnit.Framework; namespace MTConnect.NET_JSON_cppagent_Tests.Regressions @@ -20,6 +25,36 @@ namespace MTConnect.NET_JSON_cppagent_Tests.Regressions [TestFixture] public class JsonSerializerOptionsSingletonTests { + // Small POCO used by the Convert/ConvertBytes/ConvertStream overload + // smoke tests and the thread-safety pin. Nested type so the + // reachable-graph JIT emit still runs on cold options instances. + internal class SamplePayload + { + public string Name { get; set; } = "sample"; + public int Count { get; set; } = 42; + public SampleChild Child { get; set; } = new SampleChild(); + } + + internal class SampleChild + { + public string Label { get; set; } = "child"; + public double Value { get; set; } = 3.14; + } + + private sealed class NoopConverter : JsonConverter + { + public override SamplePayload Read(ref Utf8JsonReader reader, System.Type typeToConvert, JsonSerializerOptions options) + => new SamplePayload(); + + public override void Write(Utf8JsonWriter writer, SamplePayload value, JsonSerializerOptions options) + { + writer.WriteStartObject(); + writer.WriteString("Name", value.Name); + writer.WriteNumber("Count", value.Count); + writer.WriteEndObject(); + } + } + /// Pins the behaviour expressed by the test name: default options returns the same instance on repeat access. [Test] public void DefaultOptions_returns_the_same_instance_on_repeat_access() @@ -71,5 +106,208 @@ public void JsonFunctions_holds_a_static_readonly_options_field() Assert.That(optionsFields.Length, Is.GreaterThanOrEqualTo(2), "JsonFunctions must declare shared static readonly JsonSerializerOptions fields (compact + indented) so the instances outlive each serialisation call."); } + + /// + /// Thread-safety pin — cppagent flavour. The cppagent assembly + /// ships 25 Streams.Json classes vs 10 in the plain-JSON assembly, + /// so the LCG cost per serialisation is proportionally larger; + /// the concurrency pin runs on the exact JsonFunctions surface + /// DIME's MQTT sink hits in production. + /// + [Test] + public void Convert_is_thread_safe_across_100_concurrent_callers() + { + const int threadCount = 100; + var payload = new SamplePayload(); + var outputs = new ConcurrentBag(); + var exceptions = new ConcurrentBag(); + var startGate = new ManualResetEventSlim(false); + + var threads = new Thread[threadCount]; + for (int i = 0; i < threadCount; i++) + { + threads[i] = new Thread(() => + { + try + { + startGate.Wait(); + var s = JsonFunctions.Convert(payload); + outputs.Add(s); + } + catch (System.Exception ex) + { + exceptions.Add(ex); + } + }); + threads[i].Start(); + } + + // Warm the singleton once before releasing the gate. + _ = JsonFunctions.Convert(payload); + + startGate.Set(); + foreach (var t in threads) t.Join(); + + Assert.That(exceptions, Is.Empty, + "Concurrent Convert calls against the shared DefaultOptions must not throw."); + Assert.That(outputs.Count, Is.EqualTo(threadCount)); + + var canonical = JsonFunctions.Convert(payload); + foreach (var s in outputs) + { + Assert.That(s, Is.EqualTo(canonical), + "Every concurrent Convert output must be byte-identical to the single-threaded canonical output."); + } + } + + /// + /// Cold-path pin: when a caller supplies a per-call converter, the + /// implementation must NOT append that converter to the shared + /// DefaultOptions.Converters collection. Doing so would (a) permanently + /// pollute the singleton and (b) throw InvalidOperationException + /// because JsonSerializerOptions freezes its converter list after + /// first use. + /// + [Test] + public void Convert_with_custom_converter_does_not_mutate_DefaultOptions_converters() + { + _ = JsonFunctions.Convert(new SamplePayload()); // warm/freeze + + var beforeCount = JsonFunctions.DefaultOptions.Converters.Count; + var beforeIndentCount = JsonFunctions.IndentOptions.Converters.Count; + + var converter = new NoopConverter(); + var result = JsonFunctions.Convert(new SamplePayload(), converter); + var resultIndented = JsonFunctions.Convert(new SamplePayload(), converter, indented: true); + + Assert.That(result, Is.Not.Null); + Assert.That(resultIndented, Is.Not.Null); + + Assert.That(JsonFunctions.DefaultOptions.Converters.Count, Is.EqualTo(beforeCount), + "The cold-path branch must build a fresh JsonSerializerOptions — a caller-supplied converter must never leak into DefaultOptions.Converters."); + Assert.That(JsonFunctions.IndentOptions.Converters.Count, Is.EqualTo(beforeIndentCount), + "The cold-path branch must build a fresh JsonSerializerOptions — a caller-supplied converter must never leak into IndentOptions.Converters."); + } + + /// + /// Cold-path pin: the custom-converter Convert output must reflect + /// the caller's converter (proving the fresh options object used + /// it), while a following converter-less Convert on the same input + /// must NOT reflect the converter (proving the singleton was untouched). + /// + [Test] + public void Convert_with_custom_converter_uses_converter_without_affecting_singleton_output() + { + var payload = new SamplePayload(); + var canonicalWithoutConverter = JsonFunctions.Convert(payload); + + var converter = new NoopConverter(); + var withConverter = JsonFunctions.Convert(payload, converter); + + Assert.That(withConverter, Is.Not.EqualTo(canonicalWithoutConverter), + "The cold-path fresh options must apply the caller's converter — otherwise the singleton was reused, defeating the branch."); + Assert.That(withConverter, Does.Not.Contain("Child"), + "NoopConverter drops the Child field; the cold-path output should reflect that."); + + var afterCanonical = JsonFunctions.Convert(payload); + Assert.That(afterCanonical, Is.EqualTo(canonicalWithoutConverter), + "After a converter-supplied call, the singleton-path output must be byte-identical to before."); + Assert.That(afterCanonical, Does.Contain("Child"), + "The singleton must still emit the Child field — the cold-path converter must not have polluted it."); + } + + /// + /// Overload smoke pin: Convert/ConvertBytes/ConvertStream must all + /// produce the same JSON for the same input under the compact + /// preset. + /// + [Test] + public void Convert_ConvertBytes_ConvertStream_produce_identical_output_for_compact() + { + var payload = new SamplePayload(); + + var s = JsonFunctions.Convert(payload); + var bytes = JsonFunctions.ConvertBytes(payload); + var stream = JsonFunctions.ConvertStream(payload); + + Assert.That(s, Is.Not.Null); + Assert.That(bytes, Is.Not.Null); + Assert.That(stream, Is.Not.Null); + + var bytesString = Encoding.UTF8.GetString(bytes!); + Assert.That(bytesString, Is.EqualTo(s)); + + stream!.Position = 0; + using var reader = new System.IO.StreamReader(stream); + var streamString = reader.ReadToEnd(); + Assert.That(streamString, Is.EqualTo(s)); + } + + /// Overload smoke pin — indented variant. + [Test] + public void Convert_ConvertBytes_ConvertStream_produce_identical_output_for_indented() + { + var payload = new SamplePayload(); + + var s = JsonFunctions.Convert(payload, indented: true); + var bytes = JsonFunctions.ConvertBytes(payload, indented: true); + var stream = JsonFunctions.ConvertStream(payload, indented: true); + + Assert.That(s, Is.Not.Null); + Assert.That(bytes, Is.Not.Null); + Assert.That(stream, Is.Not.Null); + + var bytesString = Encoding.UTF8.GetString(bytes!); + Assert.That(bytesString, Is.EqualTo(s)); + + stream!.Position = 0; + using var reader = new System.IO.StreamReader(stream); + var streamString = reader.ReadToEnd(); + Assert.That(streamString, Is.EqualTo(s)); + + Assert.That(s, Does.Contain("\n"), + "Indented Convert output must contain newlines."); + Assert.That(bytesString, Does.Contain("\n")); + Assert.That(streamString, Does.Contain("\n")); + } + + /// + /// Null-input pin: every overload must swallow a null input and + /// return null / null / null. + /// + [Test] + public void Convert_overloads_return_null_for_null_input() + { + Assert.That(JsonFunctions.Convert(null), Is.Null); + Assert.That(JsonFunctions.ConvertBytes(null), Is.Null); + Assert.That(JsonFunctions.ConvertStream(null), Is.Null); + } + + /// + /// Async-throughput pin: 100 parallel Convert calls via + /// Parallel.For to complement the Thread-based pin. Covers the + /// ThreadPool-scheduled path DIME's MQTT sink actually uses in + /// production. + /// + [Test] + public void Convert_is_thread_safe_under_ParallelFor() + { + _ = JsonFunctions.Convert(new SamplePayload()); + + var canonical = JsonFunctions.Convert(new SamplePayload()); + var results = new ConcurrentBag(); + + Parallel.For(0, 100, _ => + { + var s = JsonFunctions.Convert(new SamplePayload()); + results.Add(s); + }); + + Assert.That(results.Count, Is.EqualTo(100)); + foreach (var r in results) + { + Assert.That(r, Is.EqualTo(canonical)); + } + } } } From 1b96da36e3c373fec2d073041d8f9a0257cac02b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 07:53:27 +0200 Subject: [PATCH 03/15] =?UTF-8?q?feat(json,json-cppagent):=20freeze=20+=20?= =?UTF-8?q?warm=20DefaultOptions=20singletons=20=E2=80=94=20dime=20H1+M1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 block on DefaultOptions / IndentOptions calls out that the immutability contract still holds by convention on those TFMs. - Add matching 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 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(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. --- .../JsonFunctions.cs | 59 +++++++++++++++++++ libraries/MTConnect.NET-JSON/JsonFunctions.cs | 55 +++++++++++++++++ .../JsonSerializerOptionsSingletonTests.cs | 33 ++++++++++- .../JsonSerializerOptionsSingletonTests.cs | 31 ++++++++++ 4 files changed, 177 insertions(+), 1 deletion(-) diff --git a/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs b/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs index 4ea77bbb8..d776c67ac 100644 --- a/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs +++ b/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs @@ -35,6 +35,43 @@ public static class JsonFunctions private static readonly JsonSerializerOptions _defaultOptions = CreateOptions(false); private static readonly JsonSerializerOptions _indentOptions = CreateOptions(true); + static JsonFunctions() + { +#if NET8_0_OR_GREATER + // Warm the default reflection resolver + serialization + // metadata cache at assembly load rather than on the first + // production /current or /sample request. Serializing a + // typed null pays the resolver bootstrap cost + fixes the + // options' internal state, so the first user-facing + // Serialize call skips that setup entirely. This matters + // more for the cppagent assembly than for the plain-JSON + // one — it ships 25 Streams.Json classes vs 10, so the + // reflection cost per fresh options is proportionally + // larger. + // + // Order matters: the warm-up must run BEFORE MakeReadOnly, + // because MakeReadOnly(populateMissingResolver: false) + // freezes the options WITHOUT choosing a TypeInfoResolver; + // 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. + JsonSerializer.Serialize(null, _defaultOptions); + JsonSerializer.Serialize(null, _indentOptions); + + // Freeze both singletons so callers cannot mutate the + // shared instance (adding a Converter, flipping + // WriteIndented, etc.). Attempted mutation throws + // InvalidOperationException — the fail-fast is preferable + // to silent cross-caller pollution, and the cold-path + // Convert branch stays open because it builds a fresh + // (non-read-only) options object per call. + _defaultOptions.MakeReadOnly(populateMissingResolver: false); + _indentOptions.MakeReadOnly(populateMissingResolver: false); +#endif + } + private static JsonSerializerOptions CreateOptions(bool indented) { return new JsonSerializerOptions @@ -73,6 +110,17 @@ private static JsonSerializerOptions GetOptions(JsonConverter converter, bool in /// at their default value, allows numbers to be read from /// strings, and ignores property-name casing. /// + /// + /// This is a process-wide shared singleton; do NOT mutate the + /// returned instance's + /// collection or any writable property. The returned object is + /// marked MakeReadOnly() under net8.0 and later; + /// attempted mutation throws . + /// On older TFMs (netstandard2.0, net4.6.1–net4.8, net6.0, net7.0) + /// the instance is not statically frozen but callers must still + /// treat it as immutable — mutating it silently corrupts every + /// other in-process serializer that shares the singleton. + /// public static JsonSerializerOptions DefaultOptions => _defaultOptions; /// @@ -80,6 +128,17 @@ private static JsonSerializerOptions GetOptions(JsonConverter converter, bool in /// indentOutput formatter option is enabled; otherwise /// identical to . /// + /// + /// This is a process-wide shared singleton; do NOT mutate the + /// returned instance's + /// collection or any writable property. The returned object is + /// marked MakeReadOnly() under net8.0 and later; + /// attempted mutation throws . + /// On older TFMs (netstandard2.0, net4.6.1–net4.8, net6.0, net7.0) + /// the instance is not statically frozen but callers must still + /// treat it as immutable — mutating it silently corrupts every + /// other in-process serializer that shares the singleton. + /// public static JsonSerializerOptions IndentOptions => _indentOptions; diff --git a/libraries/MTConnect.NET-JSON/JsonFunctions.cs b/libraries/MTConnect.NET-JSON/JsonFunctions.cs index a9be64b4c..6f9067cf6 100644 --- a/libraries/MTConnect.NET-JSON/JsonFunctions.cs +++ b/libraries/MTConnect.NET-JSON/JsonFunctions.cs @@ -31,6 +31,39 @@ public static class JsonFunctions private static readonly JsonSerializerOptions _defaultOptions = CreateOptions(false); private static readonly JsonSerializerOptions _indentOptions = CreateOptions(true); + static JsonFunctions() + { +#if NET8_0_OR_GREATER + // Warm the default reflection resolver + serialization + // metadata cache at assembly load rather than on the first + // production /current or /sample request. Serializing a + // typed null pays the resolver bootstrap cost + fixes the + // options' internal state, so the first user-facing + // Serialize call skips that setup entirely. + // + // Order matters: the warm-up must run BEFORE MakeReadOnly, + // because MakeReadOnly(populateMissingResolver: false) + // freezes the options WITHOUT choosing a TypeInfoResolver; + // 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. + JsonSerializer.Serialize(null, _defaultOptions); + JsonSerializer.Serialize(null, _indentOptions); + + // Freeze both singletons so callers cannot mutate the + // shared instance (adding a Converter, flipping + // WriteIndented, etc.). Attempted mutation throws + // InvalidOperationException — the fail-fast is preferable + // to silent cross-caller pollution, and the cold-path + // Convert branch stays open because it builds a fresh + // (non-read-only) options object per call. + _defaultOptions.MakeReadOnly(populateMissingResolver: false); + _indentOptions.MakeReadOnly(populateMissingResolver: false); +#endif + } + private static JsonSerializerOptions CreateOptions(bool indented) { return new JsonSerializerOptions @@ -69,12 +102,34 @@ private static JsonSerializerOptions GetOptions(JsonConverter converter, bool in /// omitted on write (on net5+), numbers read from strings (on net5+), /// case-insensitive property names, and a depth limit of 1000. /// + /// + /// This is a process-wide shared singleton; do NOT mutate the + /// returned instance's + /// collection or any writable property. The returned object is + /// marked MakeReadOnly() under net8.0 and later; + /// attempted mutation throws . + /// On older TFMs (netstandard2.0, net4.6.1–net4.8, net6.0, net7.0) + /// the instance is not statically frozen but callers must still + /// treat it as immutable — mutating it silently corrupts every + /// other in-process serializer that shares the singleton. + /// public static JsonSerializerOptions DefaultOptions => _defaultOptions; /// /// The indented variant of , used when the /// formatter's indentOutput option is set. /// + /// + /// This is a process-wide shared singleton; do NOT mutate the + /// returned instance's + /// collection or any writable property. The returned object is + /// marked MakeReadOnly() under net8.0 and later; + /// attempted mutation throws . + /// On older TFMs (netstandard2.0, net4.6.1–net4.8, net6.0, net7.0) + /// the instance is not statically frozen but callers must still + /// treat it as immutable — mutating it silently corrupts every + /// other in-process serializer that shares the singleton. + /// public static JsonSerializerOptions IndentOptions => _indentOptions; diff --git a/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs b/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs index 684d968b5..7263d8fb6 100644 --- a/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs +++ b/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs @@ -322,7 +322,7 @@ public void Convert_overloads_return_null_for_null_input() /// Async-throughput pin: 100 parallel Convert calls via /// Parallel.For to complement the Thread-based pin above. The /// ThreadPool scheduling differs from raw Thread scheduling, and - /// System.Text.Json's internal metadata-lock behaviour has + /// System.Text.Json's internal metadata-lock behavior has /// historically been sensitive to the difference; both paths are /// pinned so future STJ upgrades are covered. /// @@ -346,5 +346,36 @@ public void Convert_is_thread_safe_under_ParallelFor() Assert.That(r, Is.EqualTo(canonical)); } } + +#if NET8_0_OR_GREATER + /// + /// Freeze pin (net8+): the shared DefaultOptions and IndentOptions + /// singletons must be marked read-only at static-ctor time, so any + /// attempt to mutate their + /// collection throws . + /// The freeze is the enforcement half of the singleton pattern — + /// documentation alone would let a careless caller silently pollute + /// every other in-process serializer; the read-only lock makes the + /// misuse fail loudly at the point of the offending Add. + /// + [Test] + public void DefaultOptions_Converters_Add_throws_InvalidOperationException_when_frozen() + { + Assert.Throws( + () => JsonFunctions.DefaultOptions.Converters.Add(new NoopConverter()), + "DefaultOptions is a shared singleton and must be frozen on net8+ so a stray Converters.Add fails fast rather than silently mutating the process-wide instance."); + } + + /// + /// Freeze pin (net8+) — IndentOptions mirror of the DefaultOptions guard. + /// + [Test] + public void IndentOptions_Converters_Add_throws_InvalidOperationException_when_frozen() + { + Assert.Throws( + () => JsonFunctions.IndentOptions.Converters.Add(new NoopConverter()), + "IndentOptions is a shared singleton and must be frozen on net8+ so a stray Converters.Add fails fast rather than silently mutating the process-wide instance."); + } +#endif } } diff --git a/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs b/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs index f463420c8..fd8942ae8 100644 --- a/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs +++ b/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs @@ -309,5 +309,36 @@ public void Convert_is_thread_safe_under_ParallelFor() Assert.That(r, Is.EqualTo(canonical)); } } + +#if NET8_0_OR_GREATER + /// + /// Freeze pin (net8+): the shared DefaultOptions and IndentOptions + /// singletons must be marked read-only at static-ctor time, so any + /// attempt to mutate their + /// collection throws . + /// The freeze is the enforcement half of the singleton pattern — + /// documentation alone would let a careless caller silently pollute + /// every other in-process serializer; the read-only lock makes the + /// misuse fail loudly at the point of the offending Add. + /// + [Test] + public void DefaultOptions_Converters_Add_throws_InvalidOperationException_when_frozen() + { + Assert.Throws( + () => JsonFunctions.DefaultOptions.Converters.Add(new NoopConverter()), + "DefaultOptions is a shared singleton and must be frozen on net8+ so a stray Converters.Add fails fast rather than silently mutating the process-wide instance."); + } + + /// + /// Freeze pin (net8+) — IndentOptions mirror of the DefaultOptions guard. + /// + [Test] + public void IndentOptions_Converters_Add_throws_InvalidOperationException_when_frozen() + { + Assert.Throws( + () => JsonFunctions.IndentOptions.Converters.Add(new NoopConverter()), + "IndentOptions is a shared singleton and must be frozen on net8+ so a stray Converters.Add fails fast rather than silently mutating the process-wide instance."); + } +#endif } } From 72ea0845256512c952c7cabac93f081edcae1a45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 07:54:57 +0200 Subject: [PATCH 04/15] =?UTF-8?q?docs(json,json-cppagent):=20document=20Cr?= =?UTF-8?q?eateOptions/GetOptions=20helpers=20=E2=80=94=20dime=20L1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The private CreateOptions(bool indented) and GetOptions(JsonConverter, bool) helpers on JsonFunctions carried inline comments in the method body but no /// 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 + 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. --- .../JsonFunctions.cs | 47 +++++++++++++++++++ libraries/MTConnect.NET-JSON/JsonFunctions.cs | 47 +++++++++++++++++++ 2 files changed, 94 insertions(+) diff --git a/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs b/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs index d776c67ac..9485273ce 100644 --- a/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs +++ b/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs @@ -72,6 +72,25 @@ static JsonFunctions() #endif } + /// + /// Builds a fresh instance + /// with the cppagent option preset (compact / indented per + /// , default-value omission on net5+, + /// number-from-string reading on net5+, case-insensitive property + /// lookup, 1000-deep recursion limit). + /// + /// + /// This helper is intended for two callers only: (1) the + /// static-init assignments that construct the shared + /// _defaultOptions and _indentOptions singletons, + /// and (2) the cold-path branch of when + /// a per-call converter forces a private options instance. Every + /// call allocates a new object and re-emits the STJ reflection + /// metadata cache for the reachable type graph — the very cost + /// the singletons exist to amortise — so it MUST NOT be invoked + /// on any hot-path serialization site. + /// + /// When true, sets WriteIndented = true; otherwise compact JSON. private static JsonSerializerOptions CreateOptions(bool indented) { return new JsonSerializerOptions @@ -86,6 +105,34 @@ private static JsonSerializerOptions CreateOptions(bool indented) }; } + /// + /// Resolves the instance + /// used by , + /// , and + /// for a given per-call converter + indentation combination. + /// + /// + /// Hot path ( is null, which is + /// every in-tree caller): returns the shared frozen singleton + /// (_defaultOptions or _indentOptions) so STJ + /// reuses its metadata cache instead of re-emitting property + /// accessors on every call. + /// + /// Cold path ( is non-null): + /// allocates a fresh via + /// , appends the caller's converter, + /// and returns it. This branch pays the full reflection-emit + /// cost per call and is NOT thread-safe under simultaneous cold + /// callers because the fresh options is neither shared nor + /// synchronised — each call allocates and mutates its own + /// object, so per-call concurrent use is safe; multiple callers + /// sharing one converter instance is safe iff the converter + /// itself is thread-safe. The branch exists solely to preserve + /// the public API contract for external consumers that pass a + /// per-call converter. + /// + /// Optional caller-supplied converter. When non-null forces the cold path. + /// Selects the compact or indented preset. private static JsonSerializerOptions GetOptions(JsonConverter converter, bool indented) { // Hot path: no per-call converter, hand back the shared diff --git a/libraries/MTConnect.NET-JSON/JsonFunctions.cs b/libraries/MTConnect.NET-JSON/JsonFunctions.cs index 6f9067cf6..5260b2465 100644 --- a/libraries/MTConnect.NET-JSON/JsonFunctions.cs +++ b/libraries/MTConnect.NET-JSON/JsonFunctions.cs @@ -64,6 +64,25 @@ static JsonFunctions() #endif } + /// + /// Builds a fresh instance + /// with the MTConnect option preset (compact / indented per + /// , default-value omission on net5+, + /// number-from-string reading on net5+, case-insensitive property + /// lookup, 1000-deep recursion limit). + /// + /// + /// This helper is intended for two callers only: (1) the + /// static-init assignments that construct the shared + /// _defaultOptions and _indentOptions singletons, + /// and (2) the cold-path branch of when + /// a per-call converter forces a private options instance. Every + /// call allocates a new object and re-emits the STJ reflection + /// metadata cache for the reachable type graph — the very cost + /// the singletons exist to amortise — so it MUST NOT be invoked + /// on any hot-path serialization site. + /// + /// When true, sets WriteIndented = true; otherwise compact JSON. private static JsonSerializerOptions CreateOptions(bool indented) { return new JsonSerializerOptions @@ -78,6 +97,34 @@ private static JsonSerializerOptions CreateOptions(bool indented) }; } + /// + /// Resolves the instance + /// used by , + /// , and + /// for a given per-call converter + indentation combination. + /// + /// + /// Hot path ( is null, which is + /// every in-tree caller): returns the shared frozen singleton + /// (_defaultOptions or _indentOptions) so STJ + /// reuses its metadata cache instead of re-emitting property + /// accessors on every call. + /// + /// Cold path ( is non-null): + /// allocates a fresh via + /// , appends the caller's converter, + /// and returns it. This branch pays the full reflection-emit + /// cost per call and is NOT thread-safe under simultaneous cold + /// callers because the fresh options is neither shared nor + /// synchronised — each call allocates and mutates its own + /// object, so per-call concurrent use is safe; multiple callers + /// sharing one converter instance is safe iff the converter + /// itself is thread-safe. The branch exists solely to preserve + /// the public API contract for external consumers that pass a + /// per-call converter. + /// + /// Optional caller-supplied converter. When non-null forces the cold path. + /// Selects the compact or indented preset. private static JsonSerializerOptions GetOptions(JsonConverter converter, bool indented) { // Hot path: no per-call converter, hand back the shared From 691bba40e8b3c362d0f7c3d92d38f9ed83636ac9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 07:56:01 +0200 Subject: [PATCH 05/15] =?UTF-8?q?chore(json-cppagent,tests):=20AmE=20spell?= =?UTF-8?q?ing=20normalization=20=E2=80=94=20dime=20L2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 `/// ` 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. --- .../JsonFunctions.cs | 2 +- .../JsonSerializerOptionsSingletonTests.cs | 22 +++++++++---------- .../JsonSerializerOptionsSingletonTests.cs | 18 +++++++-------- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs b/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs index 9485273ce..d1fc85b58 100644 --- a/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs +++ b/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs @@ -28,7 +28,7 @@ public static class JsonFunctions // the ~3.3 MB/h RSS climb observed on DIME production hosts in // 2026-08. The cppagent-format assembly ships 25 Streams.Json // classes (2.5× the plain-JSON count), so the LCG cost per - // serialisation is proportionally larger. The instances below + // serialization is proportionally larger. The instances below // are created once and reused; JsonSerializerOptions is // thread-safe for read after its first (de)serialization, and // nothing in this file mutates them after construction. diff --git a/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs b/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs index 7263d8fb6..1d5b54af0 100644 --- a/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs +++ b/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs @@ -15,11 +15,11 @@ namespace MTConnect.NET_JSON_Tests.Regressions /// /// Regression pin for the DIME-connector native-heap leak (peer /// diagnosis dated 2026-08-21). A fresh - /// instance owns its own serialisation-metadata cache; building that + /// instance owns its own serialization-metadata cache; building that /// cache emits reflection-based property accessors for the entire type /// graph, and the emitted code accumulates in the runtime's loader /// heaps where the GC cannot reclaim it. Allocating one on every - /// serialisation call therefore leaked ~3.3 MB/h RSS in production + /// serialization call therefore leaked ~3.3 MB/h RSS in production /// (tempco-001, tim-001). /// /// The and @@ -63,7 +63,7 @@ public override void Write(Utf8JsonWriter writer, SamplePayload value, JsonSeria } } - /// Pins the behaviour expressed by the test name: default options returns the same instance on repeat access. + /// Pins the behavior expressed by the test name: default options returns the same instance on repeat access. [Test] public void DefaultOptions_returns_the_same_instance_on_repeat_access() { @@ -73,7 +73,7 @@ public void DefaultOptions_returns_the_same_instance_on_repeat_access() "JsonFunctions.DefaultOptions must be a shared singleton — a fresh JsonSerializerOptions per call leaks LCG-emitted metadata into the loader heap."); } - /// Pins the behaviour expressed by the test name: indent options returns the same instance on repeat access. + /// Pins the behavior expressed by the test name: indent options returns the same instance on repeat access. [Test] public void IndentOptions_returns_the_same_instance_on_repeat_access() { @@ -83,7 +83,7 @@ public void IndentOptions_returns_the_same_instance_on_repeat_access() "JsonFunctions.IndentOptions must be a shared singleton — a fresh JsonSerializerOptions per call leaks LCG-emitted metadata into the loader heap."); } - /// Pins the behaviour expressed by the test name: default options and indent options are distinct instances. + /// Pins the behavior expressed by the test name: default options and indent options are distinct instances. [Test] public void DefaultOptions_and_IndentOptions_are_distinct_instances() { @@ -91,31 +91,31 @@ public void DefaultOptions_and_IndentOptions_are_distinct_instances() "DefaultOptions (compact) and IndentOptions (pretty-printed) must be separate instances so WriteIndented differs."); } - /// Pins the behaviour expressed by the test name: default options write indented is false. + /// Pins the behavior expressed by the test name: default options write indented is false. [Test] public void DefaultOptions_WriteIndented_is_false() { Assert.That(JsonFunctions.DefaultOptions.WriteIndented, Is.False); } - /// Pins the behaviour expressed by the test name: indent options write indented is true. + /// Pins the behavior expressed by the test name: indent options write indented is true. [Test] public void IndentOptions_WriteIndented_is_true() { Assert.That(JsonFunctions.IndentOptions.WriteIndented, Is.True); } - /// Pins the behaviour expressed by the test name: json functions holds a static readonly options field. + /// Pins the behavior expressed by the test name: json functions holds a static readonly options field. [Test] public void JsonFunctions_holds_a_static_readonly_options_field() { // Structural guard: at least one static readonly field of type // JsonSerializerOptions must exist on JsonFunctions so the - // shared instance survives across serialisation calls. + // shared instance survives across serialization calls. var fields = typeof(JsonFunctions).GetFields(BindingFlags.NonPublic | BindingFlags.Static); var optionsFields = System.Array.FindAll(fields, f => f.FieldType == typeof(JsonSerializerOptions) && f.IsInitOnly); Assert.That(optionsFields.Length, Is.GreaterThanOrEqualTo(2), - "JsonFunctions must declare shared static readonly JsonSerializerOptions fields (compact + indented) so the instances outlive each serialisation call."); + "JsonFunctions must declare shared static readonly JsonSerializerOptions fields (compact + indented) so the instances outlive each serialization call."); } /// @@ -307,7 +307,7 @@ public void Convert_ConvertBytes_ConvertStream_produce_identical_output_for_inde /// /// Null-input pin: every overload must swallow a null input and - /// return null / null / null — this is documented behaviour and + /// return null / null / null — this is documented behavior and /// the singleton refactor must preserve it. /// [Test] diff --git a/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs b/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs index fd8942ae8..9ae5a3b5c 100644 --- a/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs +++ b/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs @@ -20,7 +20,7 @@ namespace MTConnect.NET_JSON_cppagent_Tests.Regressions /// independent copies of the same option-preset surface, so both /// must singleton their to keep /// the runtime's loader heap from accumulating LCG-emitted property - /// accessors on every serialisation call. + /// accessors on every serialization call. /// [TestFixture] public class JsonSerializerOptionsSingletonTests @@ -55,7 +55,7 @@ public override void Write(Utf8JsonWriter writer, SamplePayload value, JsonSeria } } - /// Pins the behaviour expressed by the test name: default options returns the same instance on repeat access. + /// Pins the behavior expressed by the test name: default options returns the same instance on repeat access. [Test] public void DefaultOptions_returns_the_same_instance_on_repeat_access() { @@ -65,7 +65,7 @@ public void DefaultOptions_returns_the_same_instance_on_repeat_access() "JsonFunctions.DefaultOptions must be a shared singleton — a fresh JsonSerializerOptions per call leaks LCG-emitted metadata into the loader heap."); } - /// Pins the behaviour expressed by the test name: indent options returns the same instance on repeat access. + /// Pins the behavior expressed by the test name: indent options returns the same instance on repeat access. [Test] public void IndentOptions_returns_the_same_instance_on_repeat_access() { @@ -75,7 +75,7 @@ public void IndentOptions_returns_the_same_instance_on_repeat_access() "JsonFunctions.IndentOptions must be a shared singleton — a fresh JsonSerializerOptions per call leaks LCG-emitted metadata into the loader heap."); } - /// Pins the behaviour expressed by the test name: default options and indent options are distinct instances. + /// Pins the behavior expressed by the test name: default options and indent options are distinct instances. [Test] public void DefaultOptions_and_IndentOptions_are_distinct_instances() { @@ -83,34 +83,34 @@ public void DefaultOptions_and_IndentOptions_are_distinct_instances() "DefaultOptions (compact) and IndentOptions (pretty-printed) must be separate instances so WriteIndented differs."); } - /// Pins the behaviour expressed by the test name: default options write indented is false. + /// Pins the behavior expressed by the test name: default options write indented is false. [Test] public void DefaultOptions_WriteIndented_is_false() { Assert.That(JsonFunctions.DefaultOptions.WriteIndented, Is.False); } - /// Pins the behaviour expressed by the test name: indent options write indented is true. + /// Pins the behavior expressed by the test name: indent options write indented is true. [Test] public void IndentOptions_WriteIndented_is_true() { Assert.That(JsonFunctions.IndentOptions.WriteIndented, Is.True); } - /// Pins the behaviour expressed by the test name: json functions holds a static readonly options field. + /// Pins the behavior expressed by the test name: json functions holds a static readonly options field. [Test] public void JsonFunctions_holds_a_static_readonly_options_field() { var fields = typeof(JsonFunctions).GetFields(BindingFlags.NonPublic | BindingFlags.Static); var optionsFields = System.Array.FindAll(fields, f => f.FieldType == typeof(JsonSerializerOptions) && f.IsInitOnly); Assert.That(optionsFields.Length, Is.GreaterThanOrEqualTo(2), - "JsonFunctions must declare shared static readonly JsonSerializerOptions fields (compact + indented) so the instances outlive each serialisation call."); + "JsonFunctions must declare shared static readonly JsonSerializerOptions fields (compact + indented) so the instances outlive each serialization call."); } /// /// Thread-safety pin — cppagent flavour. The cppagent assembly /// ships 25 Streams.Json classes vs 10 in the plain-JSON assembly, - /// so the LCG cost per serialisation is proportionally larger; + /// so the LCG cost per serialization is proportionally larger; /// the concurrency pin runs on the exact JsonFunctions surface /// DIME's MQTT sink hits in production. /// From 8e124ee496211ee2eadfcc1f9818aa4cbf496393 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 08:06:11 +0200 Subject: [PATCH 06/15] =?UTF-8?q?test(json,json-cppagent):=20pin=20warm-up?= =?UTF-8?q?-before-MakeReadOnly=20static-ctor=20ordering=20(net8+)=20?= =?UTF-8?q?=E2=80=94=20coverage-FLOOR?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cycle-3 test-coverage-audit gap on PR #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(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. --- .../JsonSerializerOptionsSingletonTests.cs | 37 +++++++++++++++++++ .../JsonSerializerOptionsSingletonTests.cs | 37 +++++++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs b/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs index 1d5b54af0..1d889ed20 100644 --- a/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs +++ b/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs @@ -376,6 +376,43 @@ public void IndentOptions_Converters_Add_throws_InvalidOperationException_when_f () => JsonFunctions.IndentOptions.Converters.Add(new NoopConverter()), "IndentOptions is a shared singleton and must be frozen on net8+ so a stray Converters.Add fails fast rather than silently mutating the process-wide instance."); } + + /// + /// Warm-up-before-freeze ordering pin (net8+): the frozen singleton + /// must still be able to serialize a real typed payload, proving that + /// the static-ctor warm-up (JsonSerializer.Serialize<object>(null, _defaultOptions)) + /// ran BEFORE MakeReadOnly(populateMissingResolver: false). If a + /// future refactor reordered the two calls — or removed the warm-up + /// entirely — the frozen, resolver-less options would throw + /// on the first real-payload + /// Serialize. The invariant is documented in the JsonFunctions static + /// ctor comment ("Order matters: the warm-up must run BEFORE + /// MakeReadOnly"); this test names it as a regression pin. + /// + /// Calls + /// directly rather than via because + /// Convert's catch-all silently swallows serialization exceptions into + /// a null return — routing through it would degrade a diagnostic + /// "NotSupportedException at Serialize" into an opaque "Expected: not null". + /// + [Test] + public void Frozen_singleton_can_serialize_a_real_payload_proving_warm_up_ran_before_MakeReadOnly() + { + var payload = new SamplePayload(); + + string compact = string.Empty; + string indented = string.Empty; + Assert.DoesNotThrow( + () => compact = JsonSerializer.Serialize(payload, JsonFunctions.DefaultOptions), + "Frozen DefaultOptions must have a populated TypeInfoResolver — a static-ctor reorder that runs MakeReadOnly BEFORE the Serialize(null, …) warm-up would surface here as NotSupportedException."); + Assert.DoesNotThrow( + () => indented = JsonSerializer.Serialize(payload, JsonFunctions.IndentOptions), + "Frozen IndentOptions must have a populated TypeInfoResolver — same warm-up-before-freeze invariant as DefaultOptions."); + + Assert.That(compact, Does.Contain("\"Name\""), + "The warmed-and-frozen singleton must still emit real property data — a resolver-less frozen options would either throw or emit empty output."); + Assert.That(indented, Does.Contain("\"Name\"")); + } #endif } } diff --git a/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs b/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs index 9ae5a3b5c..b4fd48836 100644 --- a/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs +++ b/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs @@ -339,6 +339,43 @@ public void IndentOptions_Converters_Add_throws_InvalidOperationException_when_f () => JsonFunctions.IndentOptions.Converters.Add(new NoopConverter()), "IndentOptions is a shared singleton and must be frozen on net8+ so a stray Converters.Add fails fast rather than silently mutating the process-wide instance."); } + + /// + /// Warm-up-before-freeze ordering pin (net8+): the frozen singleton + /// must still be able to serialize a real typed payload, proving that + /// the static-ctor warm-up (JsonSerializer.Serialize<object>(null, _defaultOptions)) + /// ran BEFORE MakeReadOnly(populateMissingResolver: false). If a + /// future refactor reordered the two calls — or removed the warm-up + /// entirely — the frozen, resolver-less options would throw + /// on the first real-payload + /// Serialize. The invariant is documented in the JsonFunctions static + /// ctor comment ("Order matters: the warm-up must run BEFORE + /// MakeReadOnly"); this test names it as a regression pin. + /// + /// Calls + /// directly rather than via because + /// Convert's catch-all silently swallows serialization exceptions into + /// a null return — routing through it would degrade a diagnostic + /// "NotSupportedException at Serialize" into an opaque "Expected: not null". + /// + [Test] + public void Frozen_singleton_can_serialize_a_real_payload_proving_warm_up_ran_before_MakeReadOnly() + { + var payload = new SamplePayload(); + + string compact = string.Empty; + string indented = string.Empty; + Assert.DoesNotThrow( + () => compact = JsonSerializer.Serialize(payload, JsonFunctions.DefaultOptions), + "Frozen DefaultOptions must have a populated TypeInfoResolver — a static-ctor reorder that runs MakeReadOnly BEFORE the Serialize(null, …) warm-up would surface here as NotSupportedException."); + Assert.DoesNotThrow( + () => indented = JsonSerializer.Serialize(payload, JsonFunctions.IndentOptions), + "Frozen IndentOptions must have a populated TypeInfoResolver — same warm-up-before-freeze invariant as DefaultOptions."); + + Assert.That(compact, Does.Contain("\"Name\""), + "The warmed-and-frozen singleton must still emit real property data — a resolver-less frozen options would either throw or emit empty output."); + Assert.That(indented, Does.Contain("\"Name\"")); + } #endif } } From ad5ac2f3aa6b4f879f1397faa3a49f43bc1d95e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 08:19:31 +0200 Subject: [PATCH 07/15] =?UTF-8?q?perf(json,json-cppagent):=20warm-up=20tra?= =?UTF-8?q?verses=20MTConnect=20response=20graph=20=E2=80=94=20dime=20M1-C?= =?UTF-8?q?3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cycle-2's static-ctor warm-up used JsonSerializer.Serialize(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(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. --- .../JsonFunctions.cs | 51 +++++++++++++++---- libraries/MTConnect.NET-JSON/JsonFunctions.cs | 46 ++++++++++++++--- .../JsonSerializerOptionsSingletonTests.cs | 10 ++-- .../JsonSerializerOptionsSingletonTests.cs | 10 ++-- 4 files changed, 91 insertions(+), 26 deletions(-) diff --git a/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs b/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs index d1fc85b58..168932dc4 100644 --- a/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs +++ b/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs @@ -39,15 +39,25 @@ static JsonFunctions() { #if NET8_0_OR_GREATER // Warm the default reflection resolver + serialization - // metadata cache at assembly load rather than on the first - // production /current or /sample request. Serializing a - // typed null pays the resolver bootstrap cost + fixes the - // options' internal state, so the first user-facing - // Serialize call skips that setup entirely. This matters - // more for the cppagent assembly than for the plain-JSON - // one — it ships 25 Streams.Json classes vs 10, so the - // reflection cost per fresh options is proportionally - // larger. + // metadata cache at assembly load rather than on the + // first production /current | /sample | /assets request. + // Serializing a real instance of each cppagent top-level + // response surrogate forces STJ to walk that type's + // reachable property graph, which is where the + // LCG DynamicMethod property accessors are emitted; paying + // that cost once at assembly load is the whole point of + // the warm-up. This matters more for the cppagent assembly + // than for the plain-JSON one — it ships 25 Streams.Json + // classes vs 10, so the reflection cost per fresh options + // is proportionally larger. + // + // A prior warm-up form (Serialize(null, options)) + // only bootstrapped the shared reflection resolver — it + // never named a concrete MTConnect type, so the cold LCG + // emit for JsonStreamsResponseDocument / + // JsonAssetsResponseDocument / JsonDevicesResponseDocument + // still fell on the first user-facing request. The typed + // calls below fix that. // // Order matters: the warm-up must run BEFORE MakeReadOnly, // because MakeReadOnly(populateMissingResolver: false) @@ -57,8 +67,8 @@ static JsonFunctions() // 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. - JsonSerializer.Serialize(null, _defaultOptions); - JsonSerializer.Serialize(null, _indentOptions); + WarmReachableGraph(_defaultOptions); + WarmReachableGraph(_indentOptions); // Freeze both singletons so callers cannot mutate the // shared instance (adding a Converter, flipping @@ -72,6 +82,25 @@ static JsonFunctions() #endif } +#if NET8_0_OR_GREATER + // Serialize an instance of each cppagent top-level response + // surrogate against , so STJ + // configures JsonTypeInfo (and emits the LCG DynamicMethod + // property accessors) for the reachable graph rooted at each + // type. Called from the static constructor for both the + // compact and indented option singletons. + // + // All three cppagent response envelopes expose public + // parameterless constructors for JSON deserialization, so the + // warm-up just news each one up and hands it to Serialize. + private static void WarmReachableGraph(JsonSerializerOptions options) + { + JsonSerializer.Serialize(new Streams.Json.JsonStreamsResponseDocument(), options); + JsonSerializer.Serialize(new Assets.Json.JsonAssetsResponseDocument(), options); + JsonSerializer.Serialize(new Devices.Json.JsonDevicesResponseDocument(), options); + } +#endif + /// /// Builds a fresh instance /// with the cppagent option preset (compact / indented per diff --git a/libraries/MTConnect.NET-JSON/JsonFunctions.cs b/libraries/MTConnect.NET-JSON/JsonFunctions.cs index 5260b2465..26e8a6d89 100644 --- a/libraries/MTConnect.NET-JSON/JsonFunctions.cs +++ b/libraries/MTConnect.NET-JSON/JsonFunctions.cs @@ -35,11 +35,21 @@ static JsonFunctions() { #if NET8_0_OR_GREATER // Warm the default reflection resolver + serialization - // metadata cache at assembly load rather than on the first - // production /current or /sample request. Serializing a - // typed null pays the resolver bootstrap cost + fixes the - // options' internal state, so the first user-facing - // Serialize call skips that setup entirely. + // metadata cache at assembly load rather than on the + // first production /current | /sample | /assets request. + // Serializing a real instance of each MTConnect top-level + // response surrogate forces STJ to walk that type's + // reachable property graph, which is where the + // LCG DynamicMethod property accessors are emitted; paying + // that cost once at assembly load is the whole point of + // the warm-up. + // + // A prior warm-up form (Serialize(null, options)) + // only bootstrapped the shared reflection resolver — it + // never named a concrete MTConnect type, so the cold LCG + // emit for JsonStreamsDocument / JsonAssetsDocument / + // JsonDevicesDocument still fell on the first user-facing + // request. The typed calls below fix that. // // Order matters: the warm-up must run BEFORE MakeReadOnly, // because MakeReadOnly(populateMissingResolver: false) @@ -49,8 +59,8 @@ static JsonFunctions() // 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. - JsonSerializer.Serialize(null, _defaultOptions); - JsonSerializer.Serialize(null, _indentOptions); + WarmReachableGraph(_defaultOptions); + WarmReachableGraph(_indentOptions); // Freeze both singletons so callers cannot mutate the // shared instance (adding a Converter, flipping @@ -64,6 +74,28 @@ static JsonFunctions() #endif } +#if NET8_0_OR_GREATER + // Serialize an instance of each MTConnect top-level response + // surrogate against , so STJ + // configures JsonTypeInfo (and emits the LCG DynamicMethod + // property accessors) for the reachable graph rooted at each + // type. Called from the static constructor for both the + // compact and indented option singletons. + // + // JsonAssetsDocument has no public parameterless constructor; + // its single (IAssetsResponseDocument) overload tolerates a + // null argument and yields an instance with null Header + + // null Assets, which is enough to walk its declared property + // types. JsonStreamsDocument and JsonDevicesDocument both + // expose parameterless constructors for JSON deserialization. + private static void WarmReachableGraph(JsonSerializerOptions options) + { + JsonSerializer.Serialize(new Streams.Json.JsonStreamsDocument(), options); + JsonSerializer.Serialize(new Assets.Json.JsonAssetsDocument(null), options); + JsonSerializer.Serialize(new Devices.Json.JsonDevicesDocument(), options); + } +#endif + /// /// Builds a fresh instance /// with the MTConnect option preset (compact / indented per diff --git a/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs b/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs index 1d889ed20..dd24349de 100644 --- a/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs +++ b/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs @@ -380,9 +380,11 @@ public void IndentOptions_Converters_Add_throws_InvalidOperationException_when_f /// /// Warm-up-before-freeze ordering pin (net8+): the frozen singleton /// must still be able to serialize a real typed payload, proving that - /// the static-ctor warm-up (JsonSerializer.Serialize<object>(null, _defaultOptions)) - /// ran BEFORE MakeReadOnly(populateMissingResolver: false). If a - /// future refactor reordered the two calls — or removed the warm-up + /// the static-ctor warm-up (typed JsonSerializer.Serialize + /// calls against each MTConnect top-level response surrogate — see + /// JsonFunctions.WarmReachableGraph) ran BEFORE + /// MakeReadOnly(populateMissingResolver: false). If a future + /// refactor reordered the two calls — or removed the warm-up /// entirely — the frozen, resolver-less options would throw /// on the first real-payload /// Serialize. The invariant is documented in the JsonFunctions static @@ -404,7 +406,7 @@ public void Frozen_singleton_can_serialize_a_real_payload_proving_warm_up_ran_be string indented = string.Empty; Assert.DoesNotThrow( () => compact = JsonSerializer.Serialize(payload, JsonFunctions.DefaultOptions), - "Frozen DefaultOptions must have a populated TypeInfoResolver — a static-ctor reorder that runs MakeReadOnly BEFORE the Serialize(null, …) warm-up would surface here as NotSupportedException."); + "Frozen DefaultOptions must have a populated TypeInfoResolver — a static-ctor reorder that runs MakeReadOnly BEFORE the WarmReachableGraph typed-Serialize calls would surface here as NotSupportedException."); Assert.DoesNotThrow( () => indented = JsonSerializer.Serialize(payload, JsonFunctions.IndentOptions), "Frozen IndentOptions must have a populated TypeInfoResolver — same warm-up-before-freeze invariant as DefaultOptions."); diff --git a/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs b/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs index b4fd48836..3cf5438eb 100644 --- a/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs +++ b/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs @@ -343,9 +343,11 @@ public void IndentOptions_Converters_Add_throws_InvalidOperationException_when_f /// /// Warm-up-before-freeze ordering pin (net8+): the frozen singleton /// must still be able to serialize a real typed payload, proving that - /// the static-ctor warm-up (JsonSerializer.Serialize<object>(null, _defaultOptions)) - /// ran BEFORE MakeReadOnly(populateMissingResolver: false). If a - /// future refactor reordered the two calls — or removed the warm-up + /// the static-ctor warm-up (typed JsonSerializer.Serialize + /// calls against each cppagent top-level response surrogate — see + /// JsonFunctions.WarmReachableGraph) ran BEFORE + /// MakeReadOnly(populateMissingResolver: false). If a future + /// refactor reordered the two calls — or removed the warm-up /// entirely — the frozen, resolver-less options would throw /// on the first real-payload /// Serialize. The invariant is documented in the JsonFunctions static @@ -367,7 +369,7 @@ public void Frozen_singleton_can_serialize_a_real_payload_proving_warm_up_ran_be string indented = string.Empty; Assert.DoesNotThrow( () => compact = JsonSerializer.Serialize(payload, JsonFunctions.DefaultOptions), - "Frozen DefaultOptions must have a populated TypeInfoResolver — a static-ctor reorder that runs MakeReadOnly BEFORE the Serialize(null, …) warm-up would surface here as NotSupportedException."); + "Frozen DefaultOptions must have a populated TypeInfoResolver — a static-ctor reorder that runs MakeReadOnly BEFORE the WarmReachableGraph typed-Serialize calls would surface here as NotSupportedException."); Assert.DoesNotThrow( () => indented = JsonSerializer.Serialize(payload, JsonFunctions.IndentOptions), "Frozen IndentOptions must have a populated TypeInfoResolver — same warm-up-before-freeze invariant as DefaultOptions."); From b072627a1ebf9f220399307f5a087f5ec12811c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 08:20:25 +0200 Subject: [PATCH 08/15] =?UTF-8?q?chore(json,json-cppagent,tests):=20AmE=20?= =?UTF-8?q?tokens=20amortize+synchronize+flavor=20=E2=80=94=20dime=20M2-C3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 ) * synchronised → synchronized (GetOptions ) - 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). --- libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs | 4 ++-- libraries/MTConnect.NET-JSON/JsonFunctions.cs | 4 ++-- .../Regressions/JsonSerializerOptionsSingletonTests.cs | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs b/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs index 168932dc4..cd83b07ec 100644 --- a/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs +++ b/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs @@ -116,7 +116,7 @@ private static void WarmReachableGraph(JsonSerializerOptions options) /// a per-call converter forces a private options instance. Every /// call allocates a new object and re-emits the STJ reflection /// metadata cache for the reachable type graph — the very cost - /// the singletons exist to amortise — so it MUST NOT be invoked + /// the singletons exist to amortize — so it MUST NOT be invoked /// on any hot-path serialization site. /// /// When true, sets WriteIndented = true; otherwise compact JSON. @@ -153,7 +153,7 @@ private static JsonSerializerOptions CreateOptions(bool indented) /// and returns it. This branch pays the full reflection-emit /// cost per call and is NOT thread-safe under simultaneous cold /// callers because the fresh options is neither shared nor - /// synchronised — each call allocates and mutates its own + /// synchronized — each call allocates and mutates its own /// object, so per-call concurrent use is safe; multiple callers /// sharing one converter instance is safe iff the converter /// itself is thread-safe. The branch exists solely to preserve diff --git a/libraries/MTConnect.NET-JSON/JsonFunctions.cs b/libraries/MTConnect.NET-JSON/JsonFunctions.cs index 26e8a6d89..64284f9e4 100644 --- a/libraries/MTConnect.NET-JSON/JsonFunctions.cs +++ b/libraries/MTConnect.NET-JSON/JsonFunctions.cs @@ -111,7 +111,7 @@ private static void WarmReachableGraph(JsonSerializerOptions options) /// a per-call converter forces a private options instance. Every /// call allocates a new object and re-emits the STJ reflection /// metadata cache for the reachable type graph — the very cost - /// the singletons exist to amortise — so it MUST NOT be invoked + /// the singletons exist to amortize — so it MUST NOT be invoked /// on any hot-path serialization site. /// /// When true, sets WriteIndented = true; otherwise compact JSON. @@ -148,7 +148,7 @@ private static JsonSerializerOptions CreateOptions(bool indented) /// and returns it. This branch pays the full reflection-emit /// cost per call and is NOT thread-safe under simultaneous cold /// callers because the fresh options is neither shared nor - /// synchronised — each call allocates and mutates its own + /// synchronized — each call allocates and mutates its own /// object, so per-call concurrent use is safe; multiple callers /// sharing one converter instance is safe iff the converter /// itself is thread-safe. The branch exists solely to preserve diff --git a/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs b/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs index 3cf5438eb..9b1eb0ec7 100644 --- a/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs +++ b/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs @@ -14,7 +14,7 @@ namespace MTConnect.NET_JSON_cppagent_Tests.Regressions { /// /// Regression pin for the DIME-connector native-heap leak (peer - /// diagnosis dated 2026-08-21). This is the cppagent-flavoured + /// diagnosis dated 2026-08-21). This is the cppagent-flavored /// mirror of the same guard applied to /// MTConnect.NET-JSON/JsonFunctions.cs. The two files ship /// independent copies of the same option-preset surface, so both @@ -108,7 +108,7 @@ public void JsonFunctions_holds_a_static_readonly_options_field() } /// - /// Thread-safety pin — cppagent flavour. The cppagent assembly + /// Thread-safety pin — cppagent flavor. The cppagent assembly /// ships 25 Streams.Json classes vs 10 in the plain-JSON assembly, /// so the LCG cost per serialization is proportionally larger; /// the concurrency pin runs on the exact JsonFunctions surface From dac03ba6929cd30c7c8f6f6f49bce397faada138 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 09:52:20 +0200 Subject: [PATCH 09/15] =?UTF-8?q?fix(json,json-cppagent):=20warm=20ErrorRe?= =?UTF-8?q?sponseDocument=20+=20WHY-LCG=20on=20option=20singletons=20?= =?UTF-8?q?=E2=80=94=20dime=20M+L-C4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cycle-4 leftovers on the JsonSerializerOptions singleton campaign (#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 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. --- .../JsonFunctions.cs | 40 +++++++++++++++++-- libraries/MTConnect.NET-JSON/JsonFunctions.cs | 36 ++++++++++++++++- .../JsonSerializerOptionsSingletonTests.cs | 30 ++++++++++++++ .../JsonSerializerOptionsSingletonTests.cs | 30 ++++++++++++++ 4 files changed, 131 insertions(+), 5 deletions(-) diff --git a/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs b/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs index cd83b07ec..687b0c542 100644 --- a/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs +++ b/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs @@ -55,9 +55,16 @@ static JsonFunctions() // only bootstrapped the shared reflection resolver — it // never named a concrete MTConnect type, so the cold LCG // emit for JsonStreamsResponseDocument / - // JsonAssetsResponseDocument / JsonDevicesResponseDocument - // still fell on the first user-facing request. The typed - // calls below fix that. + // JsonAssetsResponseDocument / JsonDevicesResponseDocument / + // ErrorResponseDocument still fell on the first user-facing + // request. The typed calls below fix that. + // ErrorResponseDocument (from MTConnect.Errors) is the + // surrogate-less envelope written directly by + // JsonHttpResponseDocumentFormatter.Format(IErrorResponseDocument); + // its cold LCG emit would otherwise hit on the first /probe + // error, /current parse failure, or unsupported device + // response — request paths that are already the symptom, + // not the happy path we want to pay reflection cost on top of. // // Order matters: the warm-up must run BEFORE MakeReadOnly, // because MakeReadOnly(populateMissingResolver: false) @@ -93,11 +100,15 @@ static JsonFunctions() // All three cppagent response envelopes expose public // parameterless constructors for JSON deserialization, so the // warm-up just news each one up and hands it to Serialize. + // ErrorResponseDocument (from MTConnect.Errors) is the fourth + // top-level envelope — no cppagent-specific surrogate, written + // by the formatter's IErrorResponseDocument overload directly. private static void WarmReachableGraph(JsonSerializerOptions options) { JsonSerializer.Serialize(new Streams.Json.JsonStreamsResponseDocument(), options); JsonSerializer.Serialize(new Assets.Json.JsonAssetsResponseDocument(), options); JsonSerializer.Serialize(new Devices.Json.JsonDevicesResponseDocument(), options); + JsonSerializer.Serialize(new Errors.ErrorResponseDocument(), options); } #endif @@ -196,6 +207,20 @@ private static JsonSerializerOptions GetOptions(JsonConverter converter, bool in /// the instance is not statically frozen but callers must still /// treat it as immutable — mutating it silently corrupts every /// other in-process serializer that shares the singleton. + /// + /// Why the invariant matters: every mutation of a + /// instance that has already + /// been used forces System.Text.Json to rebuild its serialization + /// metadata cache, which re-emits reflection-based property + /// accessors as s + /// into the runtime's LCG (lightweight code generation) loader + /// heaps. Those heaps are never reclaimed by the GC — every + /// mutated-then-reused options object leaks the emit permanently. + /// A peer measured this class of misuse at +3.2–3.8 MB/h RSS in + /// production; the cppagent assembly ships 25 Streams.Json + /// classes (2.5× the plain-JSON count), so the per-mutation LCG + /// cost here is proportionally larger and the frozen singleton + /// pays back correspondingly more. /// public static JsonSerializerOptions DefaultOptions => _defaultOptions; @@ -214,6 +239,15 @@ private static JsonSerializerOptions GetOptions(JsonConverter converter, bool in /// the instance is not statically frozen but callers must still /// treat it as immutable — mutating it silently corrupts every /// other in-process serializer that shares the singleton. + /// + /// Why the invariant matters: same reflection-emit / LCG + /// loader-heap accumulation as documented on + /// . Every per-call mutation of this + /// shared instance re-emits property-accessor + /// s into a + /// heap the GC cannot free — the mechanism behind the peer's + /// production +3.2–3.8 MB/h RSS climb before the singleton was + /// frozen. /// public static JsonSerializerOptions IndentOptions => _indentOptions; diff --git a/libraries/MTConnect.NET-JSON/JsonFunctions.cs b/libraries/MTConnect.NET-JSON/JsonFunctions.cs index 64284f9e4..73e4762c0 100644 --- a/libraries/MTConnect.NET-JSON/JsonFunctions.cs +++ b/libraries/MTConnect.NET-JSON/JsonFunctions.cs @@ -48,8 +48,16 @@ static JsonFunctions() // only bootstrapped the shared reflection resolver — it // never named a concrete MTConnect type, so the cold LCG // emit for JsonStreamsDocument / JsonAssetsDocument / - // JsonDevicesDocument still fell on the first user-facing - // request. The typed calls below fix that. + // JsonDevicesDocument / ErrorResponseDocument still fell + // on the first user-facing request. The typed calls below + // fix that. ErrorResponseDocument is the surrogate-less + // response envelope written directly by + // JsonResponseDocumentFormatter.Format(IErrorResponseDocument), + // so its cold LCG emit would otherwise hit on the first + // /probe error, /current parse failure, or unsupported + // device response — request paths that are already the + // symptom, not the happy path we want to pay reflection + // cost on top of. // // Order matters: the warm-up must run BEFORE MakeReadOnly, // because MakeReadOnly(populateMissingResolver: false) @@ -88,11 +96,15 @@ static JsonFunctions() // null Assets, which is enough to walk its declared property // types. JsonStreamsDocument and JsonDevicesDocument both // expose parameterless constructors for JSON deserialization. + // ErrorResponseDocument (MTConnect.Errors) is the fourth + // top-level response envelope — no surrogate wrapper, written + // by the formatter's IErrorResponseDocument overload directly. private static void WarmReachableGraph(JsonSerializerOptions options) { JsonSerializer.Serialize(new Streams.Json.JsonStreamsDocument(), options); JsonSerializer.Serialize(new Assets.Json.JsonAssetsDocument(null), options); JsonSerializer.Serialize(new Devices.Json.JsonDevicesDocument(), options); + JsonSerializer.Serialize(new Errors.ErrorResponseDocument(), options); } #endif @@ -191,6 +203,17 @@ private static JsonSerializerOptions GetOptions(JsonConverter converter, bool in /// the instance is not statically frozen but callers must still /// treat it as immutable — mutating it silently corrupts every /// other in-process serializer that shares the singleton. + /// + /// Why the invariant matters: every mutation of a + /// instance that has already + /// been used forces System.Text.Json to rebuild its serialization + /// metadata cache, which re-emits reflection-based property + /// accessors as s + /// into the runtime's LCG (lightweight code generation) loader + /// heaps. Those heaps are never reclaimed by the GC — every + /// mutated-then-reused options object leaks the emit permanently. + /// A peer measured this class of misuse at +3.2–3.8 MB/h RSS in + /// production, which is what the frozen singleton eliminates. /// public static JsonSerializerOptions DefaultOptions => _defaultOptions; @@ -208,6 +231,15 @@ private static JsonSerializerOptions GetOptions(JsonConverter converter, bool in /// the instance is not statically frozen but callers must still /// treat it as immutable — mutating it silently corrupts every /// other in-process serializer that shares the singleton. + /// + /// Why the invariant matters: same reflection-emit / LCG + /// loader-heap accumulation as documented on + /// . Every per-call mutation of this + /// shared instance re-emits property-accessor + /// s into a + /// heap the GC cannot free — the mechanism behind the peer's + /// production +3.2–3.8 MB/h RSS climb before the singleton was + /// frozen. /// public static JsonSerializerOptions IndentOptions => _indentOptions; diff --git a/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs b/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs index dd24349de..0eead1eac 100644 --- a/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs +++ b/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs @@ -415,6 +415,36 @@ public void Frozen_singleton_can_serialize_a_real_payload_proving_warm_up_ran_be "The warmed-and-frozen singleton must still emit real property data — a resolver-less frozen options would either throw or emit empty output."); Assert.That(indented, Does.Contain("\"Name\"")); } + + /// + /// Warm-up coverage pin (net8+) — MTConnect.Errors.ErrorResponseDocument + /// is the fourth top-level response envelope written directly by + /// JsonResponseDocumentFormatter.Format(IErrorResponseDocument, ...) + /// without a Json* surrogate wrapper. The static-ctor + /// WarmReachableGraph pass must serialize an instance of it + /// against both option singletons BEFORE + /// MakeReadOnly(populateMissingResolver: false) — otherwise the + /// first error response (a /probe failure, unsupported device request, + /// or parse error) would pay a cold LCG DynamicMethod emit against + /// a frozen, resolver-less options and throw + /// . Serializing a fresh + /// here reproduces + /// exactly that first-error path against the shared singletons; a + /// regression that dropped the Error warm-up would surface as a + /// NotSupportedException on this test. + /// + [Test] + public void Frozen_singleton_can_serialize_ErrorResponseDocument_proving_Error_warm_up_ran() + { + var document = new MTConnect.Errors.ErrorResponseDocument(); + + Assert.DoesNotThrow( + () => JsonSerializer.Serialize(document, JsonFunctions.DefaultOptions), + "Frozen DefaultOptions must have ErrorResponseDocument in its warmed TypeInfoResolver — a regression that removed the Error warm-up would surface here as NotSupportedException on the first error response."); + Assert.DoesNotThrow( + () => JsonSerializer.Serialize(document, JsonFunctions.IndentOptions), + "Frozen IndentOptions mirror — same Error warm-up invariant as DefaultOptions."); + } #endif } } diff --git a/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs b/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs index 9b1eb0ec7..436167c44 100644 --- a/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs +++ b/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs @@ -378,6 +378,36 @@ public void Frozen_singleton_can_serialize_a_real_payload_proving_warm_up_ran_be "The warmed-and-frozen singleton must still emit real property data — a resolver-less frozen options would either throw or emit empty output."); Assert.That(indented, Does.Contain("\"Name\"")); } + + /// + /// Warm-up coverage pin (net8+) — MTConnect.Errors.ErrorResponseDocument + /// is the fourth top-level response envelope written directly by + /// JsonHttpResponseDocumentFormatter.Format(IErrorResponseDocument, ...) + /// without a cppagent-specific surrogate. The static-ctor + /// WarmReachableGraph pass must serialize an instance of it + /// against both option singletons BEFORE + /// MakeReadOnly(populateMissingResolver: false) — otherwise the + /// first error response (a /probe failure, unsupported device request, + /// or parse error) would pay a cold LCG DynamicMethod emit against + /// a frozen, resolver-less options and throw + /// . Serializing a fresh + /// here reproduces + /// exactly that first-error path against the shared singletons; a + /// regression that dropped the Error warm-up would surface as a + /// NotSupportedException on this test. + /// + [Test] + public void Frozen_singleton_can_serialize_ErrorResponseDocument_proving_Error_warm_up_ran() + { + var document = new MTConnect.Errors.ErrorResponseDocument(); + + Assert.DoesNotThrow( + () => JsonSerializer.Serialize(document, JsonFunctions.DefaultOptions), + "Frozen DefaultOptions must have ErrorResponseDocument in its warmed TypeInfoResolver — a regression that removed the Error warm-up would surface here as NotSupportedException on the first error response."); + Assert.DoesNotThrow( + () => JsonSerializer.Serialize(document, JsonFunctions.IndentOptions), + "Frozen IndentOptions mirror — same Error warm-up invariant as DefaultOptions."); + } #endif } } From 78a174d541708b94d4e294f66024d1a5913503f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 10:10:50 +0200 Subject: [PATCH 10/15] =?UTF-8?q?test(json,json-cppagent,common,mqttrelay)?= =?UTF-8?q?:=20replace=20tautological=20ErrorResponseDocument=20pin=20with?= =?UTF-8?q?=20IL-inspection;=20add=20sibling=20structural=20pins=20?= =?UTF-8?q?=E2=80=94=20coverage-FLOOR?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 #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). --- .../MTConnectMqttMessageSingletonTests.cs | 55 +++++++++ ...nSerializerOptionsSiblingSingletonTests.cs | 116 ++++++++++++++++++ .../JsonSerializerOptionsSingletonTests.cs | 103 ++++++++++++---- .../JsonSerializerOptionsSingletonTests.cs | 105 ++++++++++++---- 4 files changed, 335 insertions(+), 44 deletions(-) create mode 100644 tests/MTConnect.NET-AgentModule-MqttRelay-Tests/Regressions/MTConnectMqttMessageSingletonTests.cs create mode 100644 tests/MTConnect.NET-Common-Tests/Regressions/JsonSerializerOptionsSiblingSingletonTests.cs diff --git a/tests/MTConnect.NET-AgentModule-MqttRelay-Tests/Regressions/MTConnectMqttMessageSingletonTests.cs b/tests/MTConnect.NET-AgentModule-MqttRelay-Tests/Regressions/MTConnectMqttMessageSingletonTests.cs new file mode 100644 index 000000000..ab1543195 --- /dev/null +++ b/tests/MTConnect.NET-AgentModule-MqttRelay-Tests/Regressions/MTConnectMqttMessageSingletonTests.cs @@ -0,0 +1,55 @@ +// Copyright (c) 2026 TrakHound Inc., All Rights Reserved. +// TrakHound Inc. licenses this file to you under the MIT license. + +using System.Reflection; +using System.Text.Json; +using MTConnect.Mqtt; +using NUnit.Framework; + +namespace MTConnect.AgentModule.MqttRelay.Tests.Regressions +{ + /// + /// Sibling-site structural pin for the DIME-connector native-heap leak + /// (peer diagnosis dated 2026-08-21). + /// (in MTConnect.NET-MQTT) is the fifth per-call-new JsonSerializerOptions + /// site flipped to a shared static readonly field in PR #249; it lives in + /// a library MTConnect.NET-Common-Tests does not reference, so its + /// structural pin lives here in the MqttRelay test project which + /// transitively references MTConnect.NET-MQTT. + /// + /// Hosted under an MQTT-flavored publisher path — every /agent + /// information republish flows through + /// MTConnectMqttMessage.CreateAgentInformation, and its + /// _agentInformationOptions must be a shared singleton, not a + /// per-call allocation. See the sibling fixture + /// JsonSerializerOptionsSiblingSingletonTests in + /// MTConnect.NET-Common-Tests for the rationale and the twin + /// pins on the four Common sibling sites. + /// + [TestFixture] + public class MTConnectMqttMessageSingletonTests + { + /// + /// Pin: serializes agent + /// information into every /Agents/{uuid}/Information republish. + /// Its _agentInformationOptions must be a shared singleton, + /// not a per-call allocation, so the LCG DynamicMethod + /// property-accessor emit is paid once at assembly load rather + /// than once per publish. + /// + [Test] + public void MTConnectMqttMessage_holds_a_static_readonly_JsonSerializerOptions_field() + { + var fields = typeof(MTConnectMqttMessage).GetFields( + BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Static); + var optionsFields = System.Array.FindAll( + fields, + f => f.FieldType == typeof(JsonSerializerOptions) && f.IsInitOnly); + Assert.That(optionsFields.Length, Is.GreaterThanOrEqualTo(1), + "MTConnectMqttMessage must declare at least one static readonly JsonSerializerOptions field so " + + "the instance outlives each publish. A per-call `new JsonSerializerOptions(...)` re-emits LCG " + + "DynamicMethod property accessors on every call, and those emits accumulate in the runtime's " + + "loader heap where the GC cannot reclaim them (+3.2-3.8 MB/h RSS in production)."); + } + } +} diff --git a/tests/MTConnect.NET-Common-Tests/Regressions/JsonSerializerOptionsSiblingSingletonTests.cs b/tests/MTConnect.NET-Common-Tests/Regressions/JsonSerializerOptionsSiblingSingletonTests.cs new file mode 100644 index 000000000..25deeeaac --- /dev/null +++ b/tests/MTConnect.NET-Common-Tests/Regressions/JsonSerializerOptionsSiblingSingletonTests.cs @@ -0,0 +1,116 @@ +// Copyright (c) 2026 TrakHound Inc., All Rights Reserved. +// TrakHound Inc. licenses this file to you under the MIT license. + +using System.Reflection; +using System.Text.Json; +using MTConnect.Agents; +using MTConnect.Buffers; +using MTConnect.Clients; +using MTConnect.Configurations; +using NUnit.Framework; + +namespace MTConnect.NET_Common_Tests.Regressions +{ + /// + /// Sibling-site structural pin for the DIME-connector native-heap leak + /// (peer diagnosis dated 2026-08-21). The primary singleton refactor + /// landed on MTConnect.NET-JSON.JsonFunctions and its cppagent + /// twin, but the same per-call new JsonSerializerOptions(...) + /// misuse existed at four other sites inside MTConnect.NET-Common that + /// also allocate an options object once per Save / Read / Write call: + /// , + /// , + /// , + /// , and + /// . Each was flipped to a + /// private static readonly JsonSerializerOptions field in + /// PR #249 so the LCG DynamicMethod property-accessor emit is paid + /// once per assembly-load instead of once per call. + /// + /// This fixture pins the STRUCTURAL shape (a private static + /// readonly JsonSerializerOptions field exists) rather than an + /// instance-identity via ReferenceEquals, because the fields + /// are private and un-exposed. A regression that either + /// (a) removes the field and re-inlines new JsonSerializerOptions(...) + /// per call, or (b) flips the field to instance / non-readonly, would + /// silently reintroduce the LCG leak; each pin below fails cleanly + /// on such a refactor. The MTConnect.NET-MQTT sibling + /// (MTConnectMqttMessage._agentInformationOptions) is pinned + /// in the MqttRelay test project because MTConnect.NET-Common-Tests + /// does not reference MTConnect.NET-MQTT. + /// + [TestFixture] + public class JsonSerializerOptionsSiblingSingletonTests + { + private static void AssertHoldsStaticReadonlyJsonSerializerOptionsField(System.Type type) + { + var fields = type.GetFields(BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Static); + var optionsFields = System.Array.FindAll( + fields, + f => f.FieldType == typeof(JsonSerializerOptions) && f.IsInitOnly); + Assert.That(optionsFields.Length, Is.GreaterThanOrEqualTo(1), + $"{type.FullName} must declare at least one static readonly JsonSerializerOptions field so " + + "the instance outlives each serialization call. A per-call `new JsonSerializerOptions(...)` " + + "re-emits LCG DynamicMethod property accessors on every call, and those emits accumulate in " + + "the runtime's loader heap where the GC cannot reclaim them (+3.2-3.8 MB/h RSS in production)."); + } + + /// + /// Pin: serializes itself to + /// disk on every Save call. Its _saveOptions must be a shared + /// singleton, not a per-call allocation. + /// + [Test] + public void MTConnectAgentInformation_holds_a_static_readonly_JsonSerializerOptions_field() + { + AssertHoldsStaticReadonlyJsonSerializerOptionsField(typeof(MTConnectAgentInformation)); + } + + /// + /// Pin: mirrors the Agent + /// information persistence pattern client-side. Its _saveOptions + /// must be a shared singleton, not a per-call allocation. + /// + [Test] + public void MTConnectClientInformation_holds_a_static_readonly_JsonSerializerOptions_field() + { + AssertHoldsStaticReadonlyJsonSerializerOptionsField(typeof(MTConnectClientInformation)); + } + + /// + /// Pin: writes one JSON file + /// per asset — potentially many per second under load. Its + /// _writeOptions must be a shared singleton, not a per-call + /// allocation. + /// + [Test] + public void MTConnectAssetFileBuffer_holds_a_static_readonly_JsonSerializerOptions_field() + { + AssertHoldsStaticReadonlyJsonSerializerOptionsField(typeof(MTConnectAssetFileBuffer)); + } + + /// + /// Pin: deserializes + /// its config file on startup and on every reload. Its + /// _readOptions must be a shared singleton, not a per-call + /// allocation — hot-reload watchers can trigger many parses per + /// minute. + /// + [Test] + public void AdapterApplicationConfiguration_holds_a_static_readonly_JsonSerializerOptions_field() + { + AssertHoldsStaticReadonlyJsonSerializerOptionsField(typeof(AdapterApplicationConfiguration)); + } + + /// + /// Pin: deserializes its config file + /// on startup and on every reload. Same singleton discipline as + /// . + /// + [Test] + public void AgentConfiguration_holds_a_static_readonly_JsonSerializerOptions_field() + { + AssertHoldsStaticReadonlyJsonSerializerOptionsField(typeof(AgentConfiguration)); + } + } +} diff --git a/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs b/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs index 0eead1eac..ab0a394fc 100644 --- a/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs +++ b/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs @@ -417,33 +417,92 @@ public void Frozen_singleton_can_serialize_a_real_payload_proving_warm_up_ran_be } /// - /// Warm-up coverage pin (net8+) — MTConnect.Errors.ErrorResponseDocument - /// is the fourth top-level response envelope written directly by + /// Structural warm-up-coverage pin (net8+) — WarmReachableGraph's + /// IL must contain a newobj instruction constructing + /// , the fourth + /// top-level response envelope written directly by /// JsonResponseDocumentFormatter.Format(IErrorResponseDocument, ...) - /// without a Json* surrogate wrapper. The static-ctor - /// WarmReachableGraph pass must serialize an instance of it - /// against both option singletons BEFORE - /// MakeReadOnly(populateMissingResolver: false) — otherwise the - /// first error response (a /probe failure, unsupported device request, - /// or parse error) would pay a cold LCG DynamicMethod emit against - /// a frozen, resolver-less options and throw - /// . Serializing a fresh - /// here reproduces - /// exactly that first-error path against the shared singletons; a - /// regression that dropped the Error warm-up would surface as a - /// NotSupportedException on this test. + /// without a Json* surrogate wrapper. + /// + /// A runtime Assert.DoesNotThrow(() => Serialize(new ErrorResponseDocument(), frozen)) + /// check is tautological here: once the TypeInfoResolver is set (which + /// any preceding warm-up call does), STJ's + /// lazily populates + /// on frozen options for arbitrary types — the frozen state only locks + /// the options' configuration surface (Converters, DefaultIgnoreCondition, + /// etc.), not the internal metadata cache. Verified empirically: + /// mutating out the ErrorResponseDocument warm-up line and running + /// the "can it serialize" pin still passes (see cycle-5 test-coverage-audit + /// mutation test 2026-08-21). The performance invariant the warm-up + /// enforces — pay the LCG DynamicMethod emit for ErrorResponseDocument + /// at assembly load, not on the first /probe error / parse-failure + /// request — is therefore only observable at the source-of-truth level: + /// the IL of WarmReachableGraph. /// [Test] - public void Frozen_singleton_can_serialize_ErrorResponseDocument_proving_Error_warm_up_ran() + public void WarmReachableGraph_IL_contains_newobj_for_ErrorResponseDocument_ctor() { - var document = new MTConnect.Errors.ErrorResponseDocument(); + AssertWarmReachableGraphNewsUp(typeof(MTConnect.Errors.ErrorResponseDocument)); + } - Assert.DoesNotThrow( - () => JsonSerializer.Serialize(document, JsonFunctions.DefaultOptions), - "Frozen DefaultOptions must have ErrorResponseDocument in its warmed TypeInfoResolver — a regression that removed the Error warm-up would surface here as NotSupportedException on the first error response."); - Assert.DoesNotThrow( - () => JsonSerializer.Serialize(document, JsonFunctions.IndentOptions), - "Frozen IndentOptions mirror — same Error warm-up invariant as DefaultOptions."); + /// + /// Structural warm-up-coverage pin (net8+) — companion assertion that + /// each of the three Json* top-level response surrogates is also + /// news-up-ed by WarmReachableGraph. Mirrors the ErrorResponseDocument + /// pin above; the same tautology argument applies to per-type + /// "can it serialize" runtime checks. + /// + [Test] + public void WarmReachableGraph_IL_contains_newobj_for_every_top_level_response_surrogate() + { + AssertWarmReachableGraphNewsUp(typeof(MTConnect.Streams.Json.JsonStreamsDocument)); + AssertWarmReachableGraphNewsUp(typeof(MTConnect.Assets.Json.JsonAssetsDocument)); + AssertWarmReachableGraphNewsUp(typeof(MTConnect.Devices.Json.JsonDevicesDocument)); + } + + // IL walker: locate a `newobj ` instruction inside + // JsonFunctions.WarmReachableGraph. Runs on the compiled test assembly's + // view of MTConnect.NET-JSON, so a source change that removes the + // corresponding new-expression is caught deterministically at test time. + // + // Scans for the 5-byte pattern <4-byte-token>. Full + // opcode-length decoding is unnecessary: a spurious match would need a + // non-newobj opcode at byte i whose following 4 bytes coincidentally + // decode to a valid MemberRef token whose ResolveMethod returns a ctor + // of the target type — inside a ~35-byte method body, practically + // impossible. + private static void AssertWarmReachableGraphNewsUp(System.Type expected) + { + var method = typeof(JsonFunctions).GetMethod( + "WarmReachableGraph", + BindingFlags.NonPublic | BindingFlags.Static); + Assert.That(method, Is.Not.Null, + "JsonFunctions.WarmReachableGraph private static method must exist."); + var body = method!.GetMethodBody(); + Assert.That(body, Is.Not.Null, + "WarmReachableGraph must have a method body (not abstract / p-invoke)."); + var il = body!.GetILAsByteArray(); + Assert.That(il, Is.Not.Null); + var module = method.Module; + + for (int i = 0; i + 4 < il!.Length; i++) + { + if (il[i] != 0x73) continue; + int token = System.BitConverter.ToInt32(il, i + 1); + System.Reflection.MethodBase? resolved; + try { resolved = module.ResolveMethod(token); } + catch { continue; } + if (resolved is System.Reflection.ConstructorInfo ctor + && ctor.DeclaringType == expected) + { + return; + } + } + + Assert.Fail( + $"WarmReachableGraph must contain a newobj IL instruction for {expected.FullName}. " + + "Without it, the first request that hits this envelope pays a cold LCG DynamicMethod emit " + + "against the frozen singleton — which is the +3.2-3.8 MB/h RSS leak-in-miniature the warm-up exists to prevent."); } #endif } diff --git a/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs b/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs index 436167c44..4b1ce63e4 100644 --- a/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs +++ b/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs @@ -380,33 +380,94 @@ public void Frozen_singleton_can_serialize_a_real_payload_proving_warm_up_ran_be } /// - /// Warm-up coverage pin (net8+) — MTConnect.Errors.ErrorResponseDocument - /// is the fourth top-level response envelope written directly by + /// Structural warm-up-coverage pin (net8+) — WarmReachableGraph's + /// IL must contain a newobj instruction constructing + /// , the fourth + /// top-level response envelope written directly by /// JsonHttpResponseDocumentFormatter.Format(IErrorResponseDocument, ...) - /// without a cppagent-specific surrogate. The static-ctor - /// WarmReachableGraph pass must serialize an instance of it - /// against both option singletons BEFORE - /// MakeReadOnly(populateMissingResolver: false) — otherwise the - /// first error response (a /probe failure, unsupported device request, - /// or parse error) would pay a cold LCG DynamicMethod emit against - /// a frozen, resolver-less options and throw - /// . Serializing a fresh - /// here reproduces - /// exactly that first-error path against the shared singletons; a - /// regression that dropped the Error warm-up would surface as a - /// NotSupportedException on this test. + /// without a cppagent-specific surrogate wrapper. + /// + /// A runtime Assert.DoesNotThrow(() => Serialize(new ErrorResponseDocument(), frozen)) + /// check is tautological here: once the TypeInfoResolver is set (which + /// any preceding warm-up call does), STJ's + /// lazily populates + /// on frozen options for arbitrary types — the frozen state only locks + /// the options' configuration surface (Converters, DefaultIgnoreCondition, + /// etc.), not the internal metadata cache. Verified empirically: + /// mutating out the ErrorResponseDocument warm-up line and running + /// the "can it serialize" pin still passes (see cycle-5 test-coverage-audit + /// mutation test 2026-08-21). The performance invariant the warm-up + /// enforces — pay the LCG DynamicMethod emit for ErrorResponseDocument + /// at assembly load, not on the first /probe error / parse-failure + /// request — is therefore only observable at the source-of-truth level: + /// the IL of WarmReachableGraph. /// [Test] - public void Frozen_singleton_can_serialize_ErrorResponseDocument_proving_Error_warm_up_ran() + public void WarmReachableGraph_IL_contains_newobj_for_ErrorResponseDocument_ctor() { - var document = new MTConnect.Errors.ErrorResponseDocument(); + AssertWarmReachableGraphNewsUp(typeof(MTConnect.Errors.ErrorResponseDocument)); + } - Assert.DoesNotThrow( - () => JsonSerializer.Serialize(document, JsonFunctions.DefaultOptions), - "Frozen DefaultOptions must have ErrorResponseDocument in its warmed TypeInfoResolver — a regression that removed the Error warm-up would surface here as NotSupportedException on the first error response."); - Assert.DoesNotThrow( - () => JsonSerializer.Serialize(document, JsonFunctions.IndentOptions), - "Frozen IndentOptions mirror — same Error warm-up invariant as DefaultOptions."); + /// + /// Structural warm-up-coverage pin (net8+) — companion assertion that + /// each of the three cppagent top-level response surrogates is also + /// news-up-ed by WarmReachableGraph. Mirrors the ErrorResponseDocument + /// pin above; the same tautology argument applies to per-type + /// "can it serialize" runtime checks. + /// + [Test] + public void WarmReachableGraph_IL_contains_newobj_for_every_top_level_response_surrogate() + { + AssertWarmReachableGraphNewsUp(typeof(MTConnect.Streams.Json.JsonStreamsResponseDocument)); + AssertWarmReachableGraphNewsUp(typeof(MTConnect.Assets.Json.JsonAssetsResponseDocument)); + AssertWarmReachableGraphNewsUp(typeof(MTConnect.Devices.Json.JsonDevicesResponseDocument)); + } + + // IL walker: locate a `newobj ` instruction inside + // JsonFunctions.WarmReachableGraph. Runs on the compiled test assembly's + // view of MTConnect.NET-JSON-cppagent, so a source change that removes + // the corresponding new-expression is caught deterministically at test + // time. Same walker as the plain-JSON sibling fixture; kept per-fixture + // to avoid a shared helper project just for this pin. + // + // Scans for the 5-byte pattern <4-byte-token>. Full + // opcode-length decoding is unnecessary: a spurious match would need a + // non-newobj opcode at byte i whose following 4 bytes coincidentally + // decode to a valid MemberRef token whose ResolveMethod returns a ctor + // of the target type — inside a ~35-byte method body, practically + // impossible. + private static void AssertWarmReachableGraphNewsUp(System.Type expected) + { + var method = typeof(JsonFunctions).GetMethod( + "WarmReachableGraph", + BindingFlags.NonPublic | BindingFlags.Static); + Assert.That(method, Is.Not.Null, + "JsonFunctions.WarmReachableGraph private static method must exist."); + var body = method!.GetMethodBody(); + Assert.That(body, Is.Not.Null, + "WarmReachableGraph must have a method body (not abstract / p-invoke)."); + var il = body!.GetILAsByteArray(); + Assert.That(il, Is.Not.Null); + var module = method.Module; + + for (int i = 0; i + 4 < il!.Length; i++) + { + if (il[i] != 0x73) continue; + int token = System.BitConverter.ToInt32(il, i + 1); + System.Reflection.MethodBase? resolved; + try { resolved = module.ResolveMethod(token); } + catch { continue; } + if (resolved is System.Reflection.ConstructorInfo ctor + && ctor.DeclaringType == expected) + { + return; + } + } + + Assert.Fail( + $"WarmReachableGraph must contain a newobj IL instruction for {expected.FullName}. " + + "Without it, the first request that hits this envelope pays a cold LCG DynamicMethod emit " + + "against the frozen singleton — which is the +3.2-3.8 MB/h RSS leak-in-miniature the warm-up exists to prevent."); } #endif } From 0e3ca95d24fcd4da75ba1f616cac4401045c7566 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 10:14:02 +0200 Subject: [PATCH 11/15] =?UTF-8?q?fix(json,json-cppagent):=20populate=20Err?= =?UTF-8?q?or=20warm-up=20+=20document=20JsonAssetsDocument=20null-toleran?= =?UTF-8?q?ce=20=E2=80=94=20dime=20M+L-C5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cycle-5 leftovers on the JsonSerializerOptions singleton campaign (#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 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 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 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 ; 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 5a196588 (tautological Error-warm-up pin replaced with IL-inspection walker; sibling structural pins added for the five per-call → static-readonly refactors). --- .../JsonFunctions.cs | 23 ++++++++++++++++++- .../Assets/JsonAssetsDocument.cs | 16 +++++++++++++ libraries/MTConnect.NET-JSON/JsonFunctions.cs | 22 +++++++++++++++++- 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs b/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs index 687b0c542..348357b51 100644 --- a/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs +++ b/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs @@ -1,6 +1,7 @@ // Copyright (c) 2024 TrakHound Inc., All Rights Reserved. // TrakHound Inc. licenses this file to you under the MIT license. +using System; using System.IO; using System.Text.Json; using System.Text.Json.Serialization; @@ -103,12 +104,32 @@ static JsonFunctions() // ErrorResponseDocument (from MTConnect.Errors) is the fourth // top-level envelope — no cppagent-specific surrogate, written // by the formatter's IErrorResponseDocument overload directly. + // + // The Error envelope populates a concrete + // MTConnectErrorHeader + Error entry + Version, so STJ walks + // the runtime types the production path actually serializes + // (interface-typed properties resolve to their concrete + // implementations only when the value is non-null); a naked + // `new ErrorResponseDocument()` with null Header / Errors / + // Version would only warm ErrorResponseDocument's own + // accessors, leaving the first real /probe error or parse + // failure to pay a cold LCG emit on MTConnectErrorHeader (9 + // properties), Error (2), and System.Version (6) — the exact + // symptom class the singleton pattern exists to eliminate for + // the error path (F-IMP-C5-001). private static void WarmReachableGraph(JsonSerializerOptions options) { JsonSerializer.Serialize(new Streams.Json.JsonStreamsResponseDocument(), options); JsonSerializer.Serialize(new Assets.Json.JsonAssetsResponseDocument(), options); JsonSerializer.Serialize(new Devices.Json.JsonDevicesResponseDocument(), options); - JsonSerializer.Serialize(new Errors.ErrorResponseDocument(), options); + JsonSerializer.Serialize( + new Errors.ErrorResponseDocument + { + Header = new Headers.MTConnectErrorHeader(), + Errors = new[] { new Errors.Error() }, + Version = new Version(2, 5) + }, + options); } #endif diff --git a/libraries/MTConnect.NET-JSON/Assets/JsonAssetsDocument.cs b/libraries/MTConnect.NET-JSON/Assets/JsonAssetsDocument.cs index e147db4b3..d5bb34909 100644 --- a/libraries/MTConnect.NET-JSON/Assets/JsonAssetsDocument.cs +++ b/libraries/MTConnect.NET-JSON/Assets/JsonAssetsDocument.cs @@ -43,6 +43,22 @@ public class JsonAssetsDocument /// , dispatching each asset to the /// surrogate that matches its type. /// + /// + /// This constructor MUST remain null-tolerant on + /// . The static-ctor + /// warm-up in MTConnect.JsonFunctions.WarmReachableGraph + /// calls new JsonAssetsDocument(null) to force + /// System.Text.Json to emit the LCG DynamicMethod property + /// accessors for the reachable Assets graph at assembly load, + /// avoiding a cold reflection-emit on the first user-facing + /// /assets request. Adding an + /// ArgumentNullException.ThrowIfNull(assetsDocument) + /// guard here would turn the first JSON serialization on + /// process start into a . + /// If the null-tolerance contract must change, update the + /// warm-up site atomically per §1.0d-trigies-bis + /// (F-IMP-C5-002). + /// public JsonAssetsDocument(IAssetsResponseDocument assetsDocument) { if (assetsDocument != null) diff --git a/libraries/MTConnect.NET-JSON/JsonFunctions.cs b/libraries/MTConnect.NET-JSON/JsonFunctions.cs index 73e4762c0..6ca822878 100644 --- a/libraries/MTConnect.NET-JSON/JsonFunctions.cs +++ b/libraries/MTConnect.NET-JSON/JsonFunctions.cs @@ -99,12 +99,32 @@ static JsonFunctions() // ErrorResponseDocument (MTConnect.Errors) is the fourth // top-level response envelope — no surrogate wrapper, written // by the formatter's IErrorResponseDocument overload directly. + // + // The Error envelope populates a concrete + // MTConnectErrorHeader + Error entry + Version, so STJ walks + // the runtime types the production path actually serializes + // (interface-typed properties resolve to their concrete + // implementations only when the value is non-null); a naked + // `new ErrorResponseDocument()` with null Header / Errors / + // Version would only warm ErrorResponseDocument's own + // accessors, leaving the first real /probe error or parse + // failure to pay a cold LCG emit on MTConnectErrorHeader (9 + // properties), Error (2), and System.Version (6) — the exact + // symptom class the singleton pattern exists to eliminate for + // the error path (F-IMP-C5-001). private static void WarmReachableGraph(JsonSerializerOptions options) { JsonSerializer.Serialize(new Streams.Json.JsonStreamsDocument(), options); JsonSerializer.Serialize(new Assets.Json.JsonAssetsDocument(null), options); JsonSerializer.Serialize(new Devices.Json.JsonDevicesDocument(), options); - JsonSerializer.Serialize(new Errors.ErrorResponseDocument(), options); + JsonSerializer.Serialize( + new Errors.ErrorResponseDocument + { + Header = new Headers.MTConnectErrorHeader(), + Errors = new[] { new Errors.Error() }, + Version = new Version(2, 5) + }, + options); } #endif From c013831b9c18c29d1a6fe0b6e5249116af890e5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 10:23:56 +0200 Subject: [PATCH 12/15] =?UTF-8?q?test(json,json-cppagent):=20pin=20populat?= =?UTF-8?q?ed=20Error-envelope=20newobjs=20=E2=80=94=20coverage-FLOOR?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cycle-6 gap: the F-IMP-C5-001 fix (6b081eac) 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 --- .../JsonSerializerOptionsSingletonTests.cs | 33 +++++++++++++++++++ .../JsonSerializerOptionsSingletonTests.cs | 30 +++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs b/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs index ab0a394fc..f345590e1 100644 --- a/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs +++ b/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs @@ -460,6 +460,39 @@ public void WarmReachableGraph_IL_contains_newobj_for_every_top_level_response_s AssertWarmReachableGraphNewsUp(typeof(MTConnect.Devices.Json.JsonDevicesDocument)); } + /// + /// Structural warm-up-coverage pin (net8+) — companion assertion for + /// the F-IMP-C5-001 fix that populated the Error envelope with a + /// concrete + + /// + . + /// STJ resolves accessors for interface-typed properties + /// (IMTConnectErrorHeader Header, IEnumerable<IError> Errors) + /// only when the value is non-null; a revert to a naked + /// new ErrorResponseDocument() would silently pass the + /// ErrorResponseDocument-only IL pin above while re-opening the + /// exact LCG-emit-on-first-error class F-IMP-C5-001 closed — + /// MTConnectErrorHeader (9 properties), Error (2), and + /// System.Version (6) would each pay their first cold accessor + /// emit on the first real /probe error or parse failure. + /// The three newobj instructions below are the source-of-truth + /// evidence that the populated warm-up shape survives. + /// + /// System.Version is not news-up-ed by the three surrogate + /// envelopes' Header defaulting (they use their own concrete + /// MTConnectStreamsHeader / MTConnectDevicesHeader / + /// MTConnectAssetsHeader and initialize the Version property + /// only after construction), so the walker match on + /// System.Version uniquely fingerprints the Error-envelope + /// initializer. + /// + [Test] + public void WarmReachableGraph_IL_contains_newobj_for_concrete_Error_envelope_fields() + { + AssertWarmReachableGraphNewsUp(typeof(MTConnect.Headers.MTConnectErrorHeader)); + AssertWarmReachableGraphNewsUp(typeof(MTConnect.Errors.Error)); + AssertWarmReachableGraphNewsUp(typeof(System.Version)); + } + // IL walker: locate a `newobj ` instruction inside // JsonFunctions.WarmReachableGraph. Runs on the compiled test assembly's // view of MTConnect.NET-JSON, so a source change that removes the diff --git a/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs b/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs index 4b1ce63e4..6831eb42b 100644 --- a/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs +++ b/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs @@ -423,6 +423,36 @@ public void WarmReachableGraph_IL_contains_newobj_for_every_top_level_response_s AssertWarmReachableGraphNewsUp(typeof(MTConnect.Devices.Json.JsonDevicesResponseDocument)); } + /// + /// Structural warm-up-coverage pin (net8+) — companion assertion for + /// the F-IMP-C5-001 fix that populated the Error envelope with a + /// concrete + + /// + . + /// STJ resolves accessors for interface-typed properties + /// (IMTConnectErrorHeader Header, IEnumerable<IError> Errors) + /// only when the value is non-null; a revert to a naked + /// new ErrorResponseDocument() would silently pass the + /// ErrorResponseDocument-only IL pin above while re-opening the + /// exact LCG-emit-on-first-error class F-IMP-C5-001 closed — + /// MTConnectErrorHeader (9 properties), Error (2), and + /// System.Version (6) would each pay their first cold accessor + /// emit on the first real cppagent-formatted error response. The + /// three newobj instructions below are the source-of-truth + /// evidence that the populated warm-up shape survives. + /// + /// System.Version is not news-up-ed by the three cppagent + /// surrogate envelopes' Header defaulting, so the walker match on + /// System.Version uniquely fingerprints the Error-envelope + /// initializer. + /// + [Test] + public void WarmReachableGraph_IL_contains_newobj_for_concrete_Error_envelope_fields() + { + AssertWarmReachableGraphNewsUp(typeof(MTConnect.Headers.MTConnectErrorHeader)); + AssertWarmReachableGraphNewsUp(typeof(MTConnect.Errors.Error)); + AssertWarmReachableGraphNewsUp(typeof(System.Version)); + } + // IL walker: locate a `newobj ` instruction inside // JsonFunctions.WarmReachableGraph. Runs on the compiled test assembly's // view of MTConnect.NET-JSON-cppagent, so a source change that removes From 7c6c4da91218da2cbd297fea53bdd486da8dd88d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 10:26:02 +0200 Subject: [PATCH 13/15] =?UTF-8?q?refactor(json,json-cppagent):=20warm=20Ve?= =?UTF-8?q?rsion=20via=20MTConnectVersions.Version25=20=E2=80=94=20dime=20?= =?UTF-8?q?L-C6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cycle-6 leftover on the JsonSerializerOptions singleton campaign (#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 e19674ec (IL walker now asserts newobj for MTConnectErrorHeader + Error + Version, catching a revert-to-unpopulated regression that cycle-5's ErrorResponseDocument-only pin missed). --- libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs | 9 +++++++-- libraries/MTConnect.NET-JSON/JsonFunctions.cs | 8 +++++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs b/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs index 348357b51..8229d3678 100644 --- a/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs +++ b/libraries/MTConnect.NET-JSON-cppagent/JsonFunctions.cs @@ -1,7 +1,6 @@ // Copyright (c) 2024 TrakHound Inc., All Rights Reserved. // TrakHound Inc. licenses this file to you under the MIT license. -using System; using System.IO; using System.Text.Json; using System.Text.Json.Serialization; @@ -127,7 +126,13 @@ private static void WarmReachableGraph(JsonSerializerOptions options) { Header = new Headers.MTConnectErrorHeader(), Errors = new[] { new Errors.Error() }, - Version = new Version(2, 5) + // Any non-null concrete Version warms the + // System.Version accessors identically; using the + // repo's canonical constant instead of a magic + // `new Version(2, 5)` tuple keeps the warm-up + // aligned with the rest of the codebase's + // MTConnectVersion sourcing (F-CR-C6-001). + Version = MTConnectVersions.Version25 }, options); } diff --git a/libraries/MTConnect.NET-JSON/JsonFunctions.cs b/libraries/MTConnect.NET-JSON/JsonFunctions.cs index 6ca822878..d7f9ecee6 100644 --- a/libraries/MTConnect.NET-JSON/JsonFunctions.cs +++ b/libraries/MTConnect.NET-JSON/JsonFunctions.cs @@ -122,7 +122,13 @@ private static void WarmReachableGraph(JsonSerializerOptions options) { Header = new Headers.MTConnectErrorHeader(), Errors = new[] { new Errors.Error() }, - Version = new Version(2, 5) + // Any non-null concrete Version warms the + // System.Version accessors identically; using the + // repo's canonical constant instead of a magic + // `new Version(2, 5)` tuple keeps the warm-up + // aligned with the rest of the codebase's + // MTConnectVersion sourcing (F-CR-C6-001). + Version = MTConnectVersions.Version25 }, options); } From 5c34a16a4b9bb92b1d9e52d3bcededb0cf348506 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Fri, 21 Aug 2026 10:29:00 +0200 Subject: [PATCH 14/15] =?UTF-8?q?test(json,json-cppagent):=20IL=20walker?= =?UTF-8?q?=20accepts=20ldsfld=20producers=20=E2=80=94=20chase=20F-CR-C6-0?= =?UTF-8?q?01?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The F-CR-C6-001 refactor (ec86e338) 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 `. 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 `` 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). --- .../JsonSerializerOptionsSingletonTests.cs | 92 +++++++++++++------ .../JsonSerializerOptionsSingletonTests.cs | 90 ++++++++++++------ 2 files changed, 129 insertions(+), 53 deletions(-) diff --git a/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs b/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs index f345590e1..d61c849eb 100644 --- a/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs +++ b/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs @@ -474,16 +474,28 @@ public void WarmReachableGraph_IL_contains_newobj_for_every_top_level_response_s /// MTConnectErrorHeader (9 properties), Error (2), and /// System.Version (6) would each pay their first cold accessor /// emit on the first real /probe error or parse failure. - /// The three newobj instructions below are the source-of-truth - /// evidence that the populated warm-up shape survives. + /// The three assertions below are the source-of-truth evidence + /// that the populated warm-up shape survives. /// - /// System.Version is not news-up-ed by the three surrogate - /// envelopes' Header defaulting (they use their own concrete - /// MTConnectStreamsHeader / MTConnectDevicesHeader / - /// MTConnectAssetsHeader and initialize the Version property - /// only after construction), so the walker match on - /// System.Version uniquely fingerprints the Error-envelope - /// initializer. + /// The walker () + /// accepts either a newobj producing an instance of the + /// expected type OR a ldsfld loading a static field of + /// that type — semantically equivalent for warm-up, because STJ + /// walks the runtime type of whatever value the caller passes + /// into + /// regardless of how the instance was produced. The current + /// warm-up uses MTConnectVersions.Version25 (a static + /// field of type ) rather than a + /// magic new System.Version(2, 5) literal (F-CR-C6-001); + /// dropping either producer would fail the pin. + /// + /// System.Version is not news-up-ed OR ldsfld-loaded by the + /// three surrogate envelopes' Header defaulting (they use their + /// own concrete MTConnectStreamsHeader / + /// MTConnectDevicesHeader / MTConnectAssetsHeader + /// and initialize the Version property only after construction), + /// so the walker match on System.Version uniquely + /// fingerprints the Error-envelope initializer. /// [Test] public void WarmReachableGraph_IL_contains_newobj_for_concrete_Error_envelope_fields() @@ -493,17 +505,27 @@ public void WarmReachableGraph_IL_contains_newobj_for_concrete_Error_envelope_fi AssertWarmReachableGraphNewsUp(typeof(System.Version)); } - // IL walker: locate a `newobj ` instruction inside - // JsonFunctions.WarmReachableGraph. Runs on the compiled test assembly's - // view of MTConnect.NET-JSON, so a source change that removes the - // corresponding new-expression is caught deterministically at test time. + // IL walker: proves JsonFunctions.WarmReachableGraph feeds a concrete + // instance of to STJ by locating EITHER a + // `newobj ` (fresh allocation) OR a + // `ldsfld ` (canonical-constant load) + // in the method body. Runs on the compiled test assembly's view of + // MTConnect.NET-JSON, so a source change that removes both producers + // is caught deterministically at test time. // - // Scans for the 5-byte pattern <4-byte-token>. Full - // opcode-length decoding is unnecessary: a spurious match would need a - // non-newobj opcode at byte i whose following 4 bytes coincidentally - // decode to a valid MemberRef token whose ResolveMethod returns a ctor - // of the target type — inside a ~35-byte method body, practically + // Scans for the 5-byte patterns <4-byte-token> and + // <4-byte-token>. Full opcode-length decoding is + // unnecessary: a spurious match would need a non-target opcode at + // byte i whose following 4 bytes coincidentally decode to a valid + // MemberRef token whose ResolveMethod/ResolveField returns a target + // of the expected type — inside a ~40-byte method body, practically // impossible. + // + // Both producers are semantically equivalent for warm-up: STJ walks + // the runtime type of the value passed into Serialize, and cannot + // tell whether the instance came from a fresh newobj or from a + // static field load (e.g. MTConnectVersions.Version25 for + // System.Version). private static void AssertWarmReachableGraphNewsUp(System.Type expected) { var method = typeof(JsonFunctions).GetMethod( @@ -520,20 +542,36 @@ private static void AssertWarmReachableGraphNewsUp(System.Type expected) for (int i = 0; i + 4 < il!.Length; i++) { - if (il[i] != 0x73) continue; - int token = System.BitConverter.ToInt32(il, i + 1); - System.Reflection.MethodBase? resolved; - try { resolved = module.ResolveMethod(token); } - catch { continue; } - if (resolved is System.Reflection.ConstructorInfo ctor - && ctor.DeclaringType == expected) + if (il[i] == 0x73) + { + // newobj + int token = System.BitConverter.ToInt32(il, i + 1); + System.Reflection.MethodBase? resolved; + try { resolved = module.ResolveMethod(token); } + catch { continue; } + if (resolved is System.Reflection.ConstructorInfo ctor + && ctor.DeclaringType == expected) + { + return; + } + } + else if (il[i] == 0x7E) { - return; + // ldsfld + int token = System.BitConverter.ToInt32(il, i + 1); + System.Reflection.FieldInfo? field; + try { field = module.ResolveField(token); } + catch { continue; } + if (field != null && field.FieldType == expected) + { + return; + } } } Assert.Fail( - $"WarmReachableGraph must contain a newobj IL instruction for {expected.FullName}. " + + $"WarmReachableGraph must produce a concrete instance of {expected.FullName} " + + "(via `new` or via a static-field load) so STJ walks its accessor graph at assembly load. " + "Without it, the first request that hits this envelope pays a cold LCG DynamicMethod emit " + "against the frozen singleton — which is the +3.2-3.8 MB/h RSS leak-in-miniature the warm-up exists to prevent."); } diff --git a/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs b/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs index 6831eb42b..be26ef771 100644 --- a/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs +++ b/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs @@ -437,13 +437,25 @@ public void WarmReachableGraph_IL_contains_newobj_for_every_top_level_response_s /// MTConnectErrorHeader (9 properties), Error (2), and /// System.Version (6) would each pay their first cold accessor /// emit on the first real cppagent-formatted error response. The - /// three newobj instructions below are the source-of-truth - /// evidence that the populated warm-up shape survives. + /// three assertions below are the source-of-truth evidence that + /// the populated warm-up shape survives. /// - /// System.Version is not news-up-ed by the three cppagent - /// surrogate envelopes' Header defaulting, so the walker match on - /// System.Version uniquely fingerprints the Error-envelope - /// initializer. + /// The walker () + /// accepts either a newobj producing an instance of the + /// expected type OR a ldsfld loading a static field of + /// that type — semantically equivalent for warm-up, because STJ + /// walks the runtime type of whatever value the caller passes + /// into + /// regardless of how the instance was produced. The current + /// warm-up uses MTConnectVersions.Version25 (a static + /// field of type ) rather than a + /// magic new System.Version(2, 5) literal (F-CR-C6-001); + /// dropping either producer would fail the pin. + /// + /// System.Version is not news-up-ed OR ldsfld-loaded by the + /// three cppagent surrogate envelopes' Header defaulting, so + /// the walker match on System.Version uniquely + /// fingerprints the Error-envelope initializer. /// [Test] public void WarmReachableGraph_IL_contains_newobj_for_concrete_Error_envelope_fields() @@ -453,19 +465,29 @@ public void WarmReachableGraph_IL_contains_newobj_for_concrete_Error_envelope_fi AssertWarmReachableGraphNewsUp(typeof(System.Version)); } - // IL walker: locate a `newobj ` instruction inside - // JsonFunctions.WarmReachableGraph. Runs on the compiled test assembly's - // view of MTConnect.NET-JSON-cppagent, so a source change that removes - // the corresponding new-expression is caught deterministically at test - // time. Same walker as the plain-JSON sibling fixture; kept per-fixture - // to avoid a shared helper project just for this pin. + // IL walker: proves JsonFunctions.WarmReachableGraph feeds a concrete + // instance of to STJ by locating EITHER a + // `newobj ` (fresh allocation) OR a + // `ldsfld ` (canonical-constant load) + // in the method body. Runs on the compiled test assembly's view of + // MTConnect.NET-JSON-cppagent, so a source change that removes both + // producers is caught deterministically at test time. Same walker + // shape as the plain-JSON sibling fixture; kept per-fixture to avoid + // a shared helper project just for this pin. // - // Scans for the 5-byte pattern <4-byte-token>. Full - // opcode-length decoding is unnecessary: a spurious match would need a - // non-newobj opcode at byte i whose following 4 bytes coincidentally - // decode to a valid MemberRef token whose ResolveMethod returns a ctor - // of the target type — inside a ~35-byte method body, practically + // Scans for the 5-byte patterns <4-byte-token> and + // <4-byte-token>. Full opcode-length decoding is + // unnecessary: a spurious match would need a non-target opcode at + // byte i whose following 4 bytes coincidentally decode to a valid + // MemberRef token whose ResolveMethod/ResolveField returns a target + // of the expected type — inside a ~40-byte method body, practically // impossible. + // + // Both producers are semantically equivalent for warm-up: STJ walks + // the runtime type of the value passed into Serialize, and cannot + // tell whether the instance came from a fresh newobj or from a + // static field load (e.g. MTConnectVersions.Version25 for + // System.Version). private static void AssertWarmReachableGraphNewsUp(System.Type expected) { var method = typeof(JsonFunctions).GetMethod( @@ -482,20 +504,36 @@ private static void AssertWarmReachableGraphNewsUp(System.Type expected) for (int i = 0; i + 4 < il!.Length; i++) { - if (il[i] != 0x73) continue; - int token = System.BitConverter.ToInt32(il, i + 1); - System.Reflection.MethodBase? resolved; - try { resolved = module.ResolveMethod(token); } - catch { continue; } - if (resolved is System.Reflection.ConstructorInfo ctor - && ctor.DeclaringType == expected) + if (il[i] == 0x73) + { + // newobj + int token = System.BitConverter.ToInt32(il, i + 1); + System.Reflection.MethodBase? resolved; + try { resolved = module.ResolveMethod(token); } + catch { continue; } + if (resolved is System.Reflection.ConstructorInfo ctor + && ctor.DeclaringType == expected) + { + return; + } + } + else if (il[i] == 0x7E) { - return; + // ldsfld + int token = System.BitConverter.ToInt32(il, i + 1); + System.Reflection.FieldInfo? field; + try { field = module.ResolveField(token); } + catch { continue; } + if (field != null && field.FieldType == expected) + { + return; + } } } Assert.Fail( - $"WarmReachableGraph must contain a newobj IL instruction for {expected.FullName}. " + + $"WarmReachableGraph must produce a concrete instance of {expected.FullName} " + + "(via `new` or via a static-field load) so STJ walks its accessor graph at assembly load. " + "Without it, the first request that hits this envelope pays a cold LCG DynamicMethod emit " + "against the frozen singleton — which is the +3.2-3.8 MB/h RSS leak-in-miniature the warm-up exists to prevent."); } From d0f770e8beb7f68c255fc03fb11c993ea5ccfa53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Otto=20Boly=C3=B3s?= Date: Sat, 22 Aug 2026 12:17:04 +0200 Subject: [PATCH 15/15] chore(json-tests): disambiguate cref on JsonSerializerOptionsSingletonTests --- .../Regressions/JsonSerializerOptionsSingletonTests.cs | 2 +- .../Regressions/JsonSerializerOptionsSingletonTests.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs b/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs index d61c849eb..aa9728b52 100644 --- a/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs +++ b/tests/MTConnect.NET-JSON-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs @@ -482,7 +482,7 @@ public void WarmReachableGraph_IL_contains_newobj_for_every_top_level_response_s /// expected type OR a ldsfld loading a static field of /// that type — semantically equivalent for warm-up, because STJ /// walks the runtime type of whatever value the caller passes - /// into + /// into /// regardless of how the instance was produced. The current /// warm-up uses MTConnectVersions.Version25 (a static /// field of type ) rather than a diff --git a/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs b/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs index be26ef771..248d63dad 100644 --- a/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs +++ b/tests/MTConnect.NET-JSON-cppagent-Tests/Regressions/JsonSerializerOptionsSingletonTests.cs @@ -445,7 +445,7 @@ public void WarmReachableGraph_IL_contains_newobj_for_every_top_level_response_s /// expected type OR a ldsfld loading a static field of /// that type — semantically equivalent for warm-up, because STJ /// walks the runtime type of whatever value the caller passes - /// into + /// into /// regardless of how the instance was produced. The current /// warm-up uses MTConnectVersions.Version25 (a static /// field of type ) rather than a