Issue 16384: import.c doesn't handle EOFError from PyMarshal_Read* (original) (raw)

Created on 2012-11-01 18:56 by syeberman, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
issue16384.diff eric.snow,2016-09-07 18:25 review
Messages (10)
msg174438 - (view) Author: Sye van der Veen (syeberman) * Date: 2012-11-01 18:56
The PyMarshal_Read* functions raise EOFError when the end of the file is unexpectedly met. The current import.c functions propagate this error when reading .pyc or .pyo files. One consequence of this is that Python will abort on startup if, say, encodings.utf_8's .pyc file is truncated. I have encountered a scenario where this truncation is common. If a second Python process is launched while the first is writing the "core" .pyc files, the second process may open the files before they are completely written and will thus see truncated files and abort. This is a race condition that I was able to reproduce consistently on several Windows Server 2008 RC2 Standard SP1 machines running 32-bit Python 3.2.3 from GNU make with "-j 16" (Intel Xeon E5405 2GHz 2 processors 8GB 64-bit OS). (Of course, I had to clean the __pycache__ directories between tests.) This can be fixed in load_source_module by making read_compiled_module failures non-fatal: if (cpathname != NULL && (fpc = check_compiled_module(pathname, st.st_mtime, cpathname))) { co = read_compiled_module(cpathname, fpc); if (co == NULL) PyErr_Clear(); fclose(fpc); } if (co != NULL) { // etc... } else { co = parse_source_module(pathname, fp); // etc... write_compiled_module(co, cpathname, &st); } This is similar to how write_compiled_module ignores failures in writing the .pyc files. It ensures that if the .pyc file is corrupt for _any_ reason, it will get rewritten; this could be made specific to EOFError, but I don't recommed that. Mostly, it ensures that corrupt .pyc files do not prevent Python from loading if the .py files are valid.
msg174591 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2012-11-02 21:55
The import machinery was changed in 3.3.0. I suggest checking if it gives you *exactly* the same behavior.
msg274705 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2016-09-07 02:22
I have verified that this is still a problem (basically 3.6b1). Fatal Python error: Py_Initialize: Unable to get the locale encoding Traceback (most recent call last): File "/opt/python3.6/lib/python3.6/encodings/__init__.py", line 99, in search_function File "<frozen importlib._bootstrap>", line 979, in _find_and_load File "<frozen importlib._bootstrap>", line 968, in _find_and_load_unlocked File "<frozen importlib._bootstrap>", line 673, in _load_unlocked File "<frozen importlib._bootstrap_external>", line 663, in exec_module File "<frozen importlib._bootstrap_external>", line 768, in get_code File "<frozen importlib._bootstrap_external>", line 478, in _compile_bytecode EOFError: marshal data too short Aborted (core dumped) We should be exiting with an error rather than aborting.
msg274741 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2016-09-07 03:29
After looking more closely, it looks like we should be ignoring such bogus modules. Here's a patch to do so.
msg274792 - (view) Author: Sye van der Veen (syeberman) * Date: 2016-09-07 11:04
I feel this patch () misses the mark. Any two Python processes that try to import a module, without a pyc, at the same time could suffer race conditions. The first process will start to write the pyc, get interrupted, and the second will fail with an EOFError. When importing encodings at startup, this is a nasty abort. But it's just as nasty if a running script gets unpredictable EOFErrors. Corrupt .pyc files should not prevent importing if the valid .py files are available. On Tue, Sep 6, 2016, 11:29 PM Eric Snow <report@bugs.python.org> wrote: > > Eric Snow added the comment: > > After looking more closely, it looks like we should be ignoring such bogus > modules. Here's a patch to do so. > > ---------- > keywords: +patch > stage: test needed -> patch review > Added file: http://bugs.python.org/file44424/issue16384.diff > > _______________________________________ > Python tracker <report@bugs.python.org> > <http://bugs.python.org/issue16384> > _______________________________________ >
msg274829 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2016-09-07 16:27
It's a question of whether you want the error that your .pyc files have somehow ended up in a bad state to pass silently or you should know you that something on your system is corrupting .pyc files (hence why the EOFError has been allowed to propagate). Making corrupt .pyc files pass silently would be a change in semantics and practice, so we need to think through the ramifications. At worst we would need to raise a warning that such a thing happened, if not keep the current semantics.
msg274858 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2016-09-07 18:25
Here's an updated patch. Per a suggestion from Brett, I've chained the original EOFError with an ImportError. The consequence is that the problematic encoding is skipped (silently) rather than causing the interpreter to abort. FYI, I've opened issue #28005 to address how we silently skip encodings that fail to import.
msg274873 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2016-09-07 19:51
I've opened #28007 to cover the concerns about bad a .pyc file blocking import from a valid .py file.
msg276127 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2016-09-12 22:20
With issue #28007 wrapped up, there isn't a lot left to do here. I was considering that we don't want to abort when we have problems loading a codec during startup. However, Steve Dower made the point to me that a problem with the main codec during startup should be fatal. Consequently I'm closing this bug.
msg276473 - (view) Author: Sye van der Veen (syeberman) * Date: 2016-09-14 18:41
I would also agree that failing to load the main codec should be an abort. This bug was specifically the race condition in writing the .pyc file. Thanks for all your help!
History
Date User Action Args
2022-04-11 14:57:37 admin set github: 60588
2016-09-14 18:41:56 syeberman set messages: +
2016-09-12 22:20:00 eric.snow set status: open -> closedsuperseder: Bad .pyc files prevent import of otherwise valid .py files.messages: + resolution: duplicatestage: patch review -> resolved
2016-09-07 19:51:12 eric.snow set messages: +
2016-09-07 18:26:01 eric.snow set files: - issue16384.diff
2016-09-07 18:25:09 eric.snow set files: + issue16384.diffmessages: +
2016-09-07 16:27:24 brett.cannon set messages: +
2016-09-07 11:04:34 syeberman set messages: +
2016-09-07 03:29:06 eric.snow set files: + issue16384.diffkeywords: + patchmessages: + stage: test needed -> patch review
2016-09-07 02:22:18 eric.snow set stage: test neededmessages: + components: + Interpreter Core, - Noneversions: + Python 3.6, - Python 3.2
2012-11-13 07:16:37 eric.snow set nosy: + eric.snow
2012-11-02 21:55:37 terry.reedy set nosy: + terry.reedymessages: +
2012-11-02 13:24:56 brett.cannon set type: crash -> behavior
2012-11-02 13:24:40 brett.cannon set nosy: + brett.cannon
2012-11-01 18:56:19 syeberman create