proxy-io: fix race conditions in ProxyClientBase cleanup handler

ryanofsky · ryanofsky · commit d8011c83608e · 2025-06-28T08:08:18.000-04:00
This fixes a TSAN data race in the "disconnecting and blocking during the call" unit test. The error output is below and problem it shows is that the ProxyClientBase `m_context.connection->removeSyncCleanup(disconnect_cb);` call is dereferencing `Connection::m_loop.m_loop` pointer variable shortly before the ~Connection destructor sets the pointer to null in another thread, without any synchronization in between. This race could potentially cause segfaults if the timing was different and the onDisconnect call happened at the same time as the removeSyncCleanup call. Fix the error by moving the removeSyncCleanup call into a loop->sync() block so it runs on the event loop thread instead of running asynchronously. Moving this call fixes the TSAN error shown below but it causes 3 new TSAN errors in the same unit test, which will be fixed in a seperate commit. The TSAN error below error shows the ProxyClientBase object not using proper synchronization when accessing a Connection object field before the Connection object is destroyed. But this fix here causes Connection destruction to delayed, so the new errors show the opposite problem of code called from the Connection destructor not using proper synchronization before accessing ProxyClientBase fields. To explain the error output below in more detail: The test case that triggers this data race has 3 threads. There is the main client thread calling foo->callFnAsync() which is idle during this time. There is the corresponding server thread T11 in the stack trace below which is executing the callFnAsync() function, and calling setup.client_disconnect() to check for correct behavior when there is a disconnect during the IPC call. Finally there is the event loop thread T10 in the stack trace below which is actually deleting the server Connection object in response to the client_disconnect() call (which deletes the client Connection object). During the callFnAsync() call, the server thread T11 responsible for executing that call creates a ProxyClient<mp::Thread> object and inserts it into the g_thread_context.request_threads std::map in the type-context.h CustomPassField function, in case the server needs to make an IPC call to the client (which it does not in this test). Before the CustomPassField funcion returns it deletes the entry in the request_threads map, deleting the ProxyClient<Thread> object, and leading to the ProxyClientBase<Thread> `m_context.connection->removeSyncCleanup(disconnect_cb);` call referenced in the beginning which accesses the m_loop pointer variable without a lock. Error looks like: WARNING: ThreadSanitizer: data race (pid=2025645) Write of size 8 at 0x726000012c00 by thread T10: #0 mp::EventLoopRef::reset(bool) src/mp/proxy.cpp:61 (mptest+0x2213bf) #1 mp::Connection::~Connection() include/mp/proxy.h:58 (mptest+0x221a9b) #2 kj::_::TransformPromiseNode<kj::_::Void, kj::_::Void, mp::test::TestSetup::TestSetup(bool)::{lambda()#1}::operator()() const::{lambda()#2}, kj::_::PropagateException>::getImpl(kj::_::ExceptionOrValue&) /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/unique_ptr.h:93 (mptest+0x133817) #3 kj::_::TransformPromiseNodeBase::get(kj::_::ExceptionOrValue&) ??:? (libkj-async.so.1.1.0+0x3da3d) (BuildId: 21ff3d5ab929b5e72be4dbbab1a9223f705af6e4) #4 mp::EventLoop::loop() src/mp/proxy.cpp:229 (mptest+0x222f60) #5 mp::test::TestSetup::TestSetup(bool)::{lambda()#1}::operator()() const test/mp/test/test.cpp:92 (mptest+0x130d23) #6 void std::__invoke_impl<void, mp::test::TestSetup::TestSetup(bool)::{lambda()#1}>(std::__invoke_other, mp::test::TestSetup::TestSetup(bool)::{lambda()#1}&&) /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/invoke.h:61 (mptest+0x130789) #7 std::__invoke_result<mp::test::TestSetup::TestSetup(bool)::{lambda()#1}>::type std::__invoke<mp::test::TestSetup::TestSetup(bool)::{lambda()#1}>(mp::test::TestSetup::TestSetup(bool)::{lambda()#1}&&) /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/invoke.h:96 (mptest+0x130789) #8 void std::thread::_Invoker<std::tuple<mp::test::TestSetup::TestSetup(bool)::{lambda()#1}> >::_M_invoke<0ul>(std::_Index_tuple<0ul>) /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/std_thread.h:301 (mptest+0x130789) #9 std::thread::_Invoker<std::tuple<mp::test::TestSetup::TestSetup(bool)::{lambda()#1}> >::operator()() /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/std_thread.h:308 (mptest+0x130789) #10 std::thread::_State_impl<std::thread::_Invoker<std::tuple<mp::test::TestSetup::TestSetup(bool)::{lambda()#1}> > >::_M_run() /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/std_thread.h:253 (mptest+0x130789) #11 execute_native_thread_routine ??:? (libstdc++.so.6+0xed063) Previous read of size 8 at 0x726000012c00 by thread T11 (mutexes: write M0): #0 mp::Connection::removeSyncCleanup(std::_List_iterator<std::function<void ()> >) include/mp/proxy.h:60 (mptest+0x221dbf) #1 std::_Function_handler<void (), mp::ProxyClientBase<mp::Thread, capnp::Void>::ProxyClientBase(mp::Thread::Client, mp::Connection*, bool)::{lambda()#2}>::_M_invoke(std::_Any_data const&) include/mp/proxy-io.h:419 (mptest+0x228814) #2 mp::CleanupRun(std::__cxx11::list<std::function<void ()>, std::allocator<std::function<void ()> > >&) /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/std_function.h:591 (mptest+0x1ca65b) #3 mp::ProxyClientBase<mp::Thread, capnp::Void>::~ProxyClientBase() include/mp/proxy-io.h:446 (mptest+0x227790) #4 mp::ProxyClient<mp::Thread>::~ProxyClient() src/mp/proxy.cpp:339 (mptest+0x223fbd) #5 std::pair<mp::Connection* const, mp::ProxyClient<mp::Thread> >::~pair() /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/stl_iterator.h:3013 (mptest+0x1cad8a) #6 void std::destroy_at<std::pair<mp::Connection* const, mp::ProxyClient<mp::Thread> > >(std::pair<mp::Connection* const, mp::ProxyClient<mp::Thread> >*) /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/stl_construct.h:88 (mptest+0x1cad8a) #7 void std::allocator_traits<std::allocator<std::_Rb_tree_node<std::pair<mp::Connection* const, mp::ProxyClient<mp::Thread> > > > >::destroy<std::pair<mp::Connection* const, mp::ProxyClient<mp::Thread> > >(std::allocator<std::_Rb_tree_node<std::pair<mp::Connection* const, mp::ProxyClient<mp::Thread> > > >&, std::pair<mp::Connection* const, mp::ProxyClient<mp::Thread> >*) /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/alloc_traits.h:599 (mptest+0x1cad8a) #8 std::_Rb_tree<mp::Connection*, std::pair<mp::Connection* const, mp::ProxyClient<mp::Thread> >, std::_Select1st<std::pair<mp::Connection* const, mp::ProxyClient<mp::Thread> > >, std::less<mp::Connection*>, std::allocator<std::pair<mp::Connection* const, mp::ProxyClient<mp::Thread> > > >::_M_destroy_node(std::_Rb_tree_node<std::pair<mp::Connection* const, mp::ProxyClient<mp::Thread> > >*) /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/stl_tree.h:621 (mptest+0x1cad8a) #9 std::_Rb_tree<mp::Connection*, std::pair<mp::Connection* const, mp::ProxyClient<mp::Thread> >, std::_Select1st<std::pair<mp::Connection* const, mp::ProxyClient<mp::Thread> > >, std::less<mp::Connection*>, std::allocator<std::pair<mp::Connection* const, mp::ProxyClient<mp::Thread> > > >::_M_drop_node(std::_Rb_tree_node<std::pair<mp::Connection* const, mp::ProxyClient<mp::Thread> > >*) /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/stl_tree.h:629 (mptest+0x1cad8a) #10 std::_Rb_tree<mp::Connection*, std::pair<mp::Connection* const, mp::ProxyClient<mp::Thread> >, std::_Select1st<std::pair<mp::Connection* const, mp::ProxyClient<mp::Thread> > >, std::less<mp::Connection*>, std::allocator<std::pair<mp::Connection* const, mp::ProxyClient<mp::Thread> > > >::_M_erase(std::_Rb_tree_node<std::pair<mp::Connection* const, mp::ProxyClient<mp::Thread> > >*) /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/stl_tree.h:1934 (mptest+0x1cad8a) #11 std::_Rb_tree<mp::Connection*, std::pair<mp::Connection* const, mp::ProxyClient<mp::Thread> >, std::_Select1st<std::pair<mp::Connection* const, mp::ProxyClient<mp::Thread> > >, std::less<mp::Connection*>, std::allocator<std::pair<mp::Connection* const, mp::ProxyClient<mp::Thread> > > >::_M_erase_aux(std::_Rb_tree_const_iterator<std::pair<mp::Connection* const, mp::ProxyClient<mp::Thread> > >, std::_Rb_tree_const_iterator<std::pair<mp::Connection* const, mp::ProxyClient<mp::Thread> > >) /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/stl_tree.h:1251 (mptest+0x1cace1) #12 _ZZZN2mp9PassFieldINS_8AccessorINS_10foo_fields7ContextELi17EEENS_19ServerInvokeContextINS_11ProxyServerINS_4test8messages12FooInterfaceEEEN5capnp11CallContextINS9_17CallFnAsyncParamsENS9_18CallFnAsyncResultsEEEEENS_10ServerCallEJNS_8TypeListIJEEEEEENSt9enable_ifIXsr3std7is_sameIDTclsrT_3getcldtdtfL1p1_12call_context9getParamsEEENS_7Context6ReaderEEE5valueEN2kj7PromiseINT0_11CallContextEEEE4typeENS_8PriorityILi1EEESJ_RSR_RKT1_DpOT2_ENUlvE_clEvENKUlvE0_clEv /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/stl_tree.h:2519 (mptest+0x1f9b7e) #13 _ZN2kj1_8DeferredIZZN2mp9PassFieldINS2_8AccessorINS2_10foo_fields7ContextELi17EEENS2_19ServerInvokeContextINS2_11ProxyServerINS2_4test8messages12FooInterfaceEEEN5capnp11CallContextINSC_17CallFnAsyncParamsENSC_18CallFnAsyncResultsEEEEENS2_10ServerCallEJNS2_8TypeListIJEEEEEENSt9enable_ifIXsr3std7is_sameIDTclsrT_3getcldtdtfL1p1_12call_context9getParamsEEENS2_7Context6ReaderEEE5valueENS_7PromiseINT0_11CallContextEEEE4typeENS2_8PriorityILi1EEESM_RST_RKT1_DpOT2_ENUlvE_clEvEUlvE0_E3runEv /nix/store/46kiq9naswgbqfc14kc9nxcbgd0rv0m2-capnproto-1.1.0/include/kj/common.h:2007 (mptest+0x1f915d) #14 _ZN2kj1_8DeferredIZZN2mp9PassFieldINS2_8AccessorINS2_10foo_fields7ContextELi17EEENS2_19ServerInvokeContextINS2_11ProxyServerINS2_4test8messages12FooInterfaceEEEN5capnp11CallContextINSC_17CallFnAsyncParamsENSC_18CallFnAsyncResultsEEEEENS2_10ServerCallEJNS2_8TypeListIJEEEEEENSt9enable_ifIXsr3std7is_sameIDTclsrT_3getcldtdtfL1p1_12call_context9getParamsEEENS2_7Context6ReaderEEE5valueENS_7PromiseINT0_11CallContextEEEE4typeENS2_8PriorityILi1EEESM_RST_RKT1_DpOT2_ENUlvE_clEvEUlvE0_ED2Ev /nix/store/46kiq9naswgbqfc14kc9nxcbgd0rv0m2-capnproto-1.1.0/include/kj/common.h:1996 (mptest+0x1f915d) #15 _ZZN2mp9PassFieldINS_8AccessorINS_10foo_fields7ContextELi17EEENS_19ServerInvokeContextINS_11ProxyServerINS_4test8messages12FooInterfaceEEEN5capnp11CallContextINS9_17CallFnAsyncParamsENS9_18CallFnAsyncResultsEEEEENS_10ServerCallEJNS_8TypeListIJEEEEEENSt9enable_ifIXsr3std7is_sameIDTclsrT_3getcldtdtfL0p1_12call_context9getParamsEEENS_7Context6ReaderEEE5valueEN2kj7PromiseINT0_11CallContextEEEE4typeENS_8PriorityILi1EEESJ_RSR_RKT1_DpOT2_ENUlvE_clEv include/mp/type-context.h:122 (mptest+0x1f915d) #16 _ZN2kj8FunctionIFvvEE4ImplIZN2mp9PassFieldINS4_8AccessorINS4_10foo_fields7ContextELi17EEENS4_19ServerInvokeContextINS4_11ProxyServerINS4_4test8messages12FooInterfaceEEEN5capnp11CallContextINSE_17CallFnAsyncParamsENSE_18CallFnAsyncResultsEEEEENS4_10ServerCallEJNS4_8TypeListIJEEEEEENSt9enable_ifIXsr3std7is_sameIDTclsrT_3getcldtdtfL0p1_12call_context9getParamsEEENS4_7Context6ReaderEEE5valueENS_7PromiseINT0_11CallContextEEEE4typeENS4_8PriorityILi1EEESO_RSV_RKT1_DpOT2_EUlvE_EclEv /nix/store/46kiq9naswgbqfc14kc9nxcbgd0rv0m2-capnproto-1.1.0/include/kj/function.h:142 (mptest+0x1f8e89) #17 kj::Function<void ()>::operator()() /nix/store/46kiq9naswgbqfc14kc9nxcbgd0rv0m2-capnproto-1.1.0/include/kj/function.h:119 (mptest+0x1547f6) #18 void mp::Unlock<std::unique_lock<std::mutex>, kj::Function<void ()>&>(std::unique_lock<std::mutex>&, kj::Function<void ()>&) include/mp/util.h:198 (mptest+0x1547f6) #19 std::thread::_State_impl<std::thread::_Invoker<std::tuple<mp::ProxyServer<mp::ThreadMap>::makeThread(capnp::CallContext<mp::ThreadMap::MakeThreadParams, mp::ThreadMap::MakeThreadResults>)::$_0> > >::_M_run() include/mp/proxy-io.h:294 (mptest+0x2266ca) #20 execute_native_thread_routine ??:? (libstdc++.so.6+0xed063) Location is heap block of size 1024 at 0x726000012c00 allocated by thread T10: #0 operator new(unsigned long) ??:? (mptest+0x127c3c) #1 mp::test::TestSetup::TestSetup(bool)::{lambda()#1}::operator()() const /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/unique_ptr.h:1077 (mptest+0x130879) #2 void std::__invoke_impl<void, mp::test::TestSetup::TestSetup(bool)::{lambda()#1}>(std::__invoke_other, mp::test::TestSetup::TestSetup(bool)::{lambda()#1}&&) /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/invoke.h:61 (mptest+0x130789) #3 std::__invoke_result<mp::test::TestSetup::TestSetup(bool)::{lambda()#1}>::type std::__invoke<mp::test::TestSetup::TestSetup(bool)::{lambda()#1}>(mp::test::TestSetup::TestSetup(bool)::{lambda()#1}&&) /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/invoke.h:96 (mptest+0x130789) #4 void std::thread::_Invoker<std::tuple<mp::test::TestSetup::TestSetup(bool)::{lambda()#1}> >::_M_invoke<0ul>(std::_Index_tuple<0ul>) /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/std_thread.h:301 (mptest+0x130789) #5 std::thread::_Invoker<std::tuple<mp::test::TestSetup::TestSetup(bool)::{lambda()#1}> >::operator()() /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/std_thread.h:308 (mptest+0x130789) #6 std::thread::_State_impl<std::thread::_Invoker<std::tuple<mp::test::TestSetup::TestSetup(bool)::{lambda()#1}> > >::_M_run() /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/std_thread.h:253 (mptest+0x130789) #7 execute_native_thread_routine ??:? (libstdc++.so.6+0xed063) Mutex M0 (0x721c00003790) created at: #0 pthread_mutex_lock ??:? (mptest+0x9aa5c) #1 __gthread_mutex_lock(pthread_mutex_t*) /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/x86_64-unknown-linux-gnu/bits/gthr-default.h:762 (mptest+0x2265e2) #2 std::mutex::lock() /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/std_mutex.h:113 (mptest+0x2265e2) #3 std::unique_lock<std::mutex>::lock() /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/unique_lock.h:147 (mptest+0x2265e2) #4 std::unique_lock<std::mutex>::unique_lock(std::mutex&) /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/unique_lock.h:73 (mptest+0x2265e2) #5 mp::ProxyServer<mp::ThreadMap>::makeThread(capnp::CallContext<mp::ThreadMap::MakeThreadParams, mp::ThreadMap::MakeThreadResults>)::$_0::operator()() const src/mp/proxy.cpp:399 (mptest+0x2265e2) #6 void std::__invoke_impl<void, mp::ProxyServer<mp::ThreadMap>::makeThread(capnp::CallContext<mp::ThreadMap::MakeThreadParams, mp::ThreadMap::MakeThreadResults>)::$_0>(std::__invoke_other, mp::ProxyServer<mp::ThreadMap>::makeThread(capnp::CallContext<mp::ThreadMap::MakeThreadParams, mp::ThreadMap::MakeThreadResults>)::$_0&&) /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/invoke.h:61 (mptest+0x2265e2) #7 std::__invoke_result<mp::ProxyServer<mp::ThreadMap>::makeThread(capnp::CallContext<mp::ThreadMap::MakeThreadParams, mp::ThreadMap::MakeThreadResults>)::$_0>::type std::__invoke<mp::ProxyServer<mp::ThreadMap>::makeThread(capnp::CallContext<mp::ThreadMap::MakeThreadParams, mp::ThreadMap::MakeThreadResults>)::$_0>(mp::ProxyServer<mp::ThreadMap>::makeThread(capnp::CallContext<mp::ThreadMap::MakeThreadParams, mp::ThreadMap::MakeThreadResults>)::$_0&&) /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/invoke.h:96 (mptest+0x2265e2) #8 void std::thread::_Invoker<std::tuple<mp::ProxyServer<mp::ThreadMap>::makeThread(capnp::CallContext<mp::ThreadMap::MakeThreadParams, mp::ThreadMap::MakeThreadResults>)::$_0> >::_M_invoke<0ul>(std::_Index_tuple<0ul>) /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/std_thread.h:301 (mptest+0x2265e2) #9 std::thread::_Invoker<std::tuple<mp::ProxyServer<mp::ThreadMap>::makeThread(capnp::CallContext<mp::ThreadMap::MakeThreadParams, mp::ThreadMap::MakeThreadResults>)::$_0> >::operator()() /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/std_thread.h:308 (mptest+0x2265e2) #10 std::thread::_State_impl<std::thread::_Invoker<std::tuple<mp::ProxyServer<mp::ThreadMap>::makeThread(capnp::CallContext<mp::ThreadMap::MakeThreadParams, mp::ThreadMap::MakeThreadResults>)::$_0> > >::_M_run() /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/std_thread.h:253 (mptest+0x2265e2) #11 execute_native_thread_routine ??:? (libstdc++.so.6+0xed063) Thread T10 (tid=2025656, running) created by main thread at: #0 pthread_create ??:? (mptest+0x9a2a5) #1 std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> >, void (*)()) ??:? (libstdc++.so.6+0xed138) #2 mp::test::TestCase251::run() test/mp/test/test.cpp:271 (mptest+0x12b5bd) #3 kj::Maybe<kj::Exception> kj::runCatchingExceptions<kj::TestRunner::run()::{lambda()#1}>(kj::TestRunner::run()::{lambda()#1}&&) ??:? (libkj-test.so.1.1.0+0x7290) (BuildId: 50ddf81234cd06daf5f2d3f11713ed193ade4eb7) Thread T11 (tid=2025657, running) created by thread T10 at: #0 pthread_create ??:? (mptest+0x9a2a5) #1 std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> >, void (*)()) ??:? (libstdc++.so.6+0xed138) #2 mp::ThreadMap::Server::dispatchCall(unsigned long, unsigned short, capnp::CallContext<capnp::AnyPointer, capnp::AnyPointer>) build-tsan/include/mp/proxy.capnp.c++:602 (mptest+0x220bb2) #3 virtual thunk to mp::ThreadMap::Server::dispatchCall(unsigned long, unsigned short, capnp::CallContext<capnp::AnyPointer, capnp::AnyPointer>) build-tsan/include/mp/proxy.capnp.c++:? (mptest+0x220bb2) #4 capnp::LocalClient::callInternal(unsigned long, unsigned short, capnp::CallContextHook&) ??:? (libcapnp-rpc.so.1.1.0+0x7551d) (BuildId: ec28fb7ea510d2e28e99522abd0ce44adde7cf44) #5 mp::EventLoop::loop() src/mp/proxy.cpp:229 (mptest+0x222f60) #6 mp::test::TestSetup::TestSetup(bool)::{lambda()#1}::operator()() const test/mp/test/test.cpp:92 (mptest+0x130d23) #7 void std::__invoke_impl<void, mp::test::TestSetup::TestSetup(bool)::{lambda()#1}>(std::__invoke_other, mp::test::TestSetup::TestSetup(bool)::{lambda()#1}&&) /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/invoke.h:61 (mptest+0x130789) #8 std::__invoke_result<mp::test::TestSetup::TestSetup(bool)::{lambda()#1}>::type std::__invoke<mp::test::TestSetup::TestSetup(bool)::{lambda()#1}>(mp::test::TestSetup::TestSetup(bool)::{lambda()#1}&&) /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/invoke.h:96 (mptest+0x130789) #9 void std::thread::_Invoker<std::tuple<mp::test::TestSetup::TestSetup(bool)::{lambda()#1}> >::_M_invoke<0ul>(std::_Index_tuple<0ul>) /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/std_thread.h:301 (mptest+0x130789) #10 std::thread::_Invoker<std::tuple<mp::test::TestSetup::TestSetup(bool)::{lambda()#1}> >::operator()() /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/std_thread.h:308 (mptest+0x130789) #11 std::thread::_State_impl<std::thread::_Invoker<std::tuple<mp::test::TestSetup::TestSetup(bool)::{lambda()#1}> > >::_M_run() /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/std_thread.h:253 (mptest+0x130789) #12 execute_native_thread_routine ??:? (libstdc++.so.6+0xed063) SUMMARY: ThreadSanitizer: data race src/mp/proxy.cpp:61 in mp::EventLoopRef::reset(bool) ================== [ PASS ] test.cpp:251: Calling IPC method, disconnecting and blocking during the call (175646 μs) 5 test(s) passed ThreadSanitizer: reported 1 warnings nt method call interrupted by disconnect. LOG0: {mptest-2025645/mptest-2025654} IPC server destroy N2mp11ProxyServerINS_4test8messages12FooInterfaceEEE LOG0: {mptest-2025645/mptest-2025654} EventLoop::loop done, cancelling event listeners. LOG0: {mptest-2025645/mptest-2025654} EventLoop::loop bye. LOG0: {mptest-2025645/mptest-2025645} IPC client send FooInterface.add$Params (a = 1, b = 2) LOG0: {mptest-2025645/mptest-2025656} IPC server recv request #35 FooInterface.add$Params (a = 1, b = 2) LOG0: {mptest-2025645/mptest-2025656} IPC server send response #35 FooInterface.add$Results (result = 3) LOG0: {mptest-2025645/mptest-2025645} IPC client recv FooInterface.add$Results (result = 3) LOG0: {mptest-2025645/mptest-2025645} IPC client send FooInterface.initThreadMap$Params (threadMap = <external capability>) LOG0: {mptest-2025645/mptest-2025656} IPC server recv request #36 FooInterface.initThreadMap$Params (threadMap = <external capability>) LOG0: {mptest-2025645/mptest-2025656} IPC server send response #36 FooInterface.initThreadMap$Results (threadMap = <external capability>) LOG0: {mptest-2025645/mptest-2025645} IPC client recv FooInterface.initThreadMap$Results (threadMap = <external capability>) LOG0: {mptest-2025645/mptest-2025645} IPC client send FooInterface.callFnAsync$Params (context = (thread = <external capability>, callbackThread = <external capability>)) LOG0: {mptest-2025645/mptest-2025656} IPC server recv request #37 FooInterface.callFnAsync$Params (context = (thread = <external capability>, callbackThread = <external capability>)) LOG0: {mptest-2025645/mptest-2025656} IPC server post request #37 {mptest-2025645/mptest-2025657 (from mptest-2025645/mptest-2025645)} LOG1: IPC client method call interrupted by disconnect. LOG0: {mptest-2025645/mptest-2025656} IPC server send response #37 FooInterface.callFnAsync$Results () LOG0: {mptest-2025645/mptest-2025656} IPC server destroy N2mp11ProxyServerINS_4test8messages12FooInterfaceEEE LOG0: {mptest-2025645/mptest-2025656} EventLoop::loop done, cancelling event listeners. LOG0: {mptest-2025645/mptest-2025656} EventLoop::loop bye. 0% tests passed, 1 tests failed out of 1 Total Test time (real) = 0.22 sec The following tests FAILED: 1 - mptest (Failed)
diff --git a/include/mp/proxy-io.h b/include/mp/proxy-io.h
@@ -420,11 +420,7 @@ ProxyClientBase<Interface, Impl>::ProxyClientBase(typename Interface::Client cli
     // second case is handled by the disconnect_cb function, which sets
     // m_context.connection to null so nothing happens here.
     m_context.cleanup_fns.emplace_front([this, destroy_connection, disconnect_cb]{
-    if (m_context.connection) {
-        // Remove disconnect callback so it doesn't run and try to access this
-        // object after it's already destroyed.
-        m_context.connection->removeSyncCleanup(disconnect_cb);
-
+    {
         // If the capnp interface defines a destroy method, call it to destroy
         // the remote object, waiting for it to be deleted server side. If the
         // capnp interface does not define a destroy method, this will just call
@@ -433,6 +429,14 @@ ProxyClientBase<Interface, Impl>::ProxyClientBase(typename Interface::Client cli
 
         // FIXME: Could just invoke removed addCleanup fn here instead of duplicating code
         m_context.loop->sync([&]() {
+            // Remove disconnect callback on cleanup so it doesn't run and try
+            // to access this object after it's destroyed. This call needs to
+            // run inside loop->sync() on the event loop thread because
+            // otherwise, if there were an ill-timed disconnect, the
+            // onDisconnect handler could fire and delete the Connection object
+            // before the removeSyncCleanup call.
+            if (m_context.connection) m_context.connection->removeSyncCleanup(disconnect_cb);
+
             // Release client capability by move-assigning to temporary.
             {
                 typename Interface::Client(std::move(m_client));
