python: Fix object deallocation.
authorJustus Winter <justus@gnupg.org>
Fri, 27 May 2016 10:25:59 +0000 (12:25 +0200)
committerJustus Winter <justus@gnupg.org>
Fri, 27 May 2016 10:25:59 +0000 (12:25 +0200)
Handing a reference to the wrapper object created a non-trivial
circular reference that Pythons garbage collector is unable to break.
Explicitly break it by using a weak reference.

* lang/python/helpers.c (pygpgme_stash_callback_exception): Retrieve
object from weak reference.
* lang/python/pyme/core.py (Context.__del__): Free status callback.
(Context.set_passphrase_cb): Use a weak reference.
(Context.set_progress_cb): Likewise.
(Context.set_status_cb): Likewise.
(Context.op_edit): Likewise.

Signed-off-by: Justus Winter <justus@gnupg.org>
lang/python/helpers.c
lang/python/pyme/core.py

index ec7264a..9fe81c9 100644 (file)
@@ -64,9 +64,9 @@ void pygpgme_clear_generic_cb(PyObject **cb) {
 /* Exception support for callbacks.  */
 #define EXCINFO        "_callback_excinfo"
 
-static void pygpgme_stash_callback_exception(PyObject *self)
+static void pygpgme_stash_callback_exception(PyObject *weak_self)
 {
-  PyObject *ptype, *pvalue, *ptraceback, *excinfo;
+  PyObject *self, *ptype, *pvalue, *ptraceback, *excinfo;
 
   PyErr_Fetch(&ptype, &pvalue, &ptraceback);
   excinfo = PyTuple_New(3);
@@ -86,7 +86,23 @@ static void pygpgme_stash_callback_exception(PyObject *self)
     PyTuple_SetItem(excinfo, 2, Py_None);
   }
 
-  PyObject_SetAttrString(self, EXCINFO, excinfo);
+  self = PyWeakref_GetObject(weak_self);
+  /* self only has a borrowed reference.  */
+  if (self == Py_None) {
+    /* This should not happen, as even if we're called from the data
+       release callback triggered from the wrappers destructor, the
+       object is still alive and hence the weak reference still refers
+       to the object.  However, in case this ever changes, not seeing
+       any exceptions is worse than having a little extra code, so
+       here we go.  */
+      fprintf(stderr,
+              "Error occurred in callback, but the wrapper object "
+              "has been deallocated.\n");
+      PyErr_Restore(ptype, pvalue, ptraceback);
+      PyErr_Print();
+    }
+  else
+    PyObject_SetAttrString(self, EXCINFO, excinfo);
 }
 
 PyObject *pygpgme_raise_callback_exception(PyObject *self)
index 0c2dd60..6ef2dab 100644 (file)
@@ -16,9 +16,7 @@
 #    License along with this library; if not, write to the Free Software
 #    Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
 
-# import generators for portability with python2.2
-
-
+import weakref
 from . import pygpgme
 from .errors import errorcheck, GPGMEError
 from . import errors
@@ -149,6 +147,7 @@ class Context(GpgmeWrapper):
 
         self._free_passcb()
         self._free_progresscb()
+        self._free_statuscb()
         if self.own and pygpgme.gpgme_release:
             pygpgme.gpgme_release(self.wrapped)
 
@@ -252,9 +251,9 @@ class Context(GpgmeWrapper):
         else:
             self.last_passcb = pygpgme.new_PyObject_p_p()
             if hook == None:
-                hookdata = (self, func)
+                hookdata = (weakref.ref(self), func)
             else:
-                hookdata = (self, func, hook)
+                hookdata = (weakref.ref(self), func, hook)
         pygpgme.pygpgme_set_passphrase_cb(self.wrapped, hookdata, self.last_passcb)
 
     def set_progress_cb(self, func, hook=None):
@@ -275,9 +274,9 @@ class Context(GpgmeWrapper):
         else:
             self.last_progresscb = pygpgme.new_PyObject_p_p()
             if hook == None:
-                hookdata = (self, func)
+                hookdata = (weakref.ref(self), func)
             else:
-                hookdata = (self, func, hook)
+                hookdata = (weakref.ref(self), func, hook)
         pygpgme.pygpgme_set_progress_cb(self.wrapped, hookdata, self.last_progresscb)
 
     def set_status_cb(self, func, hook=None):
@@ -297,9 +296,9 @@ class Context(GpgmeWrapper):
         else:
             self.last_statuscb = pygpgme.new_PyObject_p_p()
             if hook == None:
-                hookdata = (self, func)
+                hookdata = (weakref.ref(self), func)
             else:
-                hookdata = (self, func, hook)
+                hookdata = (weakref.ref(self), func, hook)
         pygpgme.pygpgme_set_status_cb(self.wrapped, hookdata,
                                       self.last_statuscb)
 
@@ -333,9 +332,9 @@ class Context(GpgmeWrapper):
         if key == None:
             raise ValueError("op_edit: First argument cannot be None")
         if fnc_value:
-            opaquedata = (self, func, fnc_value)
+            opaquedata = (weakref.ref(self), func, fnc_value)
         else:
-            opaquedata = (self, func)
+            opaquedata = (weakref.ref(self), func)
 
         result = pygpgme.gpgme_op_edit(self.wrapped, key, opaquedata, out)
         if self._callback_excinfo: