Merge pull request bendudson#12 from bendudson/memory-leaks

Fix some memory leaks

py4cl.asd

@@ -11,7 +11,6 @@
   :components ((:file "package")
                (:file "reader")
                (:file "writer")
-               (:file "callbacks")
                (:file "python-process")
                (:file "lisp-classes")
                (:file "callpython")

py4cl.py

@@ -27,6 +27,55 @@ def __str__(self):
     def __repr__(self):
         return "Symbol("+self._name+")"
 
+class LispCallbackObject (object):
+    """
+    Represents a lisp function which can be called. 
+    
+    An object is used rather than a lambda, so that the lifetime
+    can be monitoried, and the function removed from a hash map
+    """
+    def __init__(self, handle):
+        """
+        handle    A number, used to refer to the object in Lisp
+        """
+        self.handle = handle
+
+    def __del__(self):
+        """
+        Delete this object, sending a message to Lisp
+        """
+        try:
+            sys.stdout = write_stream
+            write_stream.write("d")
+            send_value(self.handle)
+        finally:
+            sys.stdout = redirect_stream
+
+    def __call__(self, *args, **kwargs):
+        """
+        Call back to Lisp
+        
+        args   Arguments to be passed to the function
+        """
+
+        # Convert kwargs into a sequence of ":keyword value" pairs
+        # appended to the positional arguments
+        allargs = args
+        for key, value in kwargs.items():
+            allargs += (Symbol(":"+str(key)), value)
+
+        try:
+            sys.stdout = write_stream
+            write_stream.write("c")
+            send_value((self.handle, allargs))
+        finally:
+            sys.stdout = redirect_stream
+
+        # Wait for a value to be returned.
+        # Note that the lisp function may call python before returning
+        return message_dispatch_loop()
+
+
 class UnknownLispObject (object):
     """
     Represents an object in Lisp, which could not be converted to Python
@@ -323,39 +372,13 @@ def message_dispatch_loop():
         except Exception as e:
             return_error(e)
 
-
-def callback_func(ident, *args, **kwargs):
-    """
-    Call back to Lisp
-
-    ident  Uniquely identifies the function to call
-    args   Arguments to be passed to the function
-    """
-
-    # Convert kwargs into a sequence of ":keyword value" pairs
-    # appended to the positional arguments
-    allargs = args
-    for key, value in kwargs.items():
-        allargs += (Symbol(":"+str(key)), value)
-
-    try:
-        sys.stdout = write_stream
-        write_stream.write("c")
-        send_value((ident, allargs))
-    finally:
-        sys.stdout = redirect_stream
-
-    # Wait for a value to be returned.
-    # Note that the lisp function may call python before returning
-    return message_dispatch_loop()
-
 
 # Store for python objects which can't be translated to Lisp objects
 python_objects = {}
 python_handle = itertools.count(0) # Running counter
 
 # Make callback function accessible to evaluation
-eval_globals["_py4cl_callback"] = callback_func
+eval_globals["_py4cl_LispCallbackObject"] = LispCallbackObject
 eval_globals["_py4cl_Symbol"] = Symbol
 eval_globals["_py4cl_UnknownLispObject"] = UnknownLispObject
 eval_globals["_py4cl_objects"] = python_objects

src/callpython.lisp

@@ -53,7 +53,7 @@
          ;; Callback. Value returned is a list, containing the function ID then the args
          (#\c
           (let ((call-value (stream-read-value read-stream)))
-            (let ((return-value (apply (get-callback (first call-value)) (second call-value))))
+            (let ((return-value (apply (lisp-object (first call-value)) (second call-value))))
               ;; Send a reply
               (dispatch-reply write-stream return-value))))
          (otherwise (error "Unhandled message type"))))))

src/import-export.lisp

@@ -108,7 +108,10 @@ RELOAD specifies that the package should be deleted and reloaded.
 
 (defun export-function (function python-name)
   "Makes a lisp FUNCTION available in python process as PYTHON-NAME"
-  (let ((id (register-callback function)))
-    (python-exec (concatenate 'string
-                              "def " python-name "(*args, **kwargs):
-    return _py4cl_callback(" (write-to-string id) ", *args, **kwargs)"))))
+  (python-exec (concatenate 'string
+                            python-name
+                            "=_py4cl_LispCallbackObject("
+                            (write-to-string
+                             (object-handle function))
+                            ")")))
+

src/python-process.lisp

@@ -9,6 +9,11 @@ e.g. \"python\" or \"python3\"")
 (defvar *python* nil
   "Most recently started python subprocess")
 
+(defvar *current-python-process-id* 0
+  "A number which changes when python is started. This
+is used to prevent garbage collection from deleting objects in the wrong
+python session")
+
 (defun python-start (&optional (command *python-command*))
   "Start a new python subprocess
 This sets the global variable *python* to the process handle,
@@ -24,7 +29,8 @@ By default this is is set to *PYTHON-COMMAND*
                       ;; Path *base-pathname* is defined in py4cl.asd
                       ;; Calculate full path to python script
                       (namestring (merge-pathnames #p"py4cl.py" py4cl/config:*base-directory*)))
-         :input :stream :output :stream)))
+         :input :stream :output :stream))
+  (incf *current-python-process-id*))
 
 (defun python-alive-p (&optional (process *python*))
   "Returns non-NIL if the python process is alive
@@ -41,6 +47,9 @@ If still not alive, raises a condition."
   (unless (python-alive-p)
     (error "Could not start python process")))
 
+;; Function defined in writer.lisp, which clears an object store
+(declaim (ftype (function () t) clear-lisp-objects))
+
 (defun python-stop (&optional (process *python*))
   ;; If python is not running then return
   (unless (python-alive-p process)
@@ -55,7 +64,10 @@ If still not alive, raises a condition."
   ;; Terminate
   (uiop:terminate-process process)
   ;; Mark as not alive
-  (setf *python* nil))
+  (setf *python* nil)
+
+  ;; Clear lisp objects
+  (clear-lisp-objects))
 
 (defun python-version-info ()
   "Return a list, using the result of python's sys.version_info."

src/reader.lisp

@@ -19,14 +19,17 @@ Uses trivial-garbage (public domain)
     (tg:finalize
      (make-python-object :type type
                          :handle handle)
-     (lambda () ; This function is called when the python-object is garbage collected
-       (ignore-errors
-         (if (python-alive-p) ; If not alive, python-exec will start python
-             (python-exec "
+     (let ((python-id *current-python-process-id*))
+       (lambda () ; This function is called when the python-object is garbage collected
+         (ignore-errors
+           (if (and
+                (python-alive-p) ; If not alive, python-exec will start python
+                (= *current-python-process-id* python-id)) ; Python might have restarted
+               (python-exec "
 try:
   del _py4cl_objects[" handle "]
 except:
-  pass"))))))
+  pass")))))))
 
 (defun stream-read-string (stream)
   "Reads a string from a stream

src/writer.lisp

@@ -10,6 +10,11 @@
 
 (defvar *lisp-objects* (make-hash-table :test #'eql))
 
+(defun clear-lisp-objects ()
+  "Clear the *lisp-objects* object store, allowing them to be GC'd"
+  (setf *lisp-objects* (make-hash-table :test #'eql)
+        *handle-counter* 0))
+
 (defun free-handle (handle)
   "Remove an object with HANDLE from the hash table"
   (remhash handle *lisp-objects*))
@@ -119,13 +124,13 @@ Produces a string {key1:value1, key2:value2,}"
                "}"))
 
 (defmethod pythonize ((obj function))
-  "Handle a function by converting to a callback object"
-
-  (let ((id (register-callback obj)))
-    (concatenate 'string
-                 "lambda *args, **kwargs: _py4cl_callback("
-                 (write-to-string id)
-                 ", *args, **kwargs)")))
+  "Handle a function by converting to a callback object
+The lisp function is stored in the same object store as other objects."
+  (concatenate 'string
+                 "_py4cl_LispCallbackObject("
+                 (write-to-string
+                  (object-handle obj))
+                 ")"))
 
 (defmethod pythonize ((obj python-object))
   "A handle for a python object, stored in a dict in Python"