New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Writing a pyc file is not atomic #57355
Comments
One of the buildbots recently showed the following failure: ====================================================================== Traceback (most recent call last):
File "/var/lib/buildslave/3.x.murray-gentoo/build/Lib/test/test_multiprocessing.py", line 1442, in test_rapid_restart
queue = manager.get_queue()
File "/var/lib/buildslave/3.x.murray-gentoo/build/Lib/multiprocessing/managers.py", line 670, in temp
token, exp = self._create(typeid, *args, **kwds)
File "/var/lib/buildslave/3.x.murray-gentoo/build/Lib/multiprocessing/managers.py", line 568, in _create
conn = self._Client(self._address, authkey=self._authkey)
File "/var/lib/buildslave/3.x.murray-gentoo/build/Lib/multiprocessing/connection.py", line 778, in XmlClient
import xmlrpc.client as xmlrpclib
File "/var/lib/buildslave/3.x.murray-gentoo/build/Lib/xmlrpc/client.py", line 137, in <module>
import http.client
File "/var/lib/buildslave/3.x.murray-gentoo/build/Lib/http/client.py", line 69, in <module>
import email.parser
File "/var/lib/buildslave/3.x.murray-gentoo/build/Lib/email/parser.py", line 12, in <module>
from email.feedparser import FeedParser
File "/var/lib/buildslave/3.x.murray-gentoo/build/Lib/email/feedparser.py", line 28, in <module>
from email import policy
EOFError: EOF read where not expected (http://www.python.org/dev/buildbot/all/builders/x86%20Gentoo%203.x/builds/942/steps/test/logs/stdio) "EOF read where not expected" comes from reading a pyc file in marshal.c. It is raised when the pyc file is somehow truncated or incomplete. Writing and reading the same pyc file is protected by the import lock when in a single interpreter, but not when running several Python processes at the same time (which test_multiprocessing obviously does). Under POSIX, import.c could do the traditional write-then-rename dance which guarantees that the file contents appear atomically. |
Here is a patch for import.c. |
This new patch also fixes importlib. |
So if a process replaces the PYC file whereas another is reading the PYC, the reader may read corrupted data? The ideal fix is maybe to use a file lock? |
No, this is the whole point of the patch. |
Looks good to me.
""" I don't know exactly the context in which this code runs, but you can have a corruption if multiple processes try to write the bytecode file at the same time, since they'll all open the .tmp file: it should be opened with O_EXCL. Also, as a side note, I'm wondering whether this type of check: couldn't be rewritten as Fox example, does OS-X report as POSIX? |
Or perhaps append the PID to the name of the temp file ?
No, because os.py is not available to importlib (which must be
I think so. |
I don't really like appending PIDs to generate file names:
O_EXCL is really what POSIX offers to solve this (and it's also what
OK. So is the O_EXCL approach possible? Would something like work? Also, since this can be quite tricky and redundant, how about adding a where atomic_create would be a context manager that would make |
Ok, here is a new patch using O_EXCL. |
New changeset c16063765d3a by Antoine Pitrou in branch 'default': |
Should be fixed now. Thanks for the reviews! |
And thanks for doing this, Antoine! One less thing on my never-ending On Mon, Oct 17, 2011 at 10:35, Antoine Pitrou <report@bugs.python.org> wrote:
|
Proposed patch for 2.7 |
I'm re-opening this because I'd like to get RM pronouncement on applying a patch to 2.7, 3.2, and 3.3 to make py_compile.py atomically rename its pyc/pyo file. Attached is a patch for 2.7 based on importlib's approach in 3.4. It should be easy enough to port to Python 3. |
Oh btw, if Georg and Benjamin deny this for the stable releases, I'll very likely patch the Ubuntu versions anyway. |
Some people already complained about this change. I'm not sure it's fit Besides, you can just also make py_compile write to a temporary file, |
On May 20, 2013, at 09:52 PM, Antoine Pitrou wrote:
Yeah, but that's a crazy use case. :)
That actually doesn't work as well for us, since we feed .py file names to |
IIRC, os.rename() will fail on Windows if the target file already exists. |
Good point. |
On May 20, 2013, at 09:57 PM, Charles-François Natali wrote:
Ah, that's probably a more serious blocker for adding it to upstream Python. |
The workaround would be to unlink the file first, and then try to As for issue bpo-17222, well, many applications use temporary files and Its funny how an seemingly harmless change can introduce nasty regressions... |
Barry, do you still want to keep this issue open? |
On Aug 15, 2013, at 08:25 PM, Antoine Pitrou wrote:
I don't necessarily need to. We've patched the Ubuntu version to be safe, so |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: