Annotation of /trunk/pygobject/patches/pygobject-2.28.6-deallocation-gc-crash.patch
Parent Directory | Revision Log
Revision 1697 -
(hide annotations)
(download)
Tue Mar 13 08:51:22 2012 UTC (12 years, 6 months ago) by niro
File size: 3584 byte(s)
Tue Mar 13 08:51:22 2012 UTC (12 years, 6 months ago) by niro
File size: 3584 byte(s)
-added patch to fix a crash
1 | niro | 1697 | From 92aca4416a7930e5870b8d1a4016bae8140462ee Mon Sep 17 00:00:00 2001 |
2 | From: Daniel Drake <dsd@laptop.org> | ||
3 | Date: Fri, 3 Jun 2011 16:59:15 +0100 | ||
4 | Subject: [PATCH] Fix GC-related crash during PyGObject deallocation | ||
5 | |||
6 | Python-2.7.1's GC source has the following comment: | ||
7 | |||
8 | /* Python's cyclic gc should never see an incoming refcount | ||
9 | * of 0: if something decref'ed to 0, it should have been | ||
10 | * deallocated immediately at that time. | ||
11 | * Possible cause (if the assert triggers): a tp_dealloc | ||
12 | * routine left a gc-aware object tracked during its teardown | ||
13 | * phase, and did something-- or allowed something to happen -- | ||
14 | * that called back into Python. gc can trigger then, and may | ||
15 | * see the still-tracked dying object. Before this assert | ||
16 | * was added, such mistakes went on to allow gc to try to | ||
17 | * delete the object again. In a debug build, that caused | ||
18 | * a mysterious segfault, when _Py_ForgetReference tried | ||
19 | * to remove the object from the doubly-linked list of all | ||
20 | * objects a second time. In a release build, an actual | ||
21 | * double deallocation occurred, which leads to corruption | ||
22 | * of the allocator's internal bookkeeping pointers. That's | ||
23 | * so serious that maybe this should be a release-build | ||
24 | * check instead of an assert? | ||
25 | */ | ||
26 | |||
27 | As shown in a backtrace at | ||
28 | https://bugzilla.redhat.com/show_bug.cgi?id=640972 , pygobject is making | ||
29 | this exact mistake. Before untracking its object, pygobject_dealloc | ||
30 | calls PyObject_ClearWeakRefs() which can call back into python, create | ||
31 | new allocations, and trigger the GC. | ||
32 | |||
33 | This is causing Sugar (based on pygobject2 + pygtk2 static bindings) to | ||
34 | crash on a regular basis while interacting with widgets or launching | ||
35 | applications. | ||
36 | |||
37 | Fix this by untracking the object early. Also fix the same issue spotted | ||
38 | in the GSource wrapper. | ||
39 | |||
40 | Thanks to Bernie Innocenti for initial diagnosis. | ||
41 | --- | ||
42 | glib/pygsource.c | 6 ++++-- | ||
43 | gobject/pygobject.c | 8 +++++++- | ||
44 | 2 files changed, 11 insertions(+), 3 deletions(-) | ||
45 | |||
46 | diff --git a/glib/pygsource.c b/glib/pygsource.c | ||
47 | index 684711e..d0176ab 100644 | ||
48 | --- a/glib/pygsource.c | ||
49 | +++ b/glib/pygsource.c | ||
50 | @@ -387,10 +387,12 @@ pyg_source_clear(PyGSource *self) | ||
51 | static void | ||
52 | pyg_source_dealloc(PyGSource *self) | ||
53 | { | ||
54 | - PyObject_ClearWeakRefs((PyObject *)self); | ||
55 | - | ||
56 | + /* Must be done first, so that there is no chance of Python's GC being | ||
57 | + * called while tracking this half-deallocated object */ | ||
58 | PyObject_GC_UnTrack((PyObject *)self); | ||
59 | |||
60 | + PyObject_ClearWeakRefs((PyObject *)self); | ||
61 | + | ||
62 | pyg_source_clear(self); | ||
63 | |||
64 | PyObject_GC_Del(self); | ||
65 | diff --git a/gobject/pygobject.c b/gobject/pygobject.c | ||
66 | index 0faf221..3193890 100644 | ||
67 | --- a/gobject/pygobject.c | ||
68 | +++ b/gobject/pygobject.c | ||
69 | @@ -1028,8 +1028,14 @@ PYGLIB_DEFINE_TYPE("gobject.GObject", PyGObject_Type, PyGObject); | ||
70 | static void | ||
71 | pygobject_dealloc(PyGObject *self) | ||
72 | { | ||
73 | - PyObject_ClearWeakRefs((PyObject *)self); | ||
74 | + /* Untrack must be done first. This is because followup calls such as | ||
75 | + * ClearWeakRefs could call into Python and cause new allocations to | ||
76 | + * happen, which could in turn could trigger the garbage collector, | ||
77 | + * which would then get confused as it is tracking this half-deallocated | ||
78 | + * object. */ | ||
79 | PyObject_GC_UnTrack((PyObject *)self); | ||
80 | + | ||
81 | + PyObject_ClearWeakRefs((PyObject *)self); | ||
82 | /* this forces inst_data->type to be updated, which could prove | ||
83 | * important if a new wrapper has to be created and it is of a | ||
84 | * unregistered type */ | ||
85 | -- | ||
86 | 1.7.5.2 | ||
87 |