diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h --- a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h @@ -39,9 +39,7 @@ class ThreadContextBase { public: explicit ThreadContextBase(u32 tid); - const u32 tid; // Thread ID. Main thread should have tid = 0. - u64 unique_id; // Unique thread ID. - u32 reuse_count; // Number of times this tid was reused. + const u32 tid; // Thread ID. Main thread should have tid = 0. tid_t os_id; // PID (used for reporting). uptr user_id; // Some opaque user thread id (e.g. pthread_t). char name[64]; // As annotated by user. @@ -51,7 +49,6 @@ ThreadType thread_type; u32 parent_tid; - ThreadContextBase *next; // For storing thread contexts in a list. atomic_uint32_t thread_destroyed; // To address race of Joined vs Finished @@ -61,8 +58,7 @@ void SetJoined(void *arg); void SetFinished(); void SetStarted(tid_t _os_id, ThreadType _thread_type, void *arg); - void SetCreated(uptr _user_id, u64 _unique_id, bool _detached, - u32 _parent_tid, void *arg); + void SetCreated(uptr _user_id, bool _detached, u32 _parent_tid, void *arg); void Reset(); void SetDestroyed(); @@ -83,16 +79,12 @@ ~ThreadContextBase(); }; -typedef ThreadContextBase* (*ThreadContextFactory)(u32 tid); +typedef ThreadContextBase *(*ThreadContextFactory)(u32 tid); class MUTEX ThreadRegistry { public: ThreadRegistry(ThreadContextFactory factory); - ThreadRegistry(ThreadContextFactory factory, u32 max_threads, - u32 thread_quarantine_size, u32 max_reuse); - void GetNumberOfThreads(uptr *total = nullptr, uptr *running = nullptr, - uptr *alive = nullptr); - uptr GetMaxAliveThreads(); + void GetNumberOfThreads(uptr *total, uptr *running = nullptr); void Lock() ACQUIRE() { mtx_.Lock(); } void CheckLocked() const CHECK_LOCKED() { mtx_.CheckLocked(); } @@ -133,24 +125,10 @@ private: const ThreadContextFactory context_factory_; - const u32 max_threads_; - const u32 thread_quarantine_size_; - const u32 max_reuse_; Mutex mtx_; - - u64 total_threads_; // Total number of created threads. May be greater than - // max_threads_ if contexts were reused. - uptr alive_threads_; // Created or running. - uptr max_alive_threads_; - uptr running_threads_; - InternalMmapVector threads_; - IntrusiveList dead_threads_; - IntrusiveList invalid_threads_; - - void QuarantinePush(ThreadContextBase *tctx); - ThreadContextBase *QuarantinePop(); + u32 running_threads_; }; typedef GenericScopedLock ThreadRegistryLock; diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp --- a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp @@ -16,9 +16,13 @@ namespace __sanitizer { ThreadContextBase::ThreadContextBase(u32 tid) - : tid(tid), unique_id(0), reuse_count(), os_id(0), user_id(0), - status(ThreadStatusInvalid), detached(false), - thread_type(ThreadType::Regular), parent_tid(0), next(0) { + : tid(tid), + os_id(0), + user_id(0), + status(ThreadStatusInvalid), + detached(false), + thread_type(ThreadType::Regular), + parent_tid(kInvalidTid) { name[0] = '\0'; atomic_store(&thread_destroyed, 0, memory_order_release); } @@ -78,11 +82,10 @@ OnStarted(arg); } -void ThreadContextBase::SetCreated(uptr _user_id, u64 _unique_id, - bool _detached, u32 _parent_tid, void *arg) { +void ThreadContextBase::SetCreated(uptr _user_id, bool _detached, + u32 _parent_tid, void *arg) { status = ThreadStatusCreated; user_id = _user_id; - unique_id = _unique_id; detached = _detached; // Parent tid makes no sense for the main thread. if (tid != kMainTid) @@ -100,89 +103,42 @@ // ThreadRegistry implementation. ThreadRegistry::ThreadRegistry(ThreadContextFactory factory) - : ThreadRegistry(factory, UINT32_MAX, UINT32_MAX, 0) {} - -ThreadRegistry::ThreadRegistry(ThreadContextFactory factory, u32 max_threads, - u32 thread_quarantine_size, u32 max_reuse) : context_factory_(factory), - max_threads_(max_threads), - thread_quarantine_size_(thread_quarantine_size), - max_reuse_(max_reuse), - mtx_(), - total_threads_(0), - alive_threads_(0), - max_alive_threads_(0), - running_threads_(0) { - dead_threads_.clear(); - invalid_threads_.clear(); -} + mtx_(MutexThreadRegistry), + running_threads_() {} -void ThreadRegistry::GetNumberOfThreads(uptr *total, uptr *running, - uptr *alive) { +void ThreadRegistry::GetNumberOfThreads(uptr *total, uptr *running) { ThreadRegistryLock l(this); if (total) *total = threads_.size(); - if (running) *running = running_threads_; - if (alive) *alive = alive_threads_; -} - -uptr ThreadRegistry::GetMaxAliveThreads() { - ThreadRegistryLock l(this); - return max_alive_threads_; + if (running) + *running = running_threads_; } u32 ThreadRegistry::CreateThread(uptr user_id, bool detached, u32 parent_tid, void *arg) { ThreadRegistryLock l(this); - u32 tid = kInvalidTid; - ThreadContextBase *tctx = QuarantinePop(); - if (tctx) { - tid = tctx->tid; - } else if (threads_.size() < max_threads_) { - // Allocate new thread context and tid. - tid = threads_.size(); - tctx = context_factory_(tid); - threads_.push_back(tctx); - } else { -#if !SANITIZER_GO - Report("%s: Thread limit (%u threads) exceeded. Dying.\n", - SanitizerToolName, max_threads_); -#else - Printf("race: limit on %u simultaneously alive goroutines is exceeded," - " dying\n", max_threads_); -#endif - Die(); - } - CHECK_NE(tctx, 0); - CHECK_NE(tid, kInvalidTid); - CHECK_LT(tid, max_threads_); + u32 tid = threads_.size(); + ThreadContextBase *tctx = context_factory_(tid); + threads_.push_back(tctx); CHECK_EQ(tctx->status, ThreadStatusInvalid); - alive_threads_++; - if (max_alive_threads_ < alive_threads_) { - max_alive_threads_++; - CHECK_EQ(alive_threads_, max_alive_threads_); - } - tctx->SetCreated(user_id, total_threads_++, detached, - parent_tid, arg); + tctx->SetCreated(user_id, detached, parent_tid, arg); return tid; } void ThreadRegistry::RunCallbackForEachThreadLocked(ThreadCallback cb, void *arg) { CheckLocked(); - for (u32 tid = 0; tid < threads_.size(); tid++) { - ThreadContextBase *tctx = threads_[tid]; - if (tctx == 0) - continue; - cb(tctx, arg); + for (auto tctx : threads_) { + if (tctx) + cb(tctx, arg); } } u32 ThreadRegistry::FindThread(FindThreadCallback cb, void *arg) { ThreadRegistryLock l(this); - for (u32 tid = 0; tid < threads_.size(); tid++) { - ThreadContextBase *tctx = threads_[tid]; - if (tctx != 0 && cb(tctx, arg)) + for (auto tctx : threads_) { + if (tctx && cb(tctx, arg)) return tctx->tid; } return kInvalidTid; @@ -191,9 +147,8 @@ ThreadContextBase * ThreadRegistry::FindThreadContextLocked(FindThreadCallback cb, void *arg) { CheckLocked(); - for (u32 tid = 0; tid < threads_.size(); tid++) { - ThreadContextBase *tctx = threads_[tid]; - if (tctx != 0 && cb(tctx, arg)) + for (auto tctx : threads_) { + if (tctx && cb(tctx, arg)) return tctx; } return 0; @@ -221,8 +176,7 @@ void ThreadRegistry::SetThreadNameByUserId(uptr user_id, const char *name) { ThreadRegistryLock l(this); - for (u32 tid = 0; tid < threads_.size(); tid++) { - ThreadContextBase *tctx = threads_[tid]; + for (auto tctx : threads_) { if (tctx != 0 && tctx->user_id == user_id && tctx->status != ThreadStatusInvalid) { tctx->SetName(name); @@ -242,31 +196,25 @@ tctx->OnDetached(arg); if (tctx->status == ThreadStatusFinished) { tctx->SetDead(); - QuarantinePush(tctx); } else { tctx->detached = true; } } void ThreadRegistry::JoinThread(u32 tid, void *arg) { - bool destroyed = false; - do { - { - ThreadRegistryLock l(this); - ThreadContextBase *tctx = threads_[tid]; - CHECK_NE(tctx, 0); - if (tctx->status == ThreadStatusInvalid) { - Report("%s: Join of non-existent thread\n", SanitizerToolName); - return; - } - if ((destroyed = tctx->GetDestroyed())) { - tctx->SetJoined(arg); - QuarantinePush(tctx); - } + for (;; internal_sched_yield()) { + ThreadRegistryLock l(this); + ThreadContextBase *tctx = threads_[tid]; + CHECK_NE(tctx, 0); + if (tctx->status == ThreadStatusInvalid) { + Report("%s: Join of non-existent thread\n", SanitizerToolName); + return; + } + if (tctx->GetDestroyed()) { + tctx->SetJoined(arg); + return; } - if (!destroyed) - internal_sched_yield(); - } while (!destroyed); + } } // Normally this is called when the thread is about to exit. If @@ -276,25 +224,20 @@ // create it, and so never called StartThread. ThreadStatus ThreadRegistry::FinishThread(u32 tid) { ThreadRegistryLock l(this); - CHECK_GT(alive_threads_, 0); - alive_threads_--; ThreadContextBase *tctx = threads_[tid]; CHECK_NE(tctx, 0); bool dead = tctx->detached; ThreadStatus prev_status = tctx->status; if (tctx->status == ThreadStatusRunning) { - CHECK_GT(running_threads_, 0); - running_threads_--; + CHECK_GT(--running_threads_, 0); } else { // The thread never really existed. CHECK_EQ(tctx->status, ThreadStatusCreated); dead = true; } tctx->SetFinished(); - if (dead) { + if (dead) tctx->SetDead(); - QuarantinePush(tctx); - } tctx->SetDestroyed(); return prev_status; } @@ -309,30 +252,6 @@ tctx->SetStarted(os_id, thread_type, arg); } -void ThreadRegistry::QuarantinePush(ThreadContextBase *tctx) { - if (tctx->tid == 0) - return; // Don't reuse the main thread. It's a special snowflake. - dead_threads_.push_back(tctx); - if (dead_threads_.size() <= thread_quarantine_size_) - return; - tctx = dead_threads_.front(); - dead_threads_.pop_front(); - CHECK_EQ(tctx->status, ThreadStatusDead); - tctx->Reset(); - tctx->reuse_count++; - if (max_reuse_ > 0 && tctx->reuse_count >= max_reuse_) - return; - invalid_threads_.push_back(tctx); -} - -ThreadContextBase *ThreadRegistry::QuarantinePop() { - if (invalid_threads_.size() == 0) - return 0; - ThreadContextBase *tctx = invalid_threads_.front(); - invalid_threads_.pop_front(); - return tctx; -} - void ThreadRegistry::SetThreadUserId(u32 tid, uptr user_id) { ThreadRegistryLock l(this); ThreadContextBase *tctx = threads_[tid]; diff --git a/compiler-rt/lib/sanitizer_common/tests/sanitizer_thread_registry_test.cpp b/compiler-rt/lib/sanitizer_common/tests/sanitizer_thread_registry_test.cpp --- a/compiler-rt/lib/sanitizer_common/tests/sanitizer_thread_registry_test.cpp +++ b/compiler-rt/lib/sanitizer_common/tests/sanitizer_thread_registry_test.cpp @@ -28,16 +28,12 @@ return new(tctx_allocator) TCTX(tid); } -static const u32 kMaxRegistryThreads = 1000; -static const u32 kRegistryQuarantine = 2; - static void CheckThreadQuantity(ThreadRegistry *registry, uptr exp_total, - uptr exp_running, uptr exp_alive) { - uptr total, running, alive; - registry->GetNumberOfThreads(&total, &running, &alive); + uptr exp_running) { + uptr total, running; + registry->GetNumberOfThreads(&total, &running); EXPECT_EQ(exp_total, total); EXPECT_EQ(exp_running, running); - EXPECT_EQ(exp_alive, alive); } static bool is_detached(u32 tid) { @@ -63,87 +59,72 @@ arr[tctx->tid] = true; } -static void TestRegistry(ThreadRegistry *registry, bool has_quarantine) { +TEST(SanitizerCommon, ThreadRegistryTest) { + ThreadRegistry registry(GetThreadContext); // Create and start a main thread. - EXPECT_EQ(0U, registry->CreateThread(get_uid(0), true, -1, 0)); - registry->StartThread(0, 0, ThreadType::Regular, 0); + EXPECT_EQ(0U, registry.CreateThread(get_uid(0), true, -1, 0)); + registry.StartThread(0, 0, ThreadType::Regular, 0); // Create a bunch of threads. for (u32 i = 1; i <= 10; i++) { - EXPECT_EQ(i, registry->CreateThread(get_uid(i), is_detached(i), 0, 0)); + EXPECT_EQ(i, registry.CreateThread(get_uid(i), is_detached(i), 0, 0)); } - CheckThreadQuantity(registry, 11, 1, 11); + CheckThreadQuantity(®istry, 11, 1); // Start some of them. for (u32 i = 1; i <= 5; i++) { - registry->StartThread(i, 0, ThreadType::Regular, 0); + registry.StartThread(i, 0, ThreadType::Regular, 0); } - CheckThreadQuantity(registry, 11, 6, 11); + CheckThreadQuantity(®istry, 11, 6); // Finish, create and start more threads. for (u32 i = 1; i <= 5; i++) { - registry->FinishThread(i); + registry.FinishThread(i); if (!is_detached(i)) - registry->JoinThread(i, 0); + registry.JoinThread(i, 0); } for (u32 i = 6; i <= 10; i++) { - registry->StartThread(i, 0, ThreadType::Regular, 0); + registry.StartThread(i, 0, ThreadType::Regular, 0); } std::vector new_tids; for (u32 i = 11; i <= 15; i++) { - new_tids.push_back( - registry->CreateThread(get_uid(i), is_detached(i), 0, 0)); + new_tids.push_back(registry.CreateThread(get_uid(i), is_detached(i), 0, 0)); } - ASSERT_LE(kRegistryQuarantine, 5U); - u32 exp_total = 16 - (has_quarantine ? 5 - kRegistryQuarantine : 0); - CheckThreadQuantity(registry, exp_total, 6, 11); + CheckThreadQuantity(®istry, 16, 6); // Test SetThreadName and FindThread. - registry->SetThreadName(6, "six"); - registry->SetThreadName(7, "seven"); - EXPECT_EQ(7U, registry->FindThread(HasName, (void*)"seven")); - EXPECT_EQ(kInvalidTid, registry->FindThread(HasName, (void *)"none")); - EXPECT_EQ(0U, registry->FindThread(HasUid, (void*)get_uid(0))); - EXPECT_EQ(10U, registry->FindThread(HasUid, (void*)get_uid(10))); - EXPECT_EQ(kInvalidTid, registry->FindThread(HasUid, (void *)0x1234)); + registry.SetThreadName(6, "six"); + registry.SetThreadName(7, "seven"); + EXPECT_EQ(7U, registry.FindThread(HasName, (void *)"seven")); + EXPECT_EQ(kInvalidTid, registry.FindThread(HasName, (void *)"none")); + EXPECT_EQ(0U, registry.FindThread(HasUid, (void *)get_uid(0))); + EXPECT_EQ(10U, registry.FindThread(HasUid, (void *)get_uid(10))); + EXPECT_EQ(kInvalidTid, registry.FindThread(HasUid, (void *)0x1234)); // Detach and finish and join remaining threads. for (u32 i = 6; i <= 10; i++) { - registry->DetachThread(i, 0); - registry->FinishThread(i); + registry.DetachThread(i, 0); + registry.FinishThread(i); } for (u32 i = 0; i < new_tids.size(); i++) { u32 tid = new_tids[i]; - registry->StartThread(tid, 0, ThreadType::Regular, 0); - registry->DetachThread(tid, 0); - registry->FinishThread(tid); + registry.StartThread(tid, 0, ThreadType::Regular, 0); + registry.DetachThread(tid, 0); + registry.FinishThread(tid); } - CheckThreadQuantity(registry, exp_total, 1, 1); + CheckThreadQuantity(®istry, 16, 1); // Test methods that require the caller to hold a ThreadRegistryLock. bool has_tid[16]; internal_memset(&has_tid[0], 0, sizeof(has_tid)); { - ThreadRegistryLock l(registry); - registry->RunCallbackForEachThreadLocked(MarkUidAsPresent, &has_tid[0]); + ThreadRegistryLock l(®istry); + registry.RunCallbackForEachThreadLocked(MarkUidAsPresent, &has_tid[0]); } - for (u32 i = 0; i < exp_total; i++) { + for (u32 i = 0; i < 16; i++) { EXPECT_TRUE(has_tid[i]); } { - ThreadRegistryLock l(registry); - registry->CheckLocked(); - ThreadContextBase *main_thread = registry->GetThreadLocked(0); - EXPECT_EQ(main_thread, registry->FindThreadContextLocked( - HasUid, (void*)get_uid(0))); + ThreadRegistryLock l(®istry); + registry.CheckLocked(); + ThreadContextBase *main_thread = registry.GetThreadLocked(kMainTid); + EXPECT_EQ(main_thread, + registry.FindThreadContextLocked(HasUid, (void *)get_uid(0))); } - EXPECT_EQ(11U, registry->GetMaxAliveThreads()); -} - -TEST(SanitizerCommon, ThreadRegistryTest) { - ThreadRegistry quarantine_registry(GetThreadContext, - kMaxRegistryThreads, kRegistryQuarantine, - 0); - TestRegistry(&quarantine_registry, true); - - ThreadRegistry no_quarantine_registry(GetThreadContext, - kMaxRegistryThreads, - kMaxRegistryThreads, 0); - TestRegistry(&no_quarantine_registry, false); } static const int kThreadsPerShard = 20; @@ -226,8 +207,7 @@ memset(&num_started, 0, sizeof(num_created)); memset(&num_joined, 0, sizeof(num_created)); - ThreadRegistry registry(GetThreadContext, - kThreadsPerShard * kNumShards + 1, 10, 0); + ThreadRegistry registry(GetThreadContext); ThreadedTestRegistry(®istry); } diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp --- a/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp @@ -101,7 +101,7 @@ int ThreadCount(ThreadState *thr) { uptr result; - ctx->thread_registry.GetNumberOfThreads(0, 0, &result); + ctx->thread_registry.GetNumberOfThreads(nullptr, &result); return (int)result; }