Skip to content

Commit 2d350fd

Browse files
committed
Issue #18619: Fix atexit leaking callbacks registered from sub-interpreters, and make it GC-aware.
1 parent 7a2572c commit 2d350fd

File tree

3 files changed

+122
-44
lines changed

3 files changed

+122
-44
lines changed

Lib/test/test_atexit.py

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import unittest
33
import io
44
import atexit
5+
import _testcapi
56
from test import support
67

78
### helpers
@@ -23,7 +24,9 @@ def raise1():
2324
def raise2():
2425
raise SystemError
2526

26-
class TestCase(unittest.TestCase):
27+
28+
class GeneralTest(unittest.TestCase):
29+
2730
def setUp(self):
2831
self.save_stdout = sys.stdout
2932
self.save_stderr = sys.stderr
@@ -122,8 +125,43 @@ def test_bound_methods(self):
122125
self.assertEqual(l, [5])
123126

124127

128+
class SubinterpreterTest(unittest.TestCase):
129+
130+
def test_callbacks_leak(self):
131+
# This test shows a leak in refleak mode if atexit doesn't
132+
# take care to free callbacks in its per-subinterpreter module
133+
# state.
134+
n = atexit._ncallbacks()
135+
code = r"""if 1:
136+
import atexit
137+
def f():
138+
pass
139+
atexit.register(f)
140+
del atexit
141+
"""
142+
ret = _testcapi.run_in_subinterp(code)
143+
self.assertEqual(ret, 0)
144+
self.assertEqual(atexit._ncallbacks(), n)
145+
146+
def test_callbacks_leak_refcycle(self):
147+
# Similar to the above, but with a refcycle through the atexit
148+
# module.
149+
n = atexit._ncallbacks()
150+
code = r"""if 1:
151+
import atexit
152+
def f():
153+
pass
154+
atexit.register(f)
155+
atexit.__atexit = atexit
156+
"""
157+
ret = _testcapi.run_in_subinterp(code)
158+
self.assertEqual(ret, 0)
159+
self.assertEqual(atexit._ncallbacks(), n)
160+
161+
125162
def test_main():
126-
support.run_unittest(TestCase)
163+
support.run_unittest(__name__)
164+
127165

128166
if __name__ == "__main__":
129167
test_main()

Misc/NEWS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,9 @@ Core and Builtins
179179
Library
180180
-------
181181

182+
- Issue #18619: Fix atexit leaking callbacks registered from sub-interpreters,
183+
and make it GC-aware.
184+
182185
- Issue #15699: The readline module now uses PEP 3121-style module
183186
initialization, so as to reclaim allocated resources (Python callbacks)
184187
at shutdown. Original patch by Robin Schreiber.

Modules/atexitmodule.c

Lines changed: 79 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@
1010

1111
/* Forward declaration (for atexit_cleanup) */
1212
static PyObject *atexit_clear(PyObject*, PyObject*);
13-
/* Forward declaration (for atexit_callfuncs) */
14-
static void atexit_cleanup(PyObject*);
1513
/* Forward declaration of module object */
1614
static struct PyModuleDef atexitmodule;
1715

@@ -33,6 +31,35 @@ typedef struct {
3331
#define GET_ATEXIT_STATE(mod) ((atexitmodule_state*)PyModule_GetState(mod))
3432

3533

34+
static void
35+
atexit_delete_cb(atexitmodule_state *modstate, int i)
36+
{
37+
atexit_callback *cb;
38+
39+
cb = modstate->atexit_callbacks[i];
40+
modstate->atexit_callbacks[i] = NULL;
41+
Py_DECREF(cb->func);
42+
Py_DECREF(cb->args);
43+
Py_XDECREF(cb->kwargs);
44+
PyMem_Free(cb);
45+
}
46+
47+
/* Clear all callbacks without calling them */
48+
static void
49+
atexit_cleanup(atexitmodule_state *modstate)
50+
{
51+
atexit_callback *cb;
52+
int i;
53+
for (i = 0; i < modstate->ncallbacks; i++) {
54+
cb = modstate->atexit_callbacks[i];
55+
if (cb == NULL)
56+
continue;
57+
58+
atexit_delete_cb(modstate, i);
59+
}
60+
modstate->ncallbacks = 0;
61+
}
62+
3663
/* Installed into pythonrun.c's atexit mechanism */
3764

3865
static void
@@ -78,34 +105,12 @@ atexit_callfuncs(void)
78105
}
79106
}
80107

81-
atexit_cleanup(module);
108+
atexit_cleanup(modstate);
82109

83110
if (exc_type)
84111
PyErr_Restore(exc_type, exc_value, exc_tb);
85112
}
86113

