Issue #19255: The builtins module is restored to initial value before

cleaning other modules.  The sys and builtins modules are cleaned last.

7 files changed, 97 insertions, 40 deletions

diff --git a/Include/moduleobject.h b/Include/moduleobject.h
index 8013dd9b48..f119364b88 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 1f3465fdbd..4992c22684 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 68430660f0..c078b443c5 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 142a82aa6a..f64235364a 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -89,6 +89,9 @@ Release date: 2014-02-10
 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 d59475e2f3..682171070e 100644
--- a/Objects/moduleobject.c
+++ b/Objects/moduleobject.c
@@ -299,6 +299,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
        whose name starts with a single underscore, before we clear
@@ -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 d0115e46bf..207dbced41 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);
-    }
-    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);
-        }
+            PySys_WriteStderr("# clear sys.%s\n", *p);
+        PyDict_SetItemString(interp->sysdict, *p, Py_None);
+    }
+    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 19fceb7127..2ac2fd5274 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);
 }