From 1dd0183dba05c17d0c21e3f9b3d73d0fee257a2d Mon Sep 17 00:00:00 2001
From: Andreas Kloeckner <inform@tiker.net>
Date: Sun, 15 Sep 2019 20:11:43 -0500
Subject: [PATCH] Do not release GIL in NannyEvent destructor (closes gh-296)

---
 src/wrap_cl.hpp      | 32 ++++++++++++++++++++++++++++++--
 test/test_wrapper.py | 27 +++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/src/wrap_cl.hpp b/src/wrap_cl.hpp
index 71e3d067..ec4854c0 100644
--- a/src/wrap_cl.hpp
+++ b/src/wrap_cl.hpp
@@ -1560,6 +1560,14 @@ namespace pyopencl
         PYOPENCL_CALL_GUARDED_THREADED(clWaitForEvents, (1, &m_event));
       }
 
+      // Called from a destructor context below:
+      // - Should not release the GIL
+      // - Should fail gracefully in the face of errors
+      virtual void wait_during_cleanup_without_releasing_the_gil()
+      {
+        PYOPENCL_CALL_GUARDED_CLEANUP(clWaitForEvents, (1, &m_event));
+      }
+
 #if PYOPENCL_CL_VERSION >= 0x1010
     // {{{ set_callback, by way of a a thread-based construction
 
@@ -1688,7 +1696,11 @@ namespace pyopencl
       { }
 
       ~nanny_event()
-      { wait(); }
+      {
+        // It appears that Pybind can get very confused if we release the GIL here:
+        // https://github.com/inducer/pyopencl/issues/296
+        wait_during_cleanup_without_releasing_the_gil();
+      }
 
       py::object get_ward() const
       {
@@ -1705,6 +1717,12 @@ namespace pyopencl
         event::wait();
         m_ward.reset();
       }
+
+      virtual void wait_during_cleanup_without_releasing_the_gil()
+      {
+        event::wait_during_cleanup_without_releasing_the_gil();
+        m_ward.reset();
+      }
   };
 #else
   class nanny_event : public event
@@ -1726,7 +1744,11 @@ namespace pyopencl
       { }
 
       ~nanny_event()
-      { wait(); }
+      {
+        // It appears that Pybind can get very confused if we release the GIL here:
+        // https://github.com/inducer/pyopencl/issues/296
+        wait_during_cleanup_without_releasing_the_gil();
+      }
 
       py::object get_ward() const
       { return m_ward; }
@@ -1736,6 +1758,12 @@ namespace pyopencl
         event::wait();
         m_ward = py::none();
       }
+
+      virtual void wait_during_cleanup_without_releasing_the_gil()
+      {
+        event::wait_during_cleanup_without_releasing_the_gil();
+        m_ward = py::none();
+      }
   };
 #endif
 
diff --git a/test/test_wrapper.py b/test/test_wrapper.py
index e0522955..75b39b49 100644
--- a/test/test_wrapper.py
+++ b/test/test_wrapper.py
@@ -1112,6 +1112,33 @@ def test_copy_buffer_rect(ctx_factory):
             region=arr1.shape[::-1])
 
 
+def test_threaded_nanny_events(ctx_factory):
+    # https://github.com/inducer/pyopencl/issues/296
+
+    import gc
+    import threading
+
+    def create_arrays_thread(n1=10, n2=20):
+        ctx = ctx_factory()
+        queue = cl.CommandQueue(ctx)
+        for i1 in range(n2):
+            for i in range(n1):
+                acl = cl.array.zeros(queue, 10, dtype=np.float32)
+                acl.get()
+            # Garbage collection triggers the error
+            print("collected ", str(gc.collect()))
+            print("stats ", gc.get_stats())
+
+    t1 = threading.Thread(target=create_arrays_thread)
+    t2 = threading.Thread(target=create_arrays_thread)
+
+    t1.start()
+    t2.start()
+
+    t1.join()
+    t2.join()
+
+
 if __name__ == "__main__":
     # make sure that import failures get reported, instead of skipping the tests.
     import pyopencl  # noqa
-- 
GitLab