From 27f2b839fdf951c58ab03258cac330942b81a057 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Thu, 24 May 2018 15:20:03 +0300 Subject: [PATCH 1/3] bpo-33622: Add checks for exceptions leaks in the garbage collector. Failure in adding to gc.garbage is no longer fatal. --- Modules/gcmodule.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c index 4d701cb72e8c5d..4a0f8a08d59e75 100644 --- a/Modules/gcmodule.c +++ b/Modules/gcmodule.c @@ -663,8 +663,10 @@ handle_legacy_finalizers(PyGC_Head *finalizers, PyGC_Head *old) PyObject *op = FROM_GC(gc); if ((_PyRuntime.gc.debug & DEBUG_SAVEALL) || has_legacy_finalizer(op)) { - if (PyList_Append(_PyRuntime.gc.garbage, op) < 0) + if (PyList_Append(_PyRuntime.gc.garbage, op) < 0) { + PyErr_Clear(); break; + } } } @@ -701,6 +703,7 @@ finalize_garbage(PyGC_Head *collectable) _PyGCHead_SET_FINALIZED(gc, 1); Py_INCREF(op); finalize(op); + assert(!PyErr_Occurred()); Py_DECREF(op); } } @@ -753,12 +756,16 @@ delete_garbage(PyGC_Head *collectable, PyGC_Head *old) PyObject *op = FROM_GC(gc); if (_PyRuntime.gc.debug & DEBUG_SAVEALL) { - PyList_Append(_PyRuntime.gc.garbage, op); + assert(_PyRuntime.gc.garbage != NULL); + if (PyList_Append(_PyRuntime.gc.garbage, op) < 0) { + PyErr_Clear(); + } } else { if ((clear = Py_TYPE(op)->tp_clear) != NULL) { Py_INCREF(op); - clear(op); + (void) clear(op); + assert(!PyErr_Occurred()); Py_DECREF(op); } } @@ -974,6 +981,7 @@ collect(int generation, Py_ssize_t *n_collected, Py_ssize_t *n_uncollectable, if (PyDTrace_GC_DONE_ENABLED()) PyDTrace_GC_DONE(n+m); + assert(!PyErr_Occurred()); return n+m; } @@ -987,11 +995,12 @@ invoke_gc_callback(const char *phase, int generation, Py_ssize_t i; PyObject *info = NULL; + assert(!PyErr_Occurred()); /* we may get called very early */ if (_PyRuntime.gc.callbacks == NULL) return; /* The local variable cannot be rebound, check it for sanity */ - assert(_PyRuntime.gc.callbacks != NULL && PyList_CheckExact(_PyRuntime.gc.callbacks)); + assert(PyList_CheckExact(_PyRuntime.gc.callbacks)); if (PyList_GET_SIZE(_PyRuntime.gc.callbacks) != 0) { info = Py_BuildValue("{sisnsn}", "generation", generation, @@ -1015,6 +1024,7 @@ invoke_gc_callback(const char *phase, int generation, Py_DECREF(cb); } Py_XDECREF(info); + assert(!PyErr_Occurred()); } /* Perform garbage collection of a generation and invoke @@ -1127,6 +1137,7 @@ gc_collect_impl(PyObject *module, int generation) n = 0; /* already collecting, don't do anything */ else { _PyRuntime.gc.collecting = 1; + assert(!PyErr_Occurred()); n = collect_with_callback(generation); _PyRuntime.gc.collecting = 0; } @@ -1592,6 +1603,7 @@ _PyGC_CollectNoFail(void) { Py_ssize_t n; + assert(!PyErr_Occurred()); /* Ideally, this function is only called on interpreter shutdown, and therefore not recursively. Unfortunately, when there are daemon threads, a daemon thread can start a cyclic garbage collection From 032a0ba7bbfb4bf4849daa701e1dfbf027385f9e Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 28 May 2018 12:51:09 +0300 Subject: [PATCH 2/3] Address review comments. --- Modules/gcmodule.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c index 4a0f8a08d59e75..d9b51696424034 100644 --- a/Modules/gcmodule.c +++ b/Modules/gcmodule.c @@ -654,6 +654,7 @@ handle_legacy_finalizers(PyGC_Head *finalizers, PyGC_Head *old) { PyGC_Head *gc = finalizers->gc.gc_next; + assert(!PyErr_Occurred()); if (_PyRuntime.gc.garbage == NULL) { _PyRuntime.gc.garbage = PyList_New(0); if (_PyRuntime.gc.garbage == NULL) @@ -751,6 +752,7 @@ delete_garbage(PyGC_Head *collectable, PyGC_Head *old) { inquiry clear; + assert(!PyErr_Occurred()); while (!gc_list_is_empty(collectable)) { PyGC_Head *gc = collectable->gc.gc_next; PyObject *op = FROM_GC(gc); @@ -1034,9 +1036,11 @@ static Py_ssize_t collect_with_callback(int generation) { Py_ssize_t result, collected, uncollectable; + assert(!PyErr_Occurred()); invoke_gc_callback("start", generation, 0, 0); result = collect(generation, &collected, &uncollectable, 0); invoke_gc_callback("stop", generation, collected, uncollectable); + assert(!PyErr_Occurred()); return result; } @@ -1137,7 +1141,6 @@ gc_collect_impl(PyObject *module, int generation) n = 0; /* already collecting, don't do anything */ else { _PyRuntime.gc.collecting = 1; - assert(!PyErr_Occurred()); n = collect_with_callback(generation); _PyRuntime.gc.collecting = 0; } From 8de8f206b38eade288eb0cb5ad4ad4a83b9d4fec Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 29 May 2018 10:21:42 +0300 Subject: [PATCH 3/3] Check and silence errors in tp_clear. --- Modules/gcmodule.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c index d9b51696424034..7d23fc22c81dd0 100644 --- a/Modules/gcmodule.c +++ b/Modules/gcmodule.c @@ -767,7 +767,11 @@ delete_garbage(PyGC_Head *collectable, PyGC_Head *old) if ((clear = Py_TYPE(op)->tp_clear) != NULL) { Py_INCREF(op); (void) clear(op); - assert(!PyErr_Occurred()); + if (PyErr_Occurred()) { + PySys_WriteStderr("Exception ignored in tp_clear of " + "%.50s\n", Py_TYPE(op)->tp_name); + PyErr_WriteUnraisable(NULL); + } Py_DECREF(op); } }