Skip to content

Commit fe317f5

Browse files
picnixzmiss-islington
authored andcommitted
gh-155717: use spawn as the default start method for read-only filesystems (GH-155827)
The "forkserver" start method (the default start method on non-Windows systems) requires the ability to write temporary files, which is not possible if TMPDIR is read-only (e.g., k8s containers mounted with `readOnlyRootFilesystem=True`). On such filesystems, the default start method changes from "forkserver" to "spawn". (cherry picked from commit c144799) Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
1 parent 422e679 commit fe317f5

4 files changed

Lines changed: 73 additions & 4 deletions

File tree

Lib/multiprocessing/context.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
from . import process
66
from . import reduction
7+
from . import util
78

89
__all__ = ()
910

@@ -328,7 +329,12 @@ def _check_available(self):
328329
# bpo-33725: running arbitrary code after fork() is no longer reliable
329330
# on macOS since macOS 10.14 (Mojave). Use spawn by default instead.
330331
# gh-84559: We changed everyones default to a thread safeish one in 3.14.
331-
if reduction.HAVE_SEND_HANDLE and sys.platform != 'darwin':
332+
if (
333+
reduction.HAVE_SEND_HANDLE
334+
and sys.platform != 'darwin'
335+
# gh-155717: forkserver requires to write temporary files
336+
and util._has_writeable_tempdir()
337+
):
332338
_default_context = DefaultContext(_concrete_contexts['forkserver'])
333339
else:
334340
_default_context = DefaultContext(_concrete_contexts['spawn'])

Lib/multiprocessing/util.py

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import os
1111
import itertools
1212
import sys
13+
import tempfile
1314
import weakref
1415
import atexit
1516
import threading # we want threading to install it's
@@ -143,6 +144,7 @@ def is_abstract_socket_namespace(address):
143144
# On Windows platforms, we do not create AF_UNIX sockets.
144145
_SUN_PATH_MAX = None if os.name == 'nt' else 92
145146

147+
146148
def _remove_temp_dir(rmtree, tempdir):
147149
rmtree(tempdir)
148150

@@ -152,7 +154,8 @@ def _remove_temp_dir(rmtree, tempdir):
152154
if current_process is not None:
153155
current_process._config['tempdir'] = None
154156

155-
def _get_base_temp_dir(tempfile):
157+
158+
def _get_base_temp_dir():
156159
"""Get a temporary directory where socket files will be created.
157160
158161
To prevent additional imports, pass a pre-imported 'tempfile' module.
@@ -208,12 +211,13 @@ def _get_base_temp_dir(tempfile):
208211
assert len(base_system_tempdir) + 14 + 14 < _SUN_PATH_MAX
209212
return base_system_tempdir
210213

214+
211215
def get_temp_dir():
212216
# get name of a temp directory which will be automatically cleaned up
213217
tempdir = process.current_process()._config.get('tempdir')
214218
if tempdir is None:
215-
import shutil, tempfile
216-
base_tempdir = _get_base_temp_dir(tempfile)
219+
import shutil
220+
base_tempdir = _get_base_temp_dir()
217221
tempdir = tempfile.mkdtemp(prefix='pymp-', dir=base_tempdir)
218222
info('created temp directory %s', tempdir)
219223
# keep a strong reference to shutil.rmtree(), since the finalizer
@@ -223,6 +227,27 @@ def get_temp_dir():
223227
process.current_process()._config['tempdir'] = tempdir
224228
return tempdir
225229

230+
231+
def _has_writeable_tempdir():
232+
# 'forkserver' requires writeable temporary files. This function is
233+
# called to determine the default context's start method.
234+
#
235+
# See: https://github.com/python/cpython/issues/155717.
236+
237+
path = _get_base_temp_dir()
238+
if path is None:
239+
return False
240+
241+
# os.access() is advisory and racy. It can lie on read-only filesystems,
242+
# NFS/network mounts, containers, and immutable-flag files, so we simply
243+
# try to create a file to check if this works and delete it otherwise.
244+
try:
245+
with tempfile.NamedTemporaryFile(dir=path):
246+
return True
247+
except OSError:
248+
return False
249+
250+
226251
#
227252
# Support for reinitialization of objects when bootstrapping a child process
228253
#

Lib/test/_test_multiprocessing.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import struct
2727
import tempfile
2828
import operator
29+
import pathlib
2930
import pickle
3031
import weakref
3132
import warnings
@@ -6051,6 +6052,40 @@ def test_nested_startmethod(self):
60516052
# there is no synchronization in the test.
60526053
self.assertSetEqual(set(results), set([2, 1]))
60536054

6055+
@unittest.skipIf(os.name == "nt", "requires POSIX")
6056+
@support.subTests("mode", [
6057+
os.R_OK, # read-only directory
6058+
os.R_OK | os.X_OK, # read-only directory
6059+
os.W_OK # write-only directory _without_ permissions for creating files
6060+
])
6061+
def test_forkserver_requires_writeable_tempdir(self, mode):
6062+
# Regression test to ensure that the defualt start method is
6063+
# not 'forkserver' when the temporary directory is not writeable.
6064+
#
6065+
# See https://github.com/python/cpython/issues/155717.
6066+
6067+
cmd = '''if 1:
6068+
import os, tempfile
6069+
# We fake the read-onlyiness of /tmp (which is a fallback when
6070+
# the user-defined TMPDIR is not acceptable) by hardcoding the
6071+
# temporary directory for this specific test.
6072+
tempfile.tempdir = os.environ["TMPDIR"]
6073+
6074+
# Imported after patching 'tempfile' so that the default start
6075+
# method is deduced according to the permissions of TMPDIR.
6076+
import multiprocessing
6077+
if __name__ == "__main__":
6078+
print(multiprocessing.get_start_method())
6079+
'''
6080+
6081+
with support.os_helper.temp_dir() as root:
6082+
TMPDIR = pathlib.Path(root, "TMPDIR")
6083+
TMPDIR.mkdir(mode=mode)
6084+
file = pathlib.Path(TMPDIR, "file")
6085+
self.assertRaises(OSError, file.touch)
6086+
_, out, err = script_helper.assert_python_ok('-c', cmd, TMPDIR=TMPDIR)
6087+
self.assertEqual(out.decode().strip(), "spawn")
6088+
60546089

60556090
@unittest.skipIf(sys.platform == "win32",
60566091
"test semantics don't make sense on Windows")
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
:mod:`multiprocessing`'s default start method on systems with non-writeable
2+
tempfile filesystem is now :ref:`"spawn" <multiprocessing-start-methods>`
3+
instead of ``"forkserver"``. Patch by Bénédikt Tran.

0 commit comments

Comments
 (0)