Skip to content

Commit

Permalink
⚒️fixed some data race
Browse files Browse the repository at this point in the history
🚫disabled TSan as too strict
  • Loading branch information
Lord-Turmoil committed Nov 28, 2024
1 parent 0fbaf8b commit edf93c4
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 12 deletions.
3 changes: 2 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ if (CMAKE_BUILD_TYPE STREQUAL "Debug")
add_compile_definitions(DEBUG)
# ASan and TSan are incompatible, so we can't use them together.
# And because we use std::shared_ptr, it is most likely that there is no need for ASan.
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=undefined,thread")
# However, TSan is somehow too strict, enable it only temporarily.
# set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=undefined,thread")
endif()

# strict warnings
Expand Down
14 changes: 5 additions & 9 deletions src/threading/Worker.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include "threading/Queue.h"

#include <atomic>
#include <chrono>
#include <thread>

Expand Down Expand Up @@ -66,8 +67,6 @@ template <typename TTask> class Worker final
void Start(Worker* donor);
void Stop();

void Shutdown();

/**
* @brief Post a task to the worker.
* @tparam T For std::forward to work
Expand All @@ -83,8 +82,7 @@ template <typename TTask> class Worker final
private:
Queue<TTask> _queue;
std::thread _thread;

bool _running;
std::atomic<bool> _running;
};

template <typename TTask>
Expand Down Expand Up @@ -112,15 +110,13 @@ template <typename TTask> Worker<TTask>& Worker<TTask>::operator=(Worker&& other

template <typename TTask> void Worker<TTask>::Start(Worker* donor)
{
MINET_ASSERT(!_running, "Worker is already running");
_running = true;
_running.store(true, std::memory_order_relaxed);
_thread = std::thread(&Worker::_Routine, this, donor);
}

template <typename TTask> void Worker<TTask>::Stop()
{
MINET_ASSERT(_running, "Working is not running");
_running = false;
_running.store(false, std::memory_order_relaxed);
_thread.join();
}

Expand All @@ -137,7 +133,7 @@ template <typename TTask> bool Worker<TTask>::_Steal(TTask* task)
template <typename TTask> void Worker<TTask>::_Routine(Worker* donor)
{
TTask task;
while (_running)
while (_running.load(std::memory_order_relaxed))
{
// Get one from the worker itself, or steal one to keep busy.
if (_queue.Pop(&task) || donor->_Steal(&task))
Expand Down
4 changes: 2 additions & 2 deletions tests/ThreadPoolTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ TEST_CASE("ThreadPool")
std::packaged_task<int()> tasks[TASK_NUM];

// Make it busier?
ThreadPool pool(2, TASK_NUM / 2);
unsigned int threads = HardwareConcurrency();
ThreadPool pool(threads, TASK_NUM / threads);
for (int i = 0; i < TASK_NUM; i++)
{
tasks[i] = std::packaged_task<int()>([i]() {
Expand All @@ -29,7 +30,6 @@ TEST_CASE("ThreadPool")
// clang-format off
CHECK_EQ(pool.Submit([&tasks, i]() {
tasks[i]();
return i;
}), true);
// clang-format on
}
Expand Down

0 comments on commit edf93c4

Please sign in to comment.