Many __del__ methods expect that builtins are existing at the time of executing. But when the object survives until the builtins module is wiped, __del__ will fail with unexpected AttributeError (see issue19021 for example). We can change __del__'s in the stdlib to not use non-cached builtins, but third party code will be broken.

Perhaps we should special case shutdown of the builtins module. E.g. reset it to initial value (all initial members of the builtins module are immutable and can't create resource loops) instead wiping.

> Perhaps we should special case shutdown of the builtins module. E.g.
> reset it to initial value (all initial members of the builtins module
> are immutable and can't create resource loops) instead wiping.

This sounds like a reasonable approach.

I'm not 100% sure that this is what intended, but probably this patch can fix it? Please review it.

Thanks for the patch, Gennadiy. Two comments:
1) I don't think re-doing the whole initialization like that is ok. It's too brutal and may also not work as you want.
2) To validate your changes, you should add a unit test anyway (in test_builtin perhaps).

Also, since such changes are subtle and error-prone, I think it's better to defer until 3.5 now that 3.4b2 has been released. Serhiy, what do you think?

Here is simpler patch with a test.

Unfortunately this patch doesn't fix issue19021. Popen.__del__ is called when whiping the idlelib.rpc module which was collected in a weaklist. But the builtins module also was collected in a weaklist and wiped before idlelib.rpc.

Here is revised patch which fixes issue19021. It defers wiping of the builtins and sys modules (this partially restores 3.3 algorithm) and fixes other related issues (some of these fixes should be backported to older versions).

Here is backported to 3.3 patch. It includes:

1. Operates not with sys.modules['builtins'].__dict__ and sys.modules['sys'].__dict__, but with cached interp->builtins and interp->sysdict, because sys.modules['builtins'] and sys.modules['sys'] can be changed, but builtins and standard streams for builtins are retrieved from cached interp->builtins and interp->sysdict.

2. interp->builtins is restored to saved copy.

3. Backported test. Actually it passed with current code, but can prevent future bugs.

I'm not sure about (2), but may be (1) should be applied.

I'm not convinced this should be backported. It's always a bit risky to change interpreter shutdown, and it fixes an issue which has been well-known (and worked around) for many years.

New changeset 6a1711c96fa6 by Serhiy Storchaka in branch 'default':
Issue #19255: The builtins module is restored to initial value before
http://hg.python.org/cpython/rev/6a1711c96fa6

I agree that it is risky and don't insist. We can return to this patch when encounter more serious bug.

Last commit breaks test_create_at_shutdown_without_encoding in test_io for Python implementation of io.

This happens because Python implementation of TextIOWrapper.__init__() imports the locale module if encoding is not specified. Then _find_spec() in Lib/importlib/_bootstrap.py iterates sys.meta_path, but "meta_path" is one of names cleared in sys in PyImport_Cleanup().

> This happens because Python implementation of TextIOWrapper.__init__()
> imports the locale module if encoding is not specified. Then
> _find_spec() in Lib/importlib/_bootstrap.py iterates sys.meta_path,
> but "meta_path" is one of names cleared in sys in PyImport_Cleanup().

Then perhaps PyImport_Cleanup should call _PyImportHooks_Init instead of
setting import-related attributes to None.

New changeset fa160c8145e5 by Serhiy Storchaka in branch 'default':
Temporary silence test broken by issue19255.
http://hg.python.org/cpython/rev/fa160c8145e5

test_cleanup() of test_builtin fails: I opened issue #20599 to track this bug.

Turns out call _PyImportHooks_Init is not enough, see issue20602 and issue20603.

Your checkin 6a1711c96fa6 caused a Coverity complain:

** CID 1171506:  Unchecked return value  (CHECKED_RETURN)
/Objects/moduleobject.c: 352 in _PyModule_ClearDict()
/Objects/moduleobject.c: 333 in _PyModule_ClearDict()


________________________________________________________________________________________________________
*** CID 1171506:  Unchecked return value  (CHECKED_RETURN)
/Objects/moduleobject.c: 352 in _PyModule_ClearDict()
346                         const char *s = _PyUnicode_AsString(key);
347                         if (s != NULL)
348                             PySys_WriteStderr("#   clear[2] %s\n", s);
349                         else
350                             PyErr_Clear();
351                     }
>>>     CID 1171506:  Unchecked return value  (CHECKED_RETURN)
>>>     No check of the return value of "PyDict_SetItem(d, key, &_Py_NoneStruct)".
352                     PyDict_SetItem(d, key, Py_None);
353                 }
354             }
355         }
356     
357         /* Note: we leave __builtins__ in place, so that destructors
/Objects/moduleobject.c: 333 in _PyModule_ClearDict()
327                         const char *s = _PyUnicode_AsString(key);
328                         if (s != NULL)
329                             PySys_WriteStderr("#   clear[1] %s\n", s);
330                         else
331                             PyErr_Clear();
332                     }
>>>     CID 1171506:  Unchecked return value  (CHECKED_RETURN)
>>>     No check of the return value of "PyDict_SetItem(d, key, &_Py_NoneStruct)".
333                     PyDict_SetItem(d, key, Py_None);
334                 }
335             }
336         }
337     
338         /* Next, clear all names except for __builtins__ */

The two problematic code paths were marked as "intentional" and "ignore" in the past. There is no point in reporting errors that late in the shutdown phase. Also the code paths are very unlikely to trigger an error.

> Your checkin 6a1711c96fa6 caused a Coverity complain:
>
> ** CID 1171506:  Unchecked return value  (CHECKED_RETURN)
> /Objects/moduleobject.c: 352 in _PyModule_ClearDict()
> /Objects/moduleobject.c: 333 in _PyModule_ClearDict()

Actually this was in old code. May now the code is just simpler for analysis by Coverity.

What can we do with this? Only call PyErr_Clear() as for other errors.

It's fine. We don't *want* to check the error and fail here.

New changeset c6e7ba7eee88 by Serhiy Storchaka in branch '2.7':
Issue #19255: Clear error after failed PyDict_SetItem() on shutdown.
http://hg.python.org/cpython/rev/c6e7ba7eee88

New changeset ffc4f03adcac by Serhiy Storchaka in branch '3.3':
Issue #19255: Clear error after failed PyDict_SetItem() on shutdown.
http://hg.python.org/cpython/rev/ffc4f03adcac

New changeset efaf12106d68 by Serhiy Storchaka in branch 'default':
Issue #19255: Clear error after failed PyDict_SetItem() on shutdown.
http://hg.python.org/cpython/rev/efaf12106d68

New changeset 5d3b9862f1bc by Serhiy Storchaka in branch '3.4':
Issue #19255: The builtins module is restored to initial value before
http://hg.python.org/cpython/rev/5d3b9862f1bc

New changeset a0bc45a50e43 by Serhiy Storchaka in branch '3.4':
Temporary silence test broken by issue19255.
http://hg.python.org/cpython/rev/a0bc45a50e43

New changeset af8f856db136 by Serhiy Storchaka in branch '3.4':
Issue #19255: Clear error after failed PyDict_SetItem() on shutdown.
http://hg.python.org/cpython/rev/af8f856db136