commit 07c0b2e8686cdd69635c6451a56da4fd9924af54
Author: Mikhail Fludkov <fludkov me gmail com>
Date: Mon Jan 29 14:23:37 2018 +0100
pygobject-object: fix memory corruption around list of closures
https://gitlab.gnome.org/GNOME/pygobject/issues/158
The memory corruption occurs because of the race while accessing
PyGObjectData->closures list.
Protect PyGObjectData->closures by GIL in pygobject_unwatch_closure.
Despite the fact that we don't call any Python API in the function. We use
GIL to be sure that PyGObjectData->closures list stays intact while
GC iterating the list inside pygobject_traverse. Otherwise we can
segfault while trying to call 'visit' function on an object that
was just freed in pygobject_unwatch_closure.
gi/pygobject-object.c | 5 +++++
tests/test_signal.py | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 55 insertions(+)
---
diff --git a/gi/pygobject-object.c b/gi/pygobject-object.c
index 729bb4dc..304b4277 100644
--- a/gi/pygobject-object.c
+++ b/gi/pygobject-object.c
@@ -1037,7 +1037,12 @@ pygobject_unwatch_closure(gpointer data, GClosure *closure)
{
PyGObjectData *inst_data = data;
+ /* Despite no Python API is called the list inst_data->closures
+ * must be protected by GIL as it is used by GC in
+ * pygobject_traverse */
+ PyGILState_STATE state = PyGILState_Ensure();
inst_data->closures = g_slist_remove (inst_data->closures, closure);
+ PyGILState_Release(state);
}
/**
diff --git a/tests/test_signal.py b/tests/test_signal.py
index b2f121a6..642708bf 100644
--- a/tests/test_signal.py
+++ b/tests/test_signal.py
@@ -4,12 +4,15 @@ import gc
import unittest
import sys
import weakref
+import threading
+import time
from gi.repository import GObject, GLib, Regress, Gio
from gi import _signalhelper as signalhelper
import testhelper
from compathelper import _long
from helper import capture_glib_warnings, capture_gi_deprecation_warnings
+from gi.module import repository as repo
class C(GObject.GObject):
@@ -1219,6 +1222,53 @@ class TestIntrospectedSignals(unittest.TestCase):
self.assertNotEqual(struct, held_struct)
+class TestIntrospectedSignalsIssue158(unittest.TestCase):
+ """
+ The test for https://gitlab.gnome.org/GNOME/pygobject/issues/158
+ """
+ _obj_sig_names = [sig.get_name() for sig in repo.find_by_name('Regress', 'TestObj').get_signals()]
+
+ def __init__(self, *args):
+ unittest.TestCase.__init__(self, *args)
+ self._gc_thread_stop = False
+
+ def _gc_thread(self):
+ while not self._gc_thread_stop:
+ gc.collect()
+ time.sleep(0.010)
+
+ def _callback(self, *args):
+ pass
+
+ def test_run(self):
+ """
+ Manually trigger GC from a different thread periodicaly
+ while the main thread keeps connecting/disconnecting to/from signals.
+
+ It takes a lot of time to reproduce the issue. It is possible to make it
+ fail reliably by changing the code of pygobject_unwatch_closure slightly from:
+ PyGObjectData *inst_data = data;
+ inst_data->closures = g_slist_remove (inst_data->closures, closure);
+ to
+ PyGObjectData *inst_data = data;
+ GSList *tmp = g_slist_remove (inst_data->closures, closure);
+ g_usleep(G_USEC_PER_SEC/10);
+ inst_data->closures = tmp;
+ """
+ obj = Regress.TestObj()
+ gc_thread = threading.Thread(target=self._gc_thread)
+ gc_thread.start()
+
+ for _ in range(8):
+ handlers = [obj.connect(sig, self._callback) for sig in self._obj_sig_names]
+ time.sleep(0.010)
+ while len(handlers) > 0:
+ obj.disconnect(handlers.pop())
+
+ self._gc_thread_stop = True
+ gc_thread.join()
+
+
class _ConnectObjectTestBase(object):
# Notes:
# - self.Object is overridden in sub-classes.