-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix preserved reference surrogate in collection deserialization bug #264
Fix preserved reference surrogate in collection deserialization bug #264
Conversation
Problem was caused by wrongly stored reference in DeserializeSession: #263 (comment) |
@@ -65,6 +65,15 @@ public object GetDeserializedObject(int id) | |||
return _objectById[id]; | |||
} | |||
|
|||
public void ReplaceOrAddTrackedDeserializedObject([NotNull] object origin, [NotNull] object replacement) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method will search the current reference array for surrogated object instance and replace it with the actual object reference value
@@ -37,6 +42,8 @@ public override object ReadValue(Stream stream, DeserializerSession session) | |||
{ | |||
var surrogateValue = _surrogateSerializer.ReadValue(stream, session); | |||
var value = _translator(surrogateValue); | |||
if(_preserveObjectReferences) | |||
session.ReplaceOrAddTrackedDeserializedObject(surrogateValue, value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace the wrongly injected surrogate object with the actual deserialized de-surrogated value.
Value might be wrongly stored by _surrogateSerializer.ReadValue() in line 43.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Boy this must have been a fun one to track down.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - can't see this having any wire format impact since it's all read-side.
@@ -37,6 +42,8 @@ public override object ReadValue(Stream stream, DeserializerSession session) | |||
{ | |||
var surrogateValue = _surrogateSerializer.ReadValue(stream, session); | |||
var value = _translator(surrogateValue); | |||
if(_preserveObjectReferences) | |||
session.ReplaceOrAddTrackedDeserializedObject(surrogateValue, value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Boy this must have been a fun one to track down.
Closes #263