87-
static void
88-
atexit_delete_cb(PyObject *self, int i)
89-
{
90-
atexitmodule_state *modstate;
91-
atexit_callback *cb;
92-
93-
modstate = GET_ATEXIT_STATE(self);
94-
cb = modstate->atexit_callbacks[i];
95-
modstate->atexit_callbacks[i] = NULL;
96-
Py_DECREF(cb->func);
97-
Py_DECREF(cb->args);
98-
Py_XDECREF(cb->kwargs);
99-
PyMem_Free(cb);
100-
}
101-
102-
static void
103-
atexit_cleanup(PyObject *self)
104-
{
105-
PyObject *r = atexit_clear(self, NULL);
106-
Py_DECREF(r);
107-
}
108-
109114
/* ===================================================================== */
110115
/* Module methods. */
111116

@@ -193,29 +198,59 @@ Clear the list of previously registered exit functions.");
193198

194199
static PyObject *
195200
atexit_clear(PyObject *self, PyObject *unused)
201+
{
202+
atexit_cleanup(GET_ATEXIT_STATE(self));
203+
Py_RETURN_NONE;
204+
}
205+
206+
PyDoc_STRVAR(atexit_ncallbacks__doc__,
207+
"_ncallbacks() -> int\n\
208+
\n\
209+
Return the number of registered exit functions.");
210+
211+
static PyObject *
212+
atexit_ncallbacks(PyObject *self, PyObject *unused)
196213
{
197214
atexitmodule_state *modstate;
198-
atexit_callback *cb;
199-
int i;
200215

201216
modstate = GET_ATEXIT_STATE(self);
202217

218+
return PyLong_FromSsize_t(modstate->ncallbacks);
219+
}
220+
221+
static int
222+
atexit_m_traverse(PyObject *self, visitproc visit, void *arg)
223+
{
224+
int i;
225+
atexitmodule_state *modstate;
226+
227+
modstate = GET_ATEXIT_STATE(self);
203228
for (i = 0; i < modstate->ncallbacks; i++) {
204-
cb = modstate->atexit_callbacks[i];
229+
atexit_callback *cb = modstate->atexit_callbacks[i];
205230
if (cb == NULL)
206231
continue;
207-
208-
atexit_delete_cb(self, i);
232+
Py_VISIT(cb->func);
233+
Py_VISIT(cb->args);
234+
Py_VISIT(cb->kwargs);
209235
}
210-
modstate->ncallbacks = 0;
211-
Py_RETURN_NONE;
236+
return 0;
237+
}
238+
239+
static int
240+
atexit_m_clear(PyObject *self)
241+
{
242+
atexitmodule_state *modstate;
243+
modstate = GET_ATEXIT_STATE(self);
244+
atexit_cleanup(modstate);
245+
return 0;
212246
}
213247

214248
static void
215249
atexit_free(PyObject *m)
216250
{
217251
atexitmodule_state *modstate;
218252
modstate = GET_ATEXIT_STATE(m);
253+
atexit_cleanup(modstate);
219254
PyMem_Free(modstate->atexit_callbacks);
220255
}
221256

@@ -246,7 +281,7 @@ atexit_unregister(PyObject *self, PyObject *func)
246281
if (eq < 0)
247282
return NULL;
248283
if (eq)
249-
atexit_delete_cb(self, i);
284+
atexit_delete_cb(modstate, i);
250285
}
251286
Py_RETURN_NONE;
252287
}
@@ -260,6 +295,8 @@ static PyMethodDef atexit_methods[] = {
260295
atexit_unregister__doc__},
261296
{"_run_exitfuncs", (PyCFunction) atexit_run_exitfuncs, METH_NOARGS,
262297
atexit_run_exitfuncs__doc__},
298+
{"_ncallbacks", (PyCFunction) atexit_ncallbacks, METH_NOARGS,
299+
atexit_ncallbacks__doc__},
263300
{NULL, NULL} /* sentinel */
264301
};
265302

@@ -275,15 +312,15 @@ Two public functions, register and unregister, are defined.\n\
275312

276313

277314
static struct PyModuleDef atexitmodule = {
278-
PyModuleDef_HEAD_INIT,
279-
"atexit",
280-
atexit__doc__,
281-
sizeof(atexitmodule_state),
282-
atexit_methods,
283-
NULL,
284-
NULL,
285-
NULL,
286-
(freefunc)atexit_free
315+
PyModuleDef_HEAD_INIT,
316+
"atexit",
317+
atexit__doc__,
318+
sizeof(atexitmodule_state),
319+
atexit_methods,
320+
NULL,
321+
atexit_m_traverse,
322+
atexit_m_clear,
323+
(freefunc)atexit_free
287324
};
288325

289326
PyMODINIT_FUNC

0 commit comments

Comments
 (0)