Fix Owner reference-cycle handling across all serializers - #1107
Open
fuffc wants to merge 5 commits into
Open
Conversation
The per-item JsonConverterList restarted reference tracking on every element, breaking $id/$ref across list boundaries. Replace it with a JsonTypeInfo resolver modifier that only overrides list instantiation, leaving the built-in enumerable path in control of the whole graph.
ListTypeConverter walked the deserialized tree with no cycle guard, recursing forever once the Owner back-reference was restored. Add a reference-equality visited set; the walk is idempotent.
protobuf-net 3.x cannot preserve references, so Owner/OriginalOwner are excluded from the wire. ListTypeConverter now rebuilds Owner from tree containment and reconciles ActiveDockable/DefaultDockable/FocusedDockable by Id, and picks up the Xml/Yaml visited-set cycle guard.
protobuf-net dispatches through the CLR base-class chain, but subtype registration only considered the closest interface. Register each class under its nearest tracked base class too.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix Owner reference-cycle handling across all serializers
Factory.InitLayoutwires each dockable'sOwnerback-reference, producing a cyclic object graph (Dockable.Owner → Dock → VisibleDockables → Dockable). Saving and reloading such a layout was broken in four of the five serializers — ranging from silent identity duplication to process-killing crashes. This PR fixes each one and adds round-trip regression coverage.What was broken
JsonConverterListrestarted STJ's reference tracking on every element, breaking$id/$refacross list boundaries and duplicatingActiveDockableidentity.ListTypeConverter's post-deserialize tree walk had no cycle guard, so once theOwnerback-reference was restored it recursed forever — an uncatchableStackOverflowExceptionthat kills the process.AsReferenceis obsolete-as-error), so the cycle either threw or came back with duplicate instances. A separate"Unexpected sub-type"failure also broke any customDockablesubclass defined outside theDock.Model.*assemblies.Changes
JsonConverterList/JsonConverterFactoryListwithDockListTypeInfoModifier, aJsonTypeInforesolver modifier that only overrides list instantiation, leaving STJ's built-in enumerable path (and its single ambient reference-tracking state) in control of the whole graph.ListTypeConverter; the walk is idempotent, so visiting each instance once is sufficient.Owner/OriginalOwnerfrom the wire;ListTypeConverterrebuildsOwnerstructurally from tree containment (noInitLayoutcall required, matching the other serializers) and reconcilesActiveDockable/DefaultDockable/FocusedDockablebyId, guarding against null/duplicateIds.IList<IDockable>andIList<IDockWindow>creators so trimmed source-generated applications retain the required constructors.Testing
DockLayoutReferencePreservationTestsround-trips a wired layout through all five serializers and assertsActiveDockableidentity is preserved.Ownerrebuild and custom-subtype round-tripping.Closes #1104
Closes #1105