[Pkg-gnupg-commit] [gpgme] 140/412: python: Fix object deallocation.

[Daniel Kahn Gillmor]

This is an automated email from the git hooks/post-receive script.

dkg pushed a commit to branch master
in repository gpgme.

commit ebfe2300c33a3bad311e9ac1530e6c92636a08a4
Author: Justus Winter <justus at gnupg.org>
Date:   Fri May 27 12:25:59 2016 +0200

    python: Fix object deallocation.
    
    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 at gnupg.org>
---
 lang/python/helpers.c    | 22 +++++++++++++++++++---
 lang/python/pyme/core.py | 21 ++++++++++-----------
 2 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/lang/python/helpers.c b/lang/python/helpers.c
index ec7264a..9fe81c9 100644
--- a/lang/python/helpers.c
+++ b/lang/python/helpers.c
@@ -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)
diff --git a/lang/python/pyme/core.py b/lang/python/pyme/core.py
index 0c2dd60..6ef2dab 100644
--- a/lang/python/pyme/core.py
+++ b/lang/python/pyme/core.py
@@ -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:

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-gnupg/gpgme.git