[Host] MessageProcessor never disposes object returned by IMessageSerializer.Deserialize
Closed this issue · 2 comments
Problem
MessageProvider calls delegate MessageProvider<TTransportMessage> messageProvider to deserialize a message, but never disposes of the object if it is an IDisposable / IAsyncDisposable.
While this is not an issue for any of the currently bundled serializers, it could pose an issue with third party serializers.
Proposal
Once the message has been processed, MessageProcessor<TTransportMessage>.ProcessMessage() should dispose of the object if required.
The situation is complicated, however, as the message is returned to the calling method as part of the ProcessMessageResult object. The returned instance is not used outside of the test harness currently and could be removed allowing ProcessMessage to retain ownership.
Example use case: Deserialization as JsonDocument
In the scenario where a JSON message is sourced from an external system that requires polymorphic deserialization, a custom IMessgeSerializer could be written to deserialize the byte[] as a JsonDocument.
A workaround in the this scenario is to deserialize to either a string or pass on the byte[] and deserialize it again in the consumer where the lifetime can be managed.
@EtherZa do you want to take a stab at this or you prefer for me to look at this?
I believe checking if the deserialized message implements IDisposable or IAsyncDosposable calling it after message processing would be required here (should be easy (?) to add if I understand).
@EtherZa do you want to take a stab at this or you prefer for me to look at this?
I believe checking if the deserialized message implements IDisposable or IAsyncDosposable calling it after message processing would be required here (should be easy (?) to add if I understand).
@zarusz I've submitted a quick PR (#258) for this. It is as you describe, but as the object was passed back to the caller as part of the ProcessMessageResult, I have had to remove the reference/property in ProcessMessageResult. Fortunately, this was only used as verification in four of the unit tests, none of which will be impeded by its removal.