StratisBitcoinFullNode
StratisBitcoinFullNode copied to clipboard
Ensure disposing methods doesn't throw without handling the disposal of inner disposable objects
We must be careful when implementing a Dispose() method or when we need to dispose inner disposable objects, because if we don't handle a potential throw, we may end up having disposable object not being able to be collected, or worse.
as an example, assume we have something like this:
public AsyncExecutionEvent<INetworkPeer, NetworkPeerState> StateChanged { get; private set; }
public AsyncExecutionEvent<INetworkPeer, IncomingMessage> MessageReceived { get; private set; }
where the class AsyncExecutionEvent is disposable and then we dispose manually. Now I'm not saying that we have problem in this specific code, but that if the Dispose method of the parent that should call MessageReceived.Dispose() fails, it will never been disposed.
I think as a first step we should trap all the Dispose() method of containers in such a way that in the catch we can dump a log.error that's important to see, so we can fix eventually the code
something like
///this is the Dispose of the container of other disposable objects
public void Dispose()
{
try
{
//do your disposing stuffs, calling MessageReceived.Dispose() or whatever
}
catch
{
log.Critical("DAMN, fix this");
throw
}
}
would be better maybe to have a kind of registration process for disposable resources, so we can add a finally
in the try/catch block that check if we disposed all the disposable we had to dispose.