Skip to content

Fix NativeAOT round-trip for tm7 file I/O#3

Merged
gholliday merged 1 commit into
mainfrom
aot-dictionary-roundtrip
May 28, 2026
Merged

Fix NativeAOT round-trip for tm7 file I/O#3
gholliday merged 1 commit into
mainfrom
aot-dictionary-roundtrip

Conversation

@gholliday

Copy link
Copy Markdown
Owner

Problem

The published NativeAOT exe could neither read nor write .tm7 files. The reflection-based DataContractSerializer hits three AOT-incompatible patterns under .NET 10:

  1. Dictionary<,> [DataMember] fields — DCS fell back to the ISerializable path on Dictionary<Guid,object> / Dictionary<string,SerializableThreat> / Dictionary<string,string> and tried to instantiate the internal System.Runtime.Serialization.KeyValue<K,V> for each closed generic. AOT cannot generate code for those instantiations.
  2. Nullable member serialization — DCS's reflection writer invokes GetDefaultValue1 via MakeGenericMethod, which is fundamentally AOT-incompatible.
  3. EmitDefaultValue=false on double/string members — same MakeGenericMethod path through XmlObjectSerializerWriteContext.GetDefaultValue1.

Fix

Model refactor

  • New SerializableKvpTypes.cs with three [DataContract] KVP types + matching [CollectionDataContract] List wrappers. Element names match what DCS auto-generates (KeyValueOfguidanyType, KeyValueOfstringThreatpc_P0_PhOB, KeyValueOfstringstring) so wire compatibility with TMT is preserved.
  • Borders / Lines / AllThreatsDictionary / SerializableThreat.Properties marked [IgnoreDataMember] and each backed by a private [DataMember] list field synced via [OnSerializing]/[OnDeserialized] callbacks. The public Dictionary<,> API surface is unchanged — no consumer touched.
  • bool? ThreatGenerationEnabledbool (constructor still accepts bool?; coerces null → false).
  • Removed EmitDefaultValue=false from Zoom / StrokeThickness / StrokeDashArray.

Build / serializer plumbing

  • Switch IL3050;IL3053 from NoWarn to WarningsNotAsErrors so BCL DCS warnings stay visible without failing the build.
  • Drop dead string-typed DynamicDependency entries for KeyValue<,> closed generics (failed approach). Keep Dictionary<,>/EqualityComparer<,> preservation as defensive against future [DataMember] Dictionary<,> regressions.

Tests + CI

Test Purpose
RoundTrip_StructuralEquivalence Replaces the byte-for-byte test (no longer achievable).
Roundtrip_PopulatedThreatInstances_PreservesEntries Populates AllThreatsDictionary, verifies full state round-trip.
Roundtrip_PopulatedThreatProperties_PreservesEntries Dictionary<string,string> including empty-string edge case.
Roundtrip_PopulatedThreats_WireFormatMatchesAutoDictionary Compares our KVP element name vs raw Dictionary<,>. Caught a wrong-name guess during development.
Roundtrip_PopulatedThreatProperties_WireFormatMatchesAutoDictionary Same for Dictionary<string,string>.
Roundtrip_Borders_WireFormatMatchesAutoDictionary Same for Dictionary<Guid,object>.
Roundtrip_MultipleAddRemoveCycles Multiple mutate-save-reload cycles.
Serialize_NullDictionaries_DoesNotThrow Guards the null-guard fix.
AotExe_Open_LoadsTemplate Invokes published AOT exe.
AotExe_AddEntity_RoundTripsAndPreservesExistingEntities Mutate via AOT, verify via JIT.
AotExe_PopulatedThreatsRoundTripThroughAddCommand Most important AOT guard: JIT-writes populated threats → AOT-loads/modifies/saves → JIT-reloads → asserts preservation.

Added a Publish AOT step to .github/workflows/ci.yml ahead of Test so the integration tests actually run on every matrix leg (ubuntu / windows / macos). Tests use RuntimeInformation.RuntimeIdentifier to locate the platform-correct exe.

Total: 17 → 20 tests, all passing.

Caveats

  • TMT desktop interop not auto-verified. Recommend a one-time manual check that an AOT-written .tm7 opens in the actual TMT app — the wire format diverges slightly because EmitDefaultValue=false is gone and ThreatGenerationEnabled is no longer nullable. DCS readers are tolerant; TMT-specific parsing isn't proven.
  • The EmitDefaultValue=false removal cannot be worked around without AOT supporting MakeGenericMethod on BCL-internal DCS methods.

Verification

  • All 20 tests pass under dotnet test.
  • Manual smoke test: tm7.exe open / new / add entity all succeed against samples/template.tm7 using the published AOT binary.

The published AOT exe could neither read nor write .tm7 files: the
reflection-based DataContractSerializer hit three AOT-incompatible
patterns (Dictionary<,> falling back to ISerializable, Nullable<T>
member serialization via MakeGenericMethod, and EmitDefaultValue=false
calling GetDefaultValue<T>()).

Model refactor:
- Add SerializableKvpTypes.cs with three [DataContract] KVP types plus
  matching [CollectionDataContract] List wrappers; element names match
  what DCS auto-generates (KeyValueOfguidanyType, KeyValueOfstringThreat-
  pc_P0_PhOB, KeyValueOfstringstring) so wire compat with TMT is preserved.
- Mark Borders / Lines / AllThreatsDictionary / SerializableThreat.Properties
  [IgnoreDataMember] and back each with a private [DataMember] List<Kvp>
  field synced via [OnSerializing] / [OnDeserialized] callbacks. Public
  Dictionary<,> API surface unchanged - no consumer touched.
- Change bool? ThreatGenerationEnabled to bool (Nullable<T> reflection-
  writer path is incompatible with AOT). Constructor still accepts bool?
  and coerces null to false.
- Drop EmitDefaultValue=false from Zoom / StrokeThickness / StrokeDashArray
  (DCS's GetDefaultValue<T> uses MakeGenericMethod which AOT cannot
  generate). Wire bytes now include these fields with default/null values.

Build / serializer plumbing:
- Switch IL3050/IL3053 from NoWarn to WarningsNotAsErrors so the BCL DCS
  warnings remain visible without failing the build.
- Drop the dead string-typed DynamicDependency entries for KeyValue<,>
  closed generics (a failed experimental approach) and the analogous
  trimmer-root entry. Keep Dictionary<,> / EqualityComparer<,>
  preservation as defensive in case a future regression re-introduces a
  [DataMember] Dictionary<,>.

Tests / CI:
- Replace the byte-for-byte round-trip test (no longer achievable) with
  a structural-equivalence test.
- Add PopulatedDictionaryTests covering populated AllThreatsDictionary,
  populated SerializableThreat.Properties, multiple add/remove cycles,
  null-dictionary serialization, and a wire-format guard that compares
  our KVP element names against DCS-generated names for the equivalent
  raw Dictionary<,>. The wire-format test caught a wrong-name guess
  during development.
- Add AotExeIntegrationTests that shell out to the published AOT exe and
  validate populated-threat round-trip through the reflection writer
  (the JIT in-proc tests use code-gen and cannot exercise that path).
  Self-skip when the publish output is absent.
- Add a Publish AOT step to the CI workflow ahead of Test so the
  integration tests actually run on every matrix leg.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@gholliday gholliday merged commit 40b88c3 into main May 28, 2026
3 checks passed
@gholliday gholliday deleted the aot-dictionary-roundtrip branch May 28, 2026 04:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant