pipeline: give Mutex and ReMutex more Pythonic semantics

rdb · rdb · commit c4a01ac564ea · 2019-05-12T20:58:13.000+02:00
This allows using mutices in with-blocks and wraps up the functionality of acquire() and try_acquire() into a single acquire(blocking=True).

Furthermore, the GIL is no longer released in cases of no contention.
diff --git a/direct/src/stdpy/threading.py b/direct/src/stdpy/threading.py
@@ -201,17 +201,6 @@ class Lock(core.Mutex):
     def __init__(self, name = "PythonLock"):
         core.Mutex.__init__(self, name)
 
-    def acquire(self, blocking = True):
-        if blocking:
-            core.Mutex.acquire(self)
-            return True
-        else:
-            return core.Mutex.tryAcquire(self)
-
-    __enter__ = acquire
-
-    def __exit__(self, t, v, tb):
-        self.release()
 
 class RLock(core.ReMutex):
     """ This class provides a wrapper around Panda's ReMutex object.
@@ -221,18 +210,6 @@ class RLock(core.ReMutex):
     def __init__(self, name = "PythonRLock"):
         core.ReMutex.__init__(self, name)
 
-    def acquire(self, blocking = True):
-        if blocking:
-            core.ReMutex.acquire(self)
-            return True
-        else:
-            return core.ReMutex.tryAcquire(self)
-
-    __enter__ = acquire
-
-    def __exit__(self, t, v, tb):
-        self.release()
-
 
 class Condition(core.ConditionVarFull):
     """ This class provides a wrapper around Panda's ConditionVarFull
diff --git a/panda/src/pipeline/mutexDebug.I b/panda/src/pipeline/mutexDebug.I
@@ -70,6 +70,8 @@ acquire(Thread *current_thread) const {
 /**
  * Returns immediately, with a true value indicating the mutex has been
  * acquired, and false indicating it has not.
+ *
+ * @deprecated Python users should use acquire(False), C++ users try_lock()
  */
 INLINE bool MutexDebug::
 try_acquire(Thread *current_thread) const {
diff --git a/panda/src/pipeline/mutexDirect.I b/panda/src/pipeline/mutexDirect.I
@@ -60,6 +60,8 @@ acquire() const {
 /**
  * Returns immediately, with a true value indicating the mutex has been
  * acquired, and false indicating it has not.
+ *
+ * @deprecated Python users should use acquire(False), C++ users try_lock()
  */
 INLINE bool MutexDirect::
 try_acquire() const {
diff --git a/panda/src/pipeline/pmutex.h b/panda/src/pipeline/pmutex.h
@@ -49,6 +49,10 @@ class EXPCL_PANDA_PIPELINE Mutex : public MutexDirect
 
   void operator = (const Mutex &copy) = delete;
 
+  EXTENSION(bool acquire(bool blocking=true) const);
+  EXTENSION(bool __enter__());
+  EXTENSION(void __exit__(PyObject *, PyObject *, PyObject *));
+
 public:
   // This is a global mutex set aside for the purpose of protecting Notify
   // messages from being interleaved between threads.
diff --git a/panda/src/pipeline/pmutex_ext.I b/panda/src/pipeline/pmutex_ext.I
@@ -0,0 +1,53 @@
+/**
+ * PANDA 3D SOFTWARE
+ * Copyright (c) Carnegie Mellon University.  All rights reserved.
+ *
+ * All use of this software is subject to the terms of the revised BSD
+ * license.  You should have received a copy of this license along
+ * with this source code in a file named "LICENSE."
+ *
+ * @file pmutex_ext.h
+ * @author rdb
+ * @date 2019-05-12
+ */
+
+/**
+ * Acquires the mutex.
+ */
+INLINE bool Extension<Mutex>::
+acquire(bool blocking) const {
+  if (_this->try_lock()) {
+    return true;
+  }
+
+  if (!blocking) {
+    return false;
+  }
+
+  // Release the GIL while we are waiting for the lock.
+#if defined(HAVE_THREADS) && !defined(SIMPLE_THREADS)
+  PyThreadState *_save;
+  Py_UNBLOCK_THREADS
+  _this->lock();
+  Py_BLOCK_THREADS
+#else
+  _this->lock();
+#endif
+  return true;
+}
+
+/**
+ * Acquires the mutex.
+ */
+INLINE bool Extension<Mutex>::
+__enter__() {
+  return acquire(true);
+}
+
+/**
+ * Releases the mutex.
+ */
+INLINE void Extension<Mutex>::
+__exit__(PyObject *, PyObject *, PyObject *) {
+  _this->unlock();
+}
diff --git a/panda/src/pipeline/pmutex_ext.h b/panda/src/pipeline/pmutex_ext.h
@@ -0,0 +1,41 @@
+/**
+ * PANDA 3D SOFTWARE
+ * Copyright (c) Carnegie Mellon University.  All rights reserved.
+ *
+ * All use of this software is subject to the terms of the revised BSD
+ * license.  You should have received a copy of this license along
+ * with this source code in a file named "LICENSE."
+ *
+ * @file pmutex_ext.h
+ * @author rdb
+ * @date 2019-05-12
+ */
+
+#ifndef PMUTEX_EXT_H
+#define PMUTEX_EXT_H
+
+#include "dtoolbase.h"
+
+#ifdef HAVE_PYTHON
+
+#include "extension.h"
+#include "pmutex.h"
+#include "py_panda.h"
+
+/**
+ * This class defines the extension methods for Mutex, which are called
+ * instead of any C++ methods with the same prototype.
+ */
+template<>
+class Extension<Mutex> : public ExtensionBase<Mutex> {
+public:
+  INLINE bool acquire(bool blocking) const;
+  INLINE bool __enter__();
+  INLINE void __exit__(PyObject *, PyObject *, PyObject *);
+};
+
+#include "pmutex_ext.I"
+
+#endif  // HAVE_PYTHON
+
+#endif  // PMUTEX_EXT_H
diff --git a/panda/src/pipeline/reMutex.h b/panda/src/pipeline/reMutex.h
@@ -42,6 +42,10 @@ class EXPCL_PANDA_PIPELINE ReMutex : public ReMutexDirect
   ~ReMutex() = default;
 
   void operator = (const ReMutex &copy) = delete;
+
+  EXTENSION(bool acquire(bool blocking=true) const);
+  EXTENSION(bool __enter__());
+  EXTENSION(void __exit__(PyObject *, PyObject *, PyObject *));
 };
 
 #include "reMutex.I"
diff --git a/panda/src/pipeline/reMutexDirect.I b/panda/src/pipeline/reMutexDirect.I
@@ -105,6 +105,8 @@ acquire(Thread *current_thread) const {
 /**
  * Returns immediately, with a true value indicating the mutex has been
  * acquired, and false indicating it has not.
+ *
+ * @deprecated Python users should use acquire(False), C++ users try_lock()
  */
 INLINE bool ReMutexDirect::
 try_acquire() const {
@@ -119,6 +121,8 @@ try_acquire() const {
 /**
  * Returns immediately, with a true value indicating the mutex has been
  * acquired, and false indicating it has not.
+ *
+ * @deprecated Python users should use acquire(False), C++ users try_lock()
  */
 INLINE bool ReMutexDirect::
 try_acquire(Thread *current_thread) const {
diff --git a/panda/src/pipeline/reMutex_ext.I b/panda/src/pipeline/reMutex_ext.I
@@ -0,0 +1,53 @@
+/**
+ * PANDA 3D SOFTWARE
+ * Copyright (c) Carnegie Mellon University.  All rights reserved.
+ *
+ * All use of this software is subject to the terms of the revised BSD
+ * license.  You should have received a copy of this license along
+ * with this source code in a file named "LICENSE."
+ *
+ * @file pmutex_ext.h
+ * @author rdb
+ * @date 2019-05-12
+ */
+
+/**
+ * Acquires the mutex.
+ */
+INLINE bool Extension<ReMutex>::
+acquire(bool blocking) const {
+  if (_this->try_lock()) {
+    return true;
+  }
+
+  if (!blocking) {
+    return false;
+  }
+
+  // Release the GIL while we are waiting for the lock.
+#if defined(HAVE_THREADS) && !defined(SIMPLE_THREADS)
+  PyThreadState *_save;
+  Py_UNBLOCK_THREADS
+  _this->lock();
+  Py_BLOCK_THREADS
+#else
+  _this->lock();
+#endif
+  return true;
+}
+
+/**
+ * Acquires the mutex.
+ */
+INLINE bool Extension<ReMutex>::
+__enter__() {
+  return acquire(true);
+}
+
+/**
+ * Releases the mutex.
+ */
+INLINE void Extension<ReMutex>::
+__exit__(PyObject *, PyObject *, PyObject *) {
+  _this->unlock();
+}
diff --git a/panda/src/pipeline/reMutex_ext.h b/panda/src/pipeline/reMutex_ext.h
@@ -0,0 +1,41 @@
+/**
+ * PANDA 3D SOFTWARE
+ * Copyright (c) Carnegie Mellon University.  All rights reserved.
+ *
+ * All use of this software is subject to the terms of the revised BSD
+ * license.  You should have received a copy of this license along
+ * with this source code in a file named "LICENSE."
+ *
+ * @file remutex_ext.h
+ * @author rdb
+ * @date 2019-05-12
+ */
+
+#ifndef REMUTEX_EXT_H
+#define REMUTEX_EXT_H
+
+#include "dtoolbase.h"
+
+#ifdef HAVE_PYTHON
+
+#include "extension.h"
+#include "reMutex.h"
+#include "py_panda.h"
+
+/**
+ * This class defines the extension methods for ReMutex, which are called
+ * instead of any C++ methods with the same prototype.
+ */
+template<>
+class Extension<ReMutex> : public ExtensionBase<ReMutex> {
+public:
+  INLINE bool acquire(bool blocking) const;
+  INLINE bool __enter__();
+  INLINE void __exit__(PyObject *, PyObject *, PyObject *);
+};
+
+#include "reMutex_ext.I"
+
+#endif  // HAVE_PYTHON
+
+#endif  // REMUTEX_EXT_H
diff --git a/tests/pipeline/test_condition_var.py b/tests/pipeline/test_condition_var.py
@@ -26,13 +26,12 @@ def test_cvar_notify_locked():
     m = Mutex()
     cv = ConditionVarFull(m)
 
-    m.acquire()
-    cv.notify()
-    m.release()
+    with m:
+        cv.notify()
+
+    with m:
+        cv.notify_all()
 
-    m.acquire()
-    cv.notify_all()
-    m.release()
     del cv
 
 
diff --git a/tests/pipeline/test_mutex.py b/tests/pipeline/test_mutex.py
@@ -2,6 +2,7 @@
 from panda3d import core
 from random import random
 import pytest
+import sys
 
 
 def test_mutex_acquire_release():
@@ -34,6 +35,19 @@ def test_mutex_try_acquire():
     m.release()
 
 
+def test_mutex_with():
+    m = Mutex()
+
+    rc = sys.getrefcount(m)
+    with m:
+        assert m.debug_is_locked()
+
+    with m:
+        assert m.debug_is_locked()
+
+    assert rc == sys.getrefcount(m)
+
+
 @pytest.mark.skipif(not core.Thread.is_threading_supported(),
                     reason="Threading support disabled")
 def test_mutex_contention():
@@ -124,3 +138,15 @@ def test_remutex_try_acquire():
     m.release()
     m.release()
 
+
+def test_remutex_with():
+    m = ReMutex()
+
+    rc = sys.getrefcount(m)
+    with m:
+        assert m.debug_is_locked()
+        with m:
+            assert m.debug_is_locked()
+        assert m.debug_is_locked()
+
+    assert rc == sys.getrefcount(m)
