Skip to content

Commit

Permalink
Fix cleanup by introducing a thread registry
Browse files Browse the repository at this point in the history
  • Loading branch information
dvicini committed Sep 12, 2024
1 parent 6b46f37 commit 2e43390
Showing 1 changed file with 27 additions and 3 deletions.
30 changes: 27 additions & 3 deletions src/core/thread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <sstream>
#include <chrono>
#include <cstring>
#include <unordered_map>

// Required for native thread functions
#if defined(__linux__)
Expand All @@ -35,6 +36,9 @@ static __declspec(thread) int this_thread_id;
static std::mutex task_mutex;
static std::vector<Task *> registered_tasks;

static std::mutex thread_registry_mutex;
static std::unordered_map<std::string, Thread*> thread_registry;

#if defined(_MSC_VER)
namespace {
// Helper function to set a native thread name. MSDN:
Expand Down Expand Up @@ -78,13 +82,19 @@ class MainThread : public Thread {
/// Dummy class to associate a thread identity with a worker thread
class WorkerThread : public Thread {
public:
WorkerThread(const std::string &prefix) : Thread(tfm::format("%s%i", prefix, m_counter++)) { }
WorkerThread(const std::string &prefix) : Thread(tfm::format("%s%i", prefix, m_counter++)) {
std::lock_guard guard(thread_registry_mutex);
thread_registry[this->name()] = this;
}

virtual void run() override { Throw("The worker thread is already running!"); }

MI_DECLARE_CLASS()
protected:
virtual ~WorkerThread() { }
virtual ~WorkerThread() {
std::lock_guard guard(thread_registry_mutex);
thread_registry.erase(this->name());
}
static std::atomic<uint32_t> m_counter;
};

Expand Down Expand Up @@ -478,7 +488,7 @@ bool Thread::register_external_thread(const std::string &prefix) {
self->d->running = true;
self->d->external_thread = true;

// An external thread will re-use the main thread's Logger (thread safe)
// An external thread will re-use the main thread's Logger (thread safe)
// and create a new FileResolver (since the FileResolver is not thread safe).
self->d->logger = main_thread->d->logger;
self->d->fresolver = new FileResolver();
Expand Down Expand Up @@ -535,6 +545,20 @@ void Thread::static_shutdown() {
task_wait_and_release(task);
registered_tasks.clear();

/* Remove references to Loggers and FileResolvers from worker threads.
* _Important_: This might locally acquire the Python GIL due to reference
* counting. To prevent deadlocks, it's important that Thread::static_shutdown
* is run with the GIL acquired already. This ensures consistent lock
* acquisition order and prevents deadlocks. */
{
std::lock_guard guard(thread_registry_mutex);
for (auto &thread : thread_registry) {
thread.second->d->logger = nullptr;
thread.second->d->fresolver = nullptr;
}
thread_registry.clear();
}

thread()->d->running = false;
self = nullptr;
main_thread = nullptr;
Expand Down

0 comments on commit 2e43390

Please sign in to comment.