Python gc in python thread

project.clj

@@ -5,7 +5,7 @@
             :url "https://www.eclipse.org/legal/epl-2.0/"}
   :dependencies [[org.clojure/clojure "1.10.1"]
                  [camel-snake-kebab "0.4.0"]
-                 [techascent/tech.datatype "4.69"]
+                 [techascent/tech.datatype "4.70"]
                  [org.clojure/data.json "0.2.7"]]
   :repl-options {:init-ns user}
   :java-source-paths ["java"])

src/libpython_clj/python/bridge.clj

@@ -40,24 +40,25 @@
              [expose-bridge-to-python!
               pybridge->bridge
               create-bridge-from-att-map]]
+            [libpython-clj.python.gc :as pygc]
             [clojure.stacktrace :as st]
             [tech.jna :as jna]
             [tech.v2.tensor :as dtt]
             [tech.v2.datatype.casting :as casting]
             [tech.v2.datatype.functional :as dtype-fn]
             [tech.v2.datatype :as dtype]
-            [tech.resource :as resource]
             [clojure.set :as c-set]
             [clojure.tools.logging :as log])
   (:import [java.util Map RandomAccess List Map$Entry Iterator UUID]
-           [java.util.concurrent ConcurrentHashMap]
+           [java.util.concurrent ConcurrentHashMap ConcurrentLinkedQueue]
            [clojure.lang IFn Symbol Keyword Seqable
             Fn MapEntry Range LongRange]
            [tech.v2.datatype ObjectReader ObjectWriter ObjectMutable
             ObjectIter MutableRemove]
            [tech.v2.datatype.typed_buffer TypedBuffer]
            [tech.v2.tensor.protocols PTensor]
            [com.sun.jna Pointer]
+           [tech.resource GCReference]
            [java.io Writer]
            [libpython_clj.jna JVMBridge
             CFunction$KeyWordFunction
@@ -539,8 +540,7 @@
           long-addr (get-attr ctypes "data")
           hash-ary {:ctypes-map ctypes}
           ptr-val (-> (Pointer. long-addr)
-                      (resource/track #(get hash-ary :ctypes-map)
-                                      pyobj/*pyobject-tracking-flags*))]
+                      (pygc/track #(get hash-ary :ctypes-map)))]
       {:ptr ptr-val
        :datatype np-dtype
        :shape shape
@@ -782,7 +782,7 @@
   [& body]
   `(delay
      (with-gil
-       (with-bindings {#'pyobj/*pyobject-tracking-flags* [:gc]}
+       (with-bindings {#'pygc/*stack-gc-context* nil}
          ~@body))))
 
 
@@ -1094,7 +1094,7 @@
                             initial-buffer
                             (->py-tuple shape)
                             (->py-tuple strides))]
-      (resource/track retval #(get buffer-desc :ptr) pyobj/*pyobject-tracking-flags*))))
+      (pygc/track retval #(get buffer-desc :ptr)))))
 
 
 (extend-type Object

src/libpython_clj/python/gc.clj

@@ -0,0 +1,67 @@
+(ns libpython-clj.python.gc
+  "Binding of various sort of gc semantics optimized specifically for
+  libpython-clj.  For general bindings, see tech.resource"
+  (:import [java.util.concurrent ConcurrentHashMap ConcurrentLinkedDeque]
+           [java.lang.ref ReferenceQueue]
+           [tech.resource GCReference]))
+
+
+(set! *warn-on-reflection* true)
+
+
+(defonce ^:dynamic *stack-gc-context* nil)
+(defn stack-context
+  ^ConcurrentLinkedDeque []
+  *stack-gc-context*)
+
+
+(defonce reference-queue-var (ReferenceQueue.))
+(defn reference-queue
+  ^ReferenceQueue []
+  reference-queue-var)
+
+
+(defonce ptr-set-var (ConcurrentHashMap/newKeySet))
+(defn ptr-set
+  ^java.util.Set []
+  ptr-set-var)
+
+
+(defn track
+  [item dispose-fn]
+  (let [ptr-val (GCReference. item (reference-queue) (fn [ptr-val]
+                                                       (.remove (ptr-set) ptr-val)
+                                                       (dispose-fn)))
+        ^ConcurrentLinkedDeque stack-context (stack-context)]
+    ;;We have to keep track of the pointer.  If we do not the pointer gets gc'd then
+    ;;it will not be put on the reference queue when the object itself is gc'd.
+    ;;Nice little gotcha there.
+    (if stack-context
+      (.add stack-context ptr-val)
+      ;;Ensure we don't lose track of the weak reference.  If it gets cleaned up
+      ;;the gc system will fail.
+      (.add (ptr-set) ptr-val))
+    item))
+
+
+(defn clear-reference-queue
+  []
+  (when-let [next-ref (.poll (reference-queue))]
+    (.run ^Runnable next-ref)
+    (recur)))
+
+
+(defn clear-stack-context
+  []
+  (when-let [next-ref (.pollLast (stack-context))]
+    (.run ^Runnable next-ref)
+    (recur)))
+
+
+(defmacro with-stack-context
+  [& body]
+  `(with-bindings {#'*stack-gc-context* (ConcurrentLinkedDeque.)}
+     (try
+       ~@body
+       (finally
+         (clear-stack-context)))))

src/libpython_clj/python/interpreter.clj

@@ -1,7 +1,7 @@
 (ns libpython-clj.python.interpreter
   (:require [libpython-clj.jna :as libpy ]
             [libpython-clj.jna.base :as libpy-base]
-            [tech.resource :as resource]
+            [libpython-clj.python.gc :as pygc]
             [libpython-clj.python.logging
              :refer [log-error log-warn log-info]]
             [tech.jna :as jna]
@@ -348,6 +348,7 @@ print(json.dumps(
            (try
              ~@body
              (finally
+               (pygc/clear-reference-queue)
                (when new-binding?#
                  (release-gil! interp#)))))))))
 
@@ -433,7 +434,7 @@ print(json.dumps(
           (recur library-names))))
     ;;Set program name
     (when-let [program-name (or program-name *program-name* "")]
-      (resource/stack-resource-context
+      (pygc/with-stack-context
        (libpy/PySys_SetArgv 0 (-> program-name
                                   (jna/string->wide-ptr)))))
     (let [type-symbols (libpy/lookup-type-symbols)

src/libpython_clj/python/object.clj

@@ -33,12 +33,13 @@
              :as py-proto]
             [libpython-clj.jna.base :as libpy-base]
             [libpython-clj.jna :as libpy]
+            [libpython-clj.python.gc :as pygc]
             [clojure.stacktrace :as st]
             [tech.jna :as jna]
-            [tech.resource :as resource]
             [tech.v2.datatype :as dtype]
             [tech.v2.datatype.protocols :as dtype-proto]
             [tech.v2.datatype.casting :as casting]
+            [tech.resource :as resource]
             [tech.v2.tensor]
             [clojure.tools.logging :as log])
   (:import [com.sun.jna Pointer CallbackReference]
@@ -96,13 +97,10 @@
     (py-proto/->jvm item options)))
 
 
-(def ^:dynamic *object-reference-logging* false)
-
+(def object-reference-logging nil)
 
-(def ^:dynamic *object-reference-tracker* nil)
 
-
-(def ^:dynamic *pyobject-tracking-flags* [:gc])
+(def object-reference-tracker nil)
 
 
 (defn incref
@@ -130,41 +128,44 @@
   (if (and pyobj
              (not= (Pointer/nativeValue (libpy/as-pyobj pyobj))
                    (Pointer/nativeValue (libpy/as-pyobj (libpy/Py_None)))))
-    (let [interpreter (ensure-bound-interpreter)
-          pyobj-value (Pointer/nativeValue (libpy/as-pyobj pyobj))
-          py-type-name (name (python-type pyobj))]
-      (when *object-reference-logging*
-        (let [obj-data (PyObject. (Pointer. pyobj-value))]
-          (println (format "tracking object  - 0x%x:%4d:%s"
-                            pyobj-value
-                            (.ob_refcnt obj-data)
-                            py-type-name))))
-      (when *object-reference-tracker*
-        (swap! *object-reference-tracker*
-               update pyobj-value #(inc (or % 0))))
-      ;;We ask the garbage collector to track the python object and notify
-      ;;us when it is released.  We then decref on that event.
-      (resource/track pyobj
-                      #(with-gil
-                         (try
-                           (let [refcount (refcount pyobj)
-                                 obj-data (PyObject. (Pointer. pyobj-value))]
-                             (if (< refcount 1)
-                               (log/errorf "Fatal error -- releasing object - 0x%x:%4d:%s
+    (do
+      (ensure-bound-interpreter)
+      (let [pyobj-value (Pointer/nativeValue (libpy/as-pyobj pyobj))
+            py-type-name (name (python-type pyobj))]
+        (when object-reference-logging
+          (let [obj-data (PyObject. (Pointer. pyobj-value))]
+            (println (format "tracking object  - 0x%x:%4d:%s"
+                             pyobj-value
+                             (.ob_refcnt obj-data)
+                             py-type-name))))
+        (when object-reference-tracker
+          (swap! object-reference-tracker
+                 update pyobj-value #(inc (or % 0))))
+        ;;We ask the garbage collector to track the python object and notify
+        ;;us when it is released.  We then decref on that event.
+        (pygc/track pyobj
+                    ;;No longer with-gil.  Because cleanup is cooperative, the gil is
+                    ;;guaranteed to be captured here already.
+                    #(try
+                       ;;Intentionally overshadow pyobj.  We cannot access it here.
+                       (let [pyobj (Pointer. pyobj-value)
+                             refcount (refcount pyobj)
+                             obj-data (PyObject. pyobj)]
+                         (if (< refcount 1)
+                           (log/errorf "Fatal error -- releasing object - 0x%x:%4d:%s
 Object's refcount is bad.  Crash is imminent" pyobj-value refcount py-type-name)
-                               (when *object-reference-logging*
-                                 (println (format "releasing object - 0x%x:%4d:%s"
-                                                  pyobj-value
-                                                  (.ob_refcnt obj-data)
-                                                  py-type-name))))
-                             (when *object-reference-tracker*
-                               (swap! *object-reference-tracker*
-                                      update pyobj-value (fn [arg]
-                                                           (dec (or arg 0))))))
-                           (libpy/Py_DecRef (Pointer. pyobj-value))
-                           (catch Throwable e
-                             (log/error e "Exception while releasing object"))))
-                      *pyobject-tracking-flags*))
+                           (when object-reference-logging
+                             (println (format "releasing object - 0x%x:%4d:%s"
+                                              pyobj-value
+                                              (.ob_refcnt obj-data)
+                                              py-type-name))))
+                         (when object-reference-tracker
+                           (swap! object-reference-tracker
+                                  update pyobj-value (fn [arg]
+                                                       (dec (or arg 0))))))
+                       (libpy/Py_DecRef (Pointer. pyobj-value))
+                       (catch Throwable e
+                         (log/error e "Exception while releasing object"))))))
     (do
       ;;Special handling for PyNone types
       (libpy/Py_DecRef pyobj)
@@ -173,9 +174,8 @@ Object's refcount is bad.  Crash is imminent" pyobj-value refcount py-type-name)
 
 (defmacro stack-resource-context
   [& body]
-  `(with-bindings {#'*pyobject-tracking-flags* [:stack :gc]}
-     (resource/stack-resource-context
-      ~@body)))
+  `(pygc/with-stack-context
+     ~@body))
 
 
 (defn incref-wrap-pyobject
@@ -391,37 +391,37 @@ Object's refcount is bad.  Crash is imminent" pyobj-value refcount py-type-name)
                                    doc
                                    function]
                             :as method-data}]
-  (resource/stack-resource-context
-   (when-not (cfunc-instance? function)
-     (throw (Exception.
-             (format "Callbacks must implement one of the CFunction interfaces:
+  ;;Here we really do need a resource stack context
+  (when-not (cfunc-instance? function)
+    (throw (Exception.
+            (format "Callbacks must implement one of the CFunction interfaces:
 %s" (type function)))))
-   (let [meth-flags (long (cond
-                            (instance? CFunction$NoArgFunction function)
-                            @libpy/METH_NOARGS
-
-                            (instance? CFunction$TupleFunction function)
-                            @libpy/METH_VARARGS
-
-                            (instance? CFunction$KeyWordFunction function)
-                            (bit-or @libpy/METH_KEYWORDS @libpy/METH_VARARGS)
-                            :else
-                            (throw (ex-info (format "Failed due to type: %s"
-                                                    (type function))
-                                            {}))))
-         name-ptr (jna/string->ptr name)
-         doc-ptr (jna/string->ptr doc)]
-     (set! (.ml_name method-def) name-ptr)
-     (set! (.ml_meth method-def) (CallbackReference/getFunctionPointer function))
-     (set! (.ml_flags method-def) (int meth-flags))
-     (set! (.ml_doc method-def) doc-ptr)
-     (.write method-def)
-     (pyinterp/conj-forever! (assoc method-data
-                                    :name-ptr name-ptr
-                                    :doc-ptr doc-ptr
-                                    :callback-object function
-                                    :method-definition method-def))
-     method-def)))
+  (let [meth-flags (long (cond
+                           (instance? CFunction$NoArgFunction function)
+                           @libpy/METH_NOARGS
+
+                           (instance? CFunction$TupleFunction function)
+                           @libpy/METH_VARARGS
+
+                           (instance? CFunction$KeyWordFunction function)
+                           (bit-or @libpy/METH_KEYWORDS @libpy/METH_VARARGS)
+                           :else
+                           (throw (ex-info (format "Failed due to type: %s"
+                                                   (type function))
+                                           {}))))
+        name-ptr (jna/string->ptr-untracked name)
+        doc-ptr (jna/string->ptr-untracked doc)]
+    (set! (.ml_name method-def) name-ptr)
+    (set! (.ml_meth method-def) (CallbackReference/getFunctionPointer function))
+    (set! (.ml_flags method-def) (int meth-flags))
+    (set! (.ml_doc method-def) doc-ptr)
+    (.write method-def)
+    (pyinterp/conj-forever! (assoc method-data
+                                   :name-ptr name-ptr
+                                   :doc-ptr doc-ptr
+                                   :callback-object function
+                                   :method-definition method-def))
+    method-def))
 
 
 (defn method-def-data->method-def

test/libpython_clj/stress_test.clj

@@ -56,10 +56,6 @@ def getmultidata():
     (gd-fn)))
 
 
-;;If you want to see how the sausage is made...
-(alter-var-root #'libpython-clj.python.object/*object-reference-logging*
-                (constantly false))
-
 ;;Ensure that failure to open resource context before tracking for stack
 ;;related things causes immediate failure.  Unless you feel like being pedantic,
 ;;this isn't necessary.  The python library automatically switches to normal gc-only