Add shared_ptr callback support #34
Merged
Conversation
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
Arguments to ProxyClientBase constructor were out of date and this PassField overload didn't have test coverage or usage in bitcoin
…cy with CustomBuildField
ryanofsky
added a commit
to ryanofsky/libmultiprocess
that referenced
this pull request
Jun 11, 2025
… disconnects during IPC call Failure is described in the unit test comment. Log output confirms cause of issue is that the `m_rpc_system.reset()` call which normally deletes ProxyServer objects right away will not delete them right away if an IPC call is in progress. This was the cause suspected in the bitcoin-core#182 issue description but not previously confirmed. Fixing this will probably involve adding per-connection refcounts to Connection objects and not deleting them until the last associated ProxyServer object is destroyed. Test logs below show the ~ProxyServerBase destructor being called after both the client and server ~Connection destructors: LOG0: {mptest-386261/mptest-386271} IPC server recv request bitcoin-core#34 FooInterface.callFnAsync$Params (context = (thread = <external capability>, callbackThread = <external capability>)) LOG0: {mptest-386261/mptest-386271} IPC server post request bitcoin-core#34 {mptest-386261/mptest-386272 (from mptest-386261/mptest-386261)} LOG0: {mptest-386261/mptest-386272 (from mptest-386261/mptest-386261)} |||| Server about to disconnect LOG0: {mptest-386261/mptest-386271} &&&& starting client_connection.reset() LOG0: {mptest-386261/mptest-386271} &&&& ~Connection RPC system reset LOG0: {mptest-386261/mptest-386271} &&&& done client_connection.reset() LOG0: {mptest-386261/mptest-386272 (from mptest-386261/mptest-386261)} |||| Server disconnected LOG1: IPC client method call interrupted by disconnect. >>>> Signalling server to return. >>>> Waiting for disconnect. LOG0: {mptest-386261/mptest-386272 (from mptest-386261/mptest-386261)} |||| Server returned LOG0: {mptest-386261/mptest-386271} &&&& ~Connection RPC system reset LOG0: {mptest-386261/mptest-386271} IPC server send response bitcoin-core#34 FooInterface.callFnAsync$Results () LOG0: {mptest-386261/mptest-386271} IPC server destroy N2mp11ProxyServerINS_4test8messages12FooInterfaceEEE LOG0: {mptest-386261/mptest-386271} &&&& ~ProxyServerBase async *** Fatal uncaught std::exception: Invalid argument stack: 5599ff3b020d 5599ff510313 5599ff50f7e0 5599ff50f15e 5599ff50f19b 5599ff50f1c8 5599ff3b592a 7f817bb65ed1 7f817bb65fe2 7f817bb6994d 7f817b99386f 7f817b993c1c 7f817b997cb1 7f817b990d67 5599ff5199c7 5599ff5151dd 5599ff3b37ab 5599ff3b33d4 5599ff3b3394 5599ff3b336c 5599ff3b3344 5599ff3b3278 7f817b6ed063 7f817b297e62 7f817b31bdbb
ryanofsky
added a commit
to ryanofsky/libmultiprocess
that referenced
this pull request
Jun 13, 2025
… disconnects during IPC call Failure is described in the unit test comment. Log output confirms cause of issue is that the `m_rpc_system.reset()` call which normally deletes ProxyServer objects right away will not delete them right away if an IPC call is in progress. This was the cause suspected in the bitcoin-core#182 issue description but not previously confirmed. Fixing this will probably involve adding per-connection refcounts to Connection objects and not deleting them until the last associated ProxyServer object is destroyed. Test logs below show the ~ProxyServerBase destructor being called after both the client and server ~Connection destructors: LOG0: {mptest-386261/mptest-386271} IPC server recv request bitcoin-core#34 FooInterface.callFnAsync$Params (context = (thread = <external capability>, callbackThread = <external capability>)) LOG0: {mptest-386261/mptest-386271} IPC server post request bitcoin-core#34 {mptest-386261/mptest-386272 (from mptest-386261/mptest-386261)} LOG0: {mptest-386261/mptest-386272 (from mptest-386261/mptest-386261)} |||| Server about to disconnect LOG0: {mptest-386261/mptest-386271} &&&& starting client_connection.reset() LOG0: {mptest-386261/mptest-386271} &&&& ~Connection RPC system reset LOG0: {mptest-386261/mptest-386271} &&&& done client_connection.reset() LOG0: {mptest-386261/mptest-386272 (from mptest-386261/mptest-386261)} |||| Server disconnected LOG1: IPC client method call interrupted by disconnect. >>>> Signalling server to return. >>>> Waiting for disconnect. LOG0: {mptest-386261/mptest-386272 (from mptest-386261/mptest-386261)} |||| Server returned LOG0: {mptest-386261/mptest-386271} &&&& ~Connection RPC system reset LOG0: {mptest-386261/mptest-386271} IPC server send response bitcoin-core#34 FooInterface.callFnAsync$Results () LOG0: {mptest-386261/mptest-386271} IPC server destroy N2mp11ProxyServerINS_4test8messages12FooInterfaceEEE LOG0: {mptest-386261/mptest-386271} &&&& ~ProxyServerBase async *** Fatal uncaught std::exception: Invalid argument stack: 5599ff3b020d 5599ff510313 5599ff50f7e0 5599ff50f15e 5599ff50f19b 5599ff50f1c8 5599ff3b592a 7f817bb65ed1 7f817bb65fe2 7f817bb6994d 7f817b99386f 7f817b993c1c 7f817b997cb1 7f817b990d67 5599ff5199c7 5599ff5151dd 5599ff3b37ab 5599ff3b33d4 5599ff3b3394 5599ff3b336c 5599ff3b3344 5599ff3b3278 7f817b6ed063 7f817b297e62 7f817b31bdbb
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This is needed after bitcoin/bitcoin#18338 which changed
handleNotifications()Notifications&callback argument tostd::shared_ptr<Notifications>.Easiest way to support this was to change
ProxyServerBasereference from a raw pointer to shared pointer and make necessary BuildField changes to pass along theshared_ptr. Alternative might have been to add more generic cleanup support toProxyServerBaseinstead of hardcodingshared_ptr.The change also required making the ReadField callback overload more generic, which was a straightforward but kind of big change that touched a lot of code.
There weren't any unit tests for callback support previously, so a lot of new test coverage was added. It includes coverage for
shared_ptrlifetime correctness, making sure there's an IPC call decrementing servershared_ptrreference count when clientshared_ptrproxy is reset.