The MakeProxyServer function is not checking if the connection has been broken before trying to create a new ProxyServer object referencing the Connection object.
This seems to cause a bitcoin-node crash reported by plebhash in bitcoin/bitcoin#33554 (comment), where a custom RPC client makes a Mining.waitNext call, and then exits, breaking the connection before the waitNext call finishes executing. When the waitNext call does later finish and return, the MakeProxyServer function is called to make a ProxyServer<BlockTemplate> object to return to the client. But construction of this object aborts while accessing the Connection object which was already deleted.
It would be good to write a libmultiprocess unit test triggering this problem, and then implement a fix to prevent it from happening.
I think in order to fix this, the InvokeContext struct could be changed to contain Connection* pointer instead of a Connection& reference. Then, the MakeProxyServer function could return null if the connection is null. The mp.Context PassField function could use the Connection::addSyncCleanup method to actually set the connection pointer to null when the connection is closed. The InvokeContext struct might also need to have an optional Waiter pointer added for synchronization since addSyncCleanup cleanups run on the event loop thread but MakeProxyServer method runs on the waitNext worker thread.
The
MakeProxyServerfunction is not checking if the connection has been broken before trying to create a newProxyServerobject referencing theConnectionobject.This seems to cause a
bitcoin-nodecrash reported by plebhash in bitcoin/bitcoin#33554 (comment), where a custom RPC client makes aMining.waitNextcall, and then exits, breaking the connection before the waitNext call finishes executing. When thewaitNextcall does later finish and return, theMakeProxyServerfunction is called to make aProxyServer<BlockTemplate>object to return to the client. But construction of this object aborts while accessing theConnectionobject which was already deleted.It would be good to write a libmultiprocess unit test triggering this problem, and then implement a fix to prevent it from happening.
I think in order to fix this, the
InvokeContextstruct could be changed to containConnection*pointer instead of aConnection&reference. Then, theMakeProxyServerfunction could return null if the connection is null. The mp.ContextPassFieldfunction could use theConnection::addSyncCleanupmethod to actually set the connection pointer to null when the connection is closed. TheInvokeContextstruct might also need to have an optionalWaiterpointer added for synchronization sinceaddSyncCleanupcleanups run on the event loop thread butMakeProxyServermethod runs on thewaitNextworker thread.