chaincodelabs / libmultiprocess

C++ library and code generator making it easy to call functions and reference objects in different processes
MIT License
29 stars 20 forks source link

connection: run async cleanups in LIFO not FIFO order #101

Closed ryanofsky closed 5 months ago

ryanofsky commented 5 months ago

Run Connection class asynchronous cleanups in LIFO not FIFO order, which is more natural ordering and prevents a bitcoin wallet shutdown deadlock when the connection to the node process is broken. Also add comments better documenting cleanup order.

This change fixes one of two shutdown deadlocks in the bitcoin wallet when the node process is killed in https://github.com/bitcoin/bitcoin/pull/10102 as of https://github.com/bitcoin/bitcoin/commit/3f12b4362c1b822835889151f6cd54c4829cf3ad. The other deadlock is caused by the ProxyServerCustom<WalletLoader> destructor in that PR and is described in commit efe42cc2879915b2ffdff04ec1cfe715e8dfa0a0 from #100.

The bitcoin wallet shutdown deadlocks have probably been around for some time, but were not encountered because before https://github.com/bitcoin/bitcoin/commit/0b753156ce60c29efb2386954ba7555ad8f642f5 from https://github.com/bitcoin/bitcoin/pull/26606 there weren't any tests which killed the bitcoin node process and required the bitcoin-wallet process to shut down gracefully.

ryanofsky commented 5 months ago

Updated cfdd85c6afab980f2730238d46e30d8fab8fad11 -> 6825523c6e35e6f8f56ff013d24b51d3ceb8f58d (pr/lifo.1 -> pr/lifo.2, compare) improving code comments

ryanofsky commented 5 months ago

Deadlocks mentioned are described in more detail in https://github.com/bitcoin/bitcoin/pull/10102#issuecomment-2166556815