Skip to content

Commit 5975fef

Browse files
committed
mptest: fix race condition in TestSetup constructor
Fix race condition caught by TSAN between TestSetup constructor and std::thread launched by the constructor, which could lead to the constructor setting member variables to default values after the std::thread set them, which would not be the intended order and could lead to bugs. The TSAN errors looked like the following: [ TEST ] test.cpp:108: Call FooInterface methods ================== WARNING: ThreadSanitizer: data race (pid=1970628) Write of size 8 at 0x7ffcf7ba4c30 by thread T1: #0 mp::test::TestSetup::TestSetup(bool)::{lambda()#1}::operator()() const::{lambda(mp::Connection&)#1}::operator()(mp::Connection&) const test/mp/test/test.cpp:71 (mptest+0x131c3d) #1 capnp::Capability::Client std::__invoke_impl<capnp::Capability::Client, mp::test::TestSetup::TestSetup(bool)::{lambda()#1}::operator()() const::{lambda(mp::Connection&)#1}&, mp::Connection&>(std::__invoke_other, mp::test::TestSetup::TestSetup(bool)::{lambda()#1}::operator()() const::{lambda(mp::Connection&)#1}&, mp::Connection&) /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/invoke.h:61 (mptest+0x131a25) #2 std::enable_if<is_invocable_r_v<capnp::Capability::Client, mp::test::TestSetup::TestSetup(bool)::{lambda()#1}::operator()() const::{lambda(mp::Connection&)#1}&, mp::Connection&>, capnp::Capability::Client>::type std::__invoke_r<capnp::Capability::Client, mp::test::TestSetup::TestSetup(bool)::{lambda()#1}::operator()() const::{lambda(mp::Connection&)#1}&, mp::Connection&>(mp::test::TestSetup::TestSetup(bool)::{lambda()#1}::operator()() const::{lambda(mp::Connection&)#1}&, mp::Connection&) /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/invoke.h:114 (mptest+0x131a25) #3 std::_Function_handler<capnp::Capability::Client (mp::Connection&), mp::test::TestSetup::TestSetup(bool)::{lambda()#1}::operator()() const::{lambda(mp::Connection&)#1}>::_M_invoke(std::_Any_data const&, mp::Connection&) /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/std_function.h:290 (mptest+0x131a25) #4 mp::Connection::Connection(mp::EventLoop&, kj::Own<kj::AsyncIoStream, decltype(nullptr)>&&, std::function<capnp::Capability::Client (mp::Connection&)> const&) /nix/store/9ds850ifd4jwcccpp3v14818kk74ldf2-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/std_function.h:591 (mptest+0x13167f) #5 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+0x13054c) #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+0x1303f9) #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+0x1303f9) #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+0x1303f9) #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+0x1303f9) #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+0x1303f9) #11 execute_native_thread_routine ??:? (libstdc++.so.6+0xed063) Previous write of size 8 at 0x7ffcf7ba4c30 by main thread: #0 __tsan_memset ??:? (mptest+0x8811f) #1 mp::test::TestSetup::TestSetup(bool) test/mp/test/test.cpp:57 (mptest+0x12c3b4) #2 mp::test::TestCase108::run() test/mp/test/test.cpp:110 (mptest+0x128160) #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) Location is stack of main thread. Thread T1 (tid=1970630, 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::TestCase108::run() test/mp/test/test.cpp:110 (mptest+0x128160) #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) SUMMARY: ThreadSanitizer: data race test/mp/test/test.cpp:71 in mp::test::TestSetup::TestSetup(bool)::{lambda()#1}::operator()() const::{lambda(mp::Connection&)#1}::operator()(mp::Connection&) const
1 parent 8954cc0 commit 5975fef

File tree

1 file changed

+1
-1
lines changed

1 file changed

+1
-1
lines changed

test/mp/test/test.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,12 +49,12 @@ namespace test {
4949
class TestSetup
5050
{
5151
public:
52-
std::thread thread;
5352
std::function<void()> server_disconnect;
5453
std::function<void()> client_disconnect;
5554
std::promise<std::unique_ptr<ProxyClient<messages::FooInterface>>> client_promise;
5655
std::unique_ptr<ProxyClient<messages::FooInterface>> client;
5756
ProxyServer<messages::FooInterface>* server{nullptr};
57+
std::thread thread;
5858

5959
TestSetup(bool client_owns_connection = true)
6060
: thread{[&] {

0 commit comments

Comments
 (0)