From 6bbba133f0479e774c5911ad063a534fd7bb4c63 Mon Sep 17 00:00:00 2001 From: rdb Date: Thu, 4 Jan 2024 17:15:11 +0100 Subject: [PATCH] pipeline: Fix `Thread::bind_thread()` regression Fixes #1575 --- panda/src/pipeline/thread.cxx | 15 +++++++++------ panda/src/pipeline/threadDummyImpl.I | 8 +++++--- panda/src/pipeline/threadDummyImpl.h | 2 +- panda/src/pipeline/threadPosixImpl.cxx | 11 ++++++++--- panda/src/pipeline/threadPosixImpl.h | 2 +- panda/src/pipeline/threadSimpleImpl.I | 6 ++++-- panda/src/pipeline/threadSimpleImpl.h | 2 +- panda/src/pipeline/threadWin32Impl.cxx | 11 ++++++++--- panda/src/pipeline/threadWin32Impl.h | 2 +- 9 files changed, 38 insertions(+), 21 deletions(-) diff --git a/panda/src/pipeline/thread.cxx b/panda/src/pipeline/thread.cxx index 15b31d96db..179fdba442 100644 --- a/panda/src/pipeline/thread.cxx +++ b/panda/src/pipeline/thread.cxx @@ -92,16 +92,19 @@ Thread:: */ PT(Thread) Thread:: bind_thread(const std::string &name, const std::string &sync_name) { - Thread *current_thread = get_current_thread(); - if (current_thread != get_external_thread()) { + PT(Thread) thread = new ExternalThread(name, sync_name); +#ifndef HAVE_THREADS + Thread *current_thread = get_main_thread(); +#else + Thread *current_thread = ThreadImpl::bind_thread(thread); +#endif + if (current_thread != nullptr && + current_thread != thread.p()) { // This thread already has an associated thread. nassertr(current_thread->get_name() == name && current_thread->get_sync_name() == sync_name, current_thread); - return current_thread; + thread = current_thread; } - - PT(Thread) thread = new ExternalThread(name, sync_name); - ThreadImpl::bind_thread(thread); return thread; } diff --git a/panda/src/pipeline/threadDummyImpl.I b/panda/src/pipeline/threadDummyImpl.I index 85ab2263a4..2065f0ad28 100644 --- a/panda/src/pipeline/threadDummyImpl.I +++ b/panda/src/pipeline/threadDummyImpl.I @@ -63,13 +63,15 @@ prepare_for_exit() { } /** - * Associates the indicated Thread object with the currently-executing thread. + * Associates the indicated Thread object with the currently-executing thread, + * unless a thread is already bound, in which case it is returned. * You should not call this directly; use Thread::bind_thread() instead. */ -INLINE void ThreadDummyImpl:: +INLINE Thread *ThreadDummyImpl:: bind_thread(Thread *thread) { // This method shouldn't be called in the non-threaded case. - nassertv(false); + nassertr(false, nullptr); + return nullptr; } /** diff --git a/panda/src/pipeline/threadDummyImpl.h b/panda/src/pipeline/threadDummyImpl.h index 3e1b7a8ab7..82a21e05a9 100644 --- a/panda/src/pipeline/threadDummyImpl.h +++ b/panda/src/pipeline/threadDummyImpl.h @@ -55,7 +55,7 @@ public: INLINE static void prepare_for_exit(); static Thread *get_current_thread(); - INLINE static void bind_thread(Thread *thread); + INLINE static Thread *bind_thread(Thread *thread); INLINE static bool is_threading_supported(); INLINE static bool is_true_threads(); INLINE static bool is_simple_threads(); diff --git a/panda/src/pipeline/threadPosixImpl.cxx b/panda/src/pipeline/threadPosixImpl.cxx index 773061398e..9266ded933 100644 --- a/panda/src/pipeline/threadPosixImpl.cxx +++ b/panda/src/pipeline/threadPosixImpl.cxx @@ -193,18 +193,23 @@ get_unique_id() const { } /** - * Associates the indicated Thread object with the currently-executing thread. + * Associates the indicated Thread object with the currently-executing thread, + * unless a thread is already bound, in which case it is returned. * You should not call this directly; use Thread::bind_thread() instead. */ -void ThreadPosixImpl:: +Thread *ThreadPosixImpl:: bind_thread(Thread *thread) { - if (_current_thread == nullptr && thread == Thread::get_main_thread()) { + if (_current_thread != nullptr) { + return _current_thread; + } + if (thread == Thread::get_main_thread()) { _main_thread_known.test_and_set(std::memory_order_relaxed); } _current_thread = thread; #ifdef ANDROID bind_java_thread(); #endif + return thread; } #ifdef ANDROID diff --git a/panda/src/pipeline/threadPosixImpl.h b/panda/src/pipeline/threadPosixImpl.h index 65d7f951ca..34422506d7 100644 --- a/panda/src/pipeline/threadPosixImpl.h +++ b/panda/src/pipeline/threadPosixImpl.h @@ -49,7 +49,7 @@ public: INLINE static void prepare_for_exit(); INLINE static Thread *get_current_thread(); - static void bind_thread(Thread *thread); + static Thread *bind_thread(Thread *thread); INLINE static bool is_threading_supported(); INLINE static bool is_true_threads(); INLINE static bool is_simple_threads(); diff --git a/panda/src/pipeline/threadSimpleImpl.I b/panda/src/pipeline/threadSimpleImpl.I index d712892883..04cda3a99b 100644 --- a/panda/src/pipeline/threadSimpleImpl.I +++ b/panda/src/pipeline/threadSimpleImpl.I @@ -36,11 +36,13 @@ is_same_system_thread() const { } /** - * Associates the indicated Thread object with the currently-executing thread. + * Associates the indicated Thread object with the currently-executing thread, + * unless a thread is already bound, in which case it is returned. * You should not call this directly; use Thread::bind_thread() instead. */ -INLINE void ThreadSimpleImpl:: +INLINE Thread *ThreadSimpleImpl:: bind_thread(Thread *) { + return get_current_thread(); } /** diff --git a/panda/src/pipeline/threadSimpleImpl.h b/panda/src/pipeline/threadSimpleImpl.h index 5e747ea917..819ad1a71f 100644 --- a/panda/src/pipeline/threadSimpleImpl.h +++ b/panda/src/pipeline/threadSimpleImpl.h @@ -61,7 +61,7 @@ public: INLINE static Thread *get_current_thread(); INLINE bool is_same_system_thread() const; - INLINE static void bind_thread(Thread *thread); + INLINE static Thread *bind_thread(Thread *thread); INLINE static bool is_threading_supported(); static bool is_true_threads(); INLINE static bool is_simple_threads(); diff --git a/panda/src/pipeline/threadWin32Impl.cxx b/panda/src/pipeline/threadWin32Impl.cxx index 22bf5cab79..c8653a4f5c 100644 --- a/panda/src/pipeline/threadWin32Impl.cxx +++ b/panda/src/pipeline/threadWin32Impl.cxx @@ -184,15 +184,20 @@ get_current_thread() { } /** - * Associates the indicated Thread object with the currently-executing thread. + * Associates the indicated Thread object with the currently-executing thread, + * unless a thread is already bound, in which case it is returned. * You should not call this directly; use Thread::bind_thread() instead. */ -void ThreadWin32Impl:: +Thread *ThreadWin32Impl:: bind_thread(Thread *thread) { - if (_current_thread == nullptr && thread == Thread::get_main_thread()) { + if (_current_thread != nullptr) { + return _current_thread; + } + if (thread == Thread::get_main_thread()) { _main_thread_known.test_and_set(std::memory_order_relaxed); } _current_thread = thread; + return thread; } /** diff --git a/panda/src/pipeline/threadWin32Impl.h b/panda/src/pipeline/threadWin32Impl.h index 3c9cfd275e..1d62320789 100644 --- a/panda/src/pipeline/threadWin32Impl.h +++ b/panda/src/pipeline/threadWin32Impl.h @@ -44,7 +44,7 @@ public: INLINE static void prepare_for_exit(); static Thread *get_current_thread(); - static void bind_thread(Thread *thread); + static Thread *bind_thread(Thread *thread); INLINE static bool is_threading_supported(); INLINE static bool is_true_threads(); INLINE static bool is_simple_threads();