From 87a5c515d03f3e2215f3da7b90a4b848ac510879 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 10 Feb 2014 18:21:34 +0200 Subject: [PATCH] Issue #19255: The builtins module is restored to initial value before cleaning other modules. The sys and builtins modules are cleaned last. --- Include/moduleobject.h | 1 + Include/pystate.h | 2 +- Lib/test/test_builtin.py | 29 ++++++++++++++ Misc/NEWS | 3 ++ Objects/moduleobject.c | 13 +++--- Python/import.c | 85 ++++++++++++++++++++++++---------------- Python/pystate.c | 2 + 7 files changed, 96 insertions(+), 39 deletions(-) diff --git a/Include/moduleobject.h b/Include/moduleobject.h index 8013dd9b484..f119364b889 100644 --- a/Include/moduleobject.h +++ b/Include/moduleobject.h @@ -25,6 +25,7 @@ PyAPI_FUNC(const char *) PyModule_GetFilename(PyObject *); PyAPI_FUNC(PyObject *) PyModule_GetFilenameObject(PyObject *); #ifndef Py_LIMITED_API PyAPI_FUNC(void) _PyModule_Clear(PyObject *); +PyAPI_FUNC(void) _PyModule_ClearDict(PyObject *); #endif PyAPI_FUNC(struct PyModuleDef*) PyModule_GetDef(PyObject*); PyAPI_FUNC(void*) PyModule_GetState(PyObject*); diff --git a/Include/pystate.h b/Include/pystate.h index 1f3465fdbd5..4992c22684f 100644 --- a/Include/pystate.h +++ b/Include/pystate.h @@ -33,7 +33,6 @@ typedef struct _is { int codecs_initialized; int fscodec_initialized; - #ifdef HAVE_DLOPEN int dlopenflags; #endif @@ -41,6 +40,7 @@ typedef struct _is { int tscdump; #endif + PyObject *builtins_copy; } PyInterpreterState; #endif diff --git a/Lib/test/test_builtin.py b/Lib/test/test_builtin.py index 68430660f01..c078b443c5b 100644 --- a/Lib/test/test_builtin.py +++ b/Lib/test/test_builtin.py @@ -16,6 +16,7 @@ import unittest import warnings from operator import neg from test.support import TESTFN, unlink, run_unittest, check_warnings +from test.script_helper import assert_python_ok try: import pty, signal except ImportError: @@ -1592,6 +1593,34 @@ class TestSorted(unittest.TestCase): data = 'The quick Brown fox Jumped over The lazy Dog'.split() self.assertRaises(TypeError, sorted, data, None, lambda x,y: 0) + +class ShutdownTest(unittest.TestCase): + + def test_cleanup(self): + # Issue #19255: builtins are still available at shutdown + code = """if 1: + import builtins + import sys + + class C: + def __del__(self): + print("before") + # Check that builtins still exist + len(()) + print("after") + + c = C() + # Make this module survive until builtins and sys are cleaned + builtins.here = sys.modules[__name__] + sys.here = sys.modules[__name__] + # Create a reference loop so that this module needs to go + # through a GC phase. + here = sys.modules[__name__] + """ + rc, out, err = assert_python_ok("-c", code) + self.assertEqual(["before", "after"], out.decode().splitlines()) + + def load_tests(loader, tests, pattern): from doctest import DocTestSuite tests.addTest(DocTestSuite(builtins)) diff --git a/Misc/NEWS b/Misc/NEWS index 30abc7f5be2..4b326ff98e0 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -10,6 +10,9 @@ Release date: 2014-02-09 Core and Builtins ----------------- +- Issue #19255: The builtins module is restored to initial value before + cleaning other modules. The sys and builtins modules are cleaned last. + - Issue #20437: Fixed 22 potential bugs when deleting objects references. - Issue #20500: Displaying an exception at interpreter shutdown no longer diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index d59475e2f3c..682171070ec 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -298,6 +298,14 @@ PyModule_GetState(PyObject* m) void _PyModule_Clear(PyObject *m) +{ + PyObject *d = ((PyModuleObject *)m)->md_dict; + if (d != NULL) + _PyModule_ClearDict(d); +} + +void +_PyModule_ClearDict(PyObject *d) { /* To make the execution order of destructors for global objects a bit more predictable, we first zap all objects @@ -308,11 +316,6 @@ _PyModule_Clear(PyObject *m) Py_ssize_t pos; PyObject *key, *value; - PyObject *d; - - d = ((PyModuleObject *)m)->md_dict; - if (d == NULL) - return; /* First, clear only names starting with a single underscore */ pos = 0; diff --git a/Python/import.c b/Python/import.c index d0115e46bf1..207dbced414 100644 --- a/Python/import.c +++ b/Python/import.c @@ -49,9 +49,13 @@ class fs_unicode_converter(CConverter): void _PyImport_Init(void) { + PyInterpreterState *interp = PyThreadState_Get()->interp; initstr = PyUnicode_InternFromString("__init__"); if (initstr == NULL) Py_FatalError("Can't initialize import variables"); + interp->builtins_copy = PyDict_Copy(interp->builtins); + if (interp->builtins_copy == NULL) + Py_FatalError("Can't backup builtins dict"); } void @@ -397,8 +401,10 @@ PyImport_Cleanup(void) PyObject *key, *value, *dict; PyInterpreterState *interp = PyThreadState_GET()->interp; PyObject *modules = interp->modules; - PyObject *builtins = interp->builtins; + PyObject *builtins_mod = NULL; + PyObject *sys_mod = NULL; PyObject *weaklist = NULL; + char **p; if (modules == NULL) return; /* Already done */ @@ -411,31 +417,22 @@ PyImport_Cleanup(void) /* XXX Perhaps these precautions are obsolete. Who knows? */ - value = PyDict_GetItemString(modules, "builtins"); - if (value != NULL && PyModule_Check(value)) { - dict = PyModule_GetDict(value); + if (Py_VerboseFlag) + PySys_WriteStderr("# clear builtins._\n"); + PyDict_SetItemString(interp->builtins, "_", Py_None); + + for (p = sys_deletes; *p != NULL; p++) { if (Py_VerboseFlag) - PySys_WriteStderr("# clear builtins._\n"); - PyDict_SetItemString(dict, "_", Py_None); + PySys_WriteStderr("# clear sys.%s\n", *p); + PyDict_SetItemString(interp->sysdict, *p, Py_None); } - value = PyDict_GetItemString(modules, "sys"); - if (value != NULL && PyModule_Check(value)) { - char **p; - PyObject *v; - dict = PyModule_GetDict(value); - for (p = sys_deletes; *p != NULL; p++) { - if (Py_VerboseFlag) - PySys_WriteStderr("# clear sys.%s\n", *p); - PyDict_SetItemString(dict, *p, Py_None); - } - for (p = sys_files; *p != NULL; p+=2) { - if (Py_VerboseFlag) - PySys_WriteStderr("# restore sys.%s\n", *p); - v = PyDict_GetItemString(dict, *(p+1)); - if (v == NULL) - v = Py_None; - PyDict_SetItemString(dict, *p, v); - } + for (p = sys_files; *p != NULL; p+=2) { + if (Py_VerboseFlag) + PySys_WriteStderr("# restore sys.%s\n", *p); + value = PyDict_GetItemString(interp->sysdict, *(p+1)); + if (value == NULL) + value = Py_None; + PyDict_SetItemString(interp->sysdict, *p, value); } /* We prepare a list which will receive (name, weakref) tuples of @@ -473,11 +470,15 @@ PyImport_Cleanup(void) /* Clear the modules dict. */ PyDict_Clear(modules); - /* Replace the interpreter's reference to builtins with an empty dict - (module globals still have a reference to the original builtins). */ - builtins = interp->builtins; - interp->builtins = PyDict_New(); - Py_DECREF(builtins); + /* Restore the original builtins dict, to ensure that any + user data gets cleared. */ + dict = PyDict_Copy(interp->builtins); + if (dict == NULL) + PyErr_Clear(); + PyDict_Clear(interp->builtins); + if (PyDict_Update(interp->builtins, interp->builtins_copy)) + PyErr_Clear(); + Py_XDECREF(dict); /* Clear module dict copies stored in the interpreter state */ _PyState_ClearModules(); /* Collect references */ @@ -488,7 +489,15 @@ PyImport_Cleanup(void) /* Now, if there are any modules left alive, clear their globals to minimize potential leaks. All C extension modules actually end - up here, since they are kept alive in the interpreter state. */ + up here, since they are kept alive in the interpreter state. + + The special treatment of "builtins" here is because even + when it's not referenced as a module, its dictionary is + referenced by almost every module's __builtins__. Since + deleting a module clears its dictionary (even if there are + references left to it), we need to delete the "builtins" + module last. Likewise, we don't delete sys until the very + end because it is implicitly referenced (e.g. by print). */ if (weaklist != NULL) { Py_ssize_t i, n; n = PyList_GET_SIZE(weaklist); @@ -498,17 +507,27 @@ PyImport_Cleanup(void) PyObject *mod = PyWeakref_GET_OBJECT(PyTuple_GET_ITEM(tup, 1)); if (mod == Py_None) continue; - Py_INCREF(mod); assert(PyModule_Check(mod)); + dict = PyModule_GetDict(mod); + if (dict == interp->builtins || dict == interp->sysdict) + continue; + Py_INCREF(mod); if (Py_VerboseFlag && PyUnicode_Check(name)) - PySys_FormatStderr("# cleanup[3] wiping %U\n", - name, mod); + PySys_FormatStderr("# cleanup[3] wiping %U\n", name); _PyModule_Clear(mod); Py_DECREF(mod); } Py_DECREF(weaklist); } + /* Next, delete sys and builtins (in that order) */ + if (Py_VerboseFlag) + PySys_FormatStderr("# cleanup[3] wiping sys\n"); + _PyModule_ClearDict(interp->sysdict); + if (Py_VerboseFlag) + PySys_FormatStderr("# cleanup[3] wiping builtins\n"); + _PyModule_ClearDict(interp->builtins); + /* Clear and delete the modules directory. Actual modules will still be there only if imported during the execution of some destructor. */ diff --git a/Python/pystate.c b/Python/pystate.c index 19fceb7127a..2ac2fd5274d 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -72,6 +72,7 @@ PyInterpreterState_New(void) interp->modules_by_index = NULL; interp->sysdict = NULL; interp->builtins = NULL; + interp->builtins_copy = NULL; interp->tstate_head = NULL; interp->codec_search_path = NULL; interp->codec_search_cache = NULL; @@ -115,6 +116,7 @@ PyInterpreterState_Clear(PyInterpreterState *interp) Py_CLEAR(interp->modules_by_index); Py_CLEAR(interp->sysdict); Py_CLEAR(interp->builtins); + Py_CLEAR(interp->builtins_copy); Py_CLEAR(interp->importlib); }