From 8e1faa9df16896149e8169ec1d012a10308d1fc5 Mon Sep 17 00:00:00 2001
From: mxmlnkn <mxmlnkn@github.de>
Date: Sat, 25 Nov 2023 15:07:28 +0100
Subject: [PATCH] [wip] use block-scoped thread_local

---
 src/core/filereader/Python.hpp | 142 +++++++++++++++------------------
 1 file changed, 66 insertions(+), 76 deletions(-)

diff --git a/src/core/filereader/Python.hpp b/src/core/filereader/Python.hpp
index 2fe58b05..722070e2 100644
--- a/src/core/filereader/Python.hpp
+++ b/src/core/filereader/Python.hpp
@@ -34,86 +34,21 @@ class ScopedGIL
         size_t counter{ 0 };
     };
 
-private:
-    class GILMutex
-    {
-    public:
-        GILMutex() :
-            m_isLocked( PyGILState_Check() == 1 ),
-            m_isPythonThread( m_isLocked )
-        {
-            std::cerr << "[" << std::this_thread::get_id() << "][GILMutex::GILMutex] " << this << ", is locked: "
-                      << m_isLocked << "\n";
-        }
-
-        void
-        lock( bool doLock = true )
-        {
-            std::cerr << "[" << std::this_thread::get_id() << "][GILMutex::lock] " << this << ", lock: " << doLock
-                      << ", is locked: " << m_isLocked << ", is Python thread: " << m_isPythonThread << "\n";
-
-            if ( m_isLocked == doLock ) {
-                return;
-            }
-
-            if ( doLock ) {
-                if ( m_isPythonThread ) {
-                    PyEval_RestoreThread( m_unlockState );
-                    m_unlockState = nullptr;
-                } else {
-                    m_lockState = PyGILState_Ensure();
-                }
-            } else {
-                if ( m_isPythonThread ) {
-                    m_unlockState = PyEval_SaveThread();
-                } else {
-                    PyGILState_Release( m_lockState );
-                    m_lockState = {};
-                }
-            }
-
-            m_isLocked = doLock;
-
-            std::cerr << "[" << std::this_thread::get_id() << "][GILMutex::lock] " << this << ", new lock state: "
-                      << m_isLocked << "\n";
-        }
-
-        ~GILMutex()
-        {
-            std::cerr << "[" << std::this_thread::get_id() << "][GILMutex::~GILMutex] " << this << "\n";
-        }
-
-        [[nodiscard]] constexpr bool
-        isLocked() const
-        {
-            return m_isLocked;
-        }
-
-    private:
-        bool m_isLocked;
-        const bool m_isPythonThread;
-
-        /** Used for locking non-Python threads. */
-        PyGILState_STATE m_lockState{};
-        /** Used for unlocking and relocking the Python main thread. */
-        PyThreadState* m_unlockState{ nullptr };
-    };
-
 public:
-    ScopedGIL( bool lock )
+    ScopedGIL( bool doLock )
     {
-        std::cerr << "[" << std::this_thread::get_id() << "][ScopedGIL::ScopedGIL] lock: " << lock << "\n";
-        if ( m_referenceCounters.empty() && ( m_mutex.isLocked() == lock ) ) {
-            std::cerr << "  Ignore because initial state is as desired.\n";
-            return;
-        }
+        std::cerr << "[" << std::this_thread::get_id() << "][ScopedGIL::ScopedGIL] lock: " << doLock << "\n";
 
-        if ( !m_referenceCounters.empty() && ( m_referenceCounters.back().isLocked == lock ) ) {
+        if ( !m_referenceCounters.empty() && ( m_referenceCounters.back().isLocked == doLock ) ) {
             std::cerr << "  Increase counter because last state is as desired.\n";
             ++m_referenceCounters.back().counter;
         } else {
-            m_referenceCounters.emplace_back( ReferenceCounter{ lock, 1 } );
-            m_mutex.lock( lock );
+            const auto wasLocked = lock( doLock );
+            if ( !m_referenceCounters.empty() || ( wasLocked != doLock ) ) {
+                m_referenceCounters.emplace_back( ReferenceCounter{ doLock, 1 } );
+            } else {
+                std::cerr << "  Ignore because initial state is as desired.\n";
+            }
         }
     }
 
@@ -136,7 +71,7 @@ class ScopedGIL
 
         --m_referenceCounters.back().counter;
         if ( m_referenceCounters.back().counter == 0 ) {
-            m_mutex.lock( !m_referenceCounters.back().isLocked );
+            lock( !m_referenceCounters.back().isLocked );
             m_referenceCounters.pop_back();
         }
     }
@@ -147,7 +82,62 @@ class ScopedGIL
     ScopedGIL& operator=( ScopedGIL&& ) = delete;
 
 private:
-    inline static thread_local GILMutex m_mutex;
+    /**
+     * @return the old lock state.
+     */
+    bool
+    lock( bool doLock = true )
+    {
+        /**
+         * I would have liked a GILMutex class that can be declared as a static thread_local member but
+         * on Windows, these members are initialized too soon, i.e., at static initialization time instead of
+         * on first usage, which leads to bugs because PyGILState_Check will return 0 at this point.
+         * Therefore, use block-scoped thread_local variables, which are initialized on first pass as per the standard.
+         * @see https://stackoverflow.com/a/49821006/2191065
+         */
+        static thread_local bool isLocked{ PyGILState_Check() == 1 };
+        static thread_local bool const isPythonThread{ isLocked };
+
+        /** Used for locking non-Python threads. */
+        static thread_local PyGILState_STATE lockState{};
+        /** Used for unlocking and relocking the Python main thread. */
+        static thread_local PyThreadState* unlockState{ nullptr };
+
+        std::cerr << "[" << std::this_thread::get_id() << "][GILMutex::GILMutex] " << this << ", is locked: "
+                  << isLocked << "\n";
+
+        std::cerr << "[" << std::this_thread::get_id() << "][GILMutex::lock] " << this << ", lock: " << doLock
+                  << ", is locked: " << isLocked << ", is Python thread: " << isPythonThread << "\n";
+
+        const auto wasLocked = isLocked;
+        if ( isLocked == doLock ) {
+            return wasLocked;
+        }
+
+        if ( doLock ) {
+            if ( isPythonThread ) {
+                PyEval_RestoreThread( unlockState );
+                unlockState = nullptr;
+            } else {
+                lockState = PyGILState_Ensure();
+            }
+        } else {
+            if ( isPythonThread ) {
+                unlockState = PyEval_SaveThread();
+            } else {
+                PyGILState_Release( lockState );
+                lockState = {};
+            }
+        }
+
+        isLocked = doLock;
+
+        std::cerr << "[" << std::this_thread::get_id() << "][GILMutex::lock] " << this << ", new lock state: "
+                  << isLocked << "\n";
+        return wasLocked;
+    }
+
+private:
     inline static thread_local std::vector<ReferenceCounter> m_referenceCounters;
 };