Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TSAN issue with libuv based scheduler #7083

Open
kiburtse opened this issue Oct 26, 2023 · 2 comments
Open

TSAN issue with libuv based scheduler #7083

kiburtse opened this issue Oct 26, 2023 · 2 comments

Comments

@kiburtse
Copy link
Contributor

Expected results

No TSAN warnings at least

Actual Results

tsan warning discovered in #6911 on Realm::get_shared_realm -> Scheduler::make_default -> uv_async_init:

WARNING: ThreadSanitizer: data race (pid=218426)
  Write of size 4 at 0x00000398e6f0 by thread T6:
    #0 uv_async_init /data/mci/edb4f55af760b20532a969e6b02bb968/realm-core/build/_deps/libuv-src/src/unix/async.c:56 (realm-object-store-tests+0x26bab75)
    #1 UvMainLoopScheduler /data/mci/edb4f55af760b20532a969e6b02bb968/realm-core/build/../src/realm/object-store/util/uv/scheduler.hpp:36 (realm-object-store-tests+0xee6a64)
    ...
    #13 realm::util::Scheduler::make_default() /data/mci/edb4f55af760b20532a969e6b02bb968/realm-core/build/../src/realm/object-store/util/scheduler.cpp:118 (realm-object-store-tests+0x25690c9)
    #14 realm::Realm::get_shared_realm(realm::ThreadSafeReference, std::shared_ptr<realm::util::Scheduler>) /data/mci/edb4f55af760b20532a969e6b02bb968/realm-core/build/../src/realm/object-store/shared_realm.cpp:173 (realm-object-store-tests+0x24fd0e0)
    #15 operator() /data/mci/edb4f55af760b20532a969e6b02bb968/realm-core/build/../test/object-store/realm.cpp:1079 (realm-object-store-tests+0x14024f0)
    ....
    #18 realm::AsyncOpenTask::async_open_complete(realm::util::UniqueFunction<void (realm::ThreadSafeReference, std::__exception_ptr::exception_ptr)>&&, std::shared_ptr<realm::_impl::RealmCoordinator>, realm::Status) 

  Previous read of size 4 at 0x00000398e6f0 by main thread:
    #0 uv_backend_timeout /data/mci/edb4f55af760b20532a969e6b02bb968/realm-core/build/_deps/libuv-src/src/unix/core.c:332 (realm-object-store-tests+0x26bb9c1)
    #1 uv_run /data/mci/edb4f55af760b20532a969e6b02bb968/realm-core/build/_deps/libuv-src/src/unix/core.c:378 (realm-object-store-tests+0x26bb9c1)
    #2 realm::util::EventLoop::Impl::run_until(realm::util::FunctionRef<bool ()>) /data/mci/edb4f55af760b20532a969e6b02bb968/realm-core/build/../test/object-store/util/event_loop.cpp:203 (realm-object-store-tests+0x1c06c5c)
    #3 realm::util::EventLoop::run_until(realm::util::FunctionRef<bool ()>) /data/mci/edb4f55af760b20532a969e6b02bb968/realm-core/build/../test/object-store/util/event_loop.cpp:122 (realm-object-store-tests+0x1c06ba5)
    #4 CATCH2_INTERNAL_TEST_56() /data/mci/edb4f55af760b20532a969e6b02bb968/realm-core/build/../test/object-store/realm.cpp:1085 (realm-object-store-tests+0x13a8bcb)

Steps & Code to Reproduce

Consider code like this from test/object-store/realm.cpp:

std::shared_ptr<AsyncOpenTask> task = Realm::get_synchronized_realm(config);

bool completed = false;
task->start([&](ThreadSafeReference realm_ref, std::exception_ptr err) {
    auto realm = Realm::get_shared_realm(std::move(realm_ref));
    // ...
    completed = true;
}

util::EventLoop::main().run_until([&] {
    return completed;
});

This will trigger tsan since without set scheduler get_shared_realm will construct default scheduler which in case of UV impl is not supposed to be anything other than main thread. It is also problematic when it's shared with at least testing code inside util::EventLoop. It's separate instance and handling without integration with the make_default.

Also, according to the doc for libuv uv_default_loop is already initialized by first invocation. There is nothing specific what needs to be done about it.

Core version

Core version: 13.23.1

@tgoyne
Copy link
Member

tgoyne commented Oct 27, 2023

We should probably assert that we're on the main thread when constructing a uv scheduler as this has been a recurring problem.

@tgoyne
Copy link
Member

tgoyne commented Oct 27, 2023

It also would probably not be too hard to make it partially work on background threads and just not support actually scheduling tasks. The current state of it is just a result of it being written for node before node supported threads.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants