Skip to content
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

gh-126606: don't write incomplete pyc files #126627

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion Lib/importlib/_bootstrap_external.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,11 @@ def _write_atomic(path, data, mode=0o666):
# We first write data to a temporary file, and then use os.replace() to
# perform an atomic rename.
with _io.FileIO(fd, 'wb') as file:
file.write(data)
brettcannon marked this conversation as resolved.
Show resolved Hide resolved
bytes_written = file.write(data)
if bytes_written != len(data):
# Raise an OSError so the 'except' below cleans up the partially
# written file.
raise OSError("os.write() didn't write the full pyc file")
_os.replace(path_tmp, path)
except OSError:
try:
Expand Down
32 changes: 32 additions & 0 deletions Lib/test/test_importlib/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@
importlib_util = util.import_importlib('importlib.util')

import importlib.util
from importlib import _bootstrap_external
import os
import pathlib
import re
import string
import sys
from test import support
from test.support import os_helper
import textwrap
import types
import unittest
Expand Down Expand Up @@ -775,5 +777,35 @@ def test_complete_multi_phase_init_module(self):
self.run_with_own_gil(script)


class MiscTests(unittest.TestCase):
def test_atomic_write_should_notice_incomplete_writes(self):
import _pyio

oldwrite = os.write
seen_write = False

truncate_at_length = 100

# Emulate an os.write that only writes partial data.
def write(fd, data):
nonlocal seen_write
seen_write = True
return oldwrite(fd, data[:truncate_at_length])

# Need to patch _io to be _pyio, so that io.FileIO is affected by the
# os.write patch.
with (support.swap_attr(_bootstrap_external, '_io', _pyio),
support.swap_attr(os, 'write', write)):
with self.assertRaises(OSError):
# Make sure we write something longer than the point where we
# truncate.
content = b'x' * (truncate_at_length * 2)
_bootstrap_external._write_atomic(os_helper.TESTFN, content)
assert seen_write

with self.assertRaises(OSError):
os.stat(support.os_helper.TESTFN) # Check that the file did not get written.


if __name__ == '__main__':
unittest.main()
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix :mod:`importlib` to not write an incomplete .pyc files when a ulimit or some
other operating system mechanism is preventing the write to go through
fully.
Loading