Skip to content

Commit 6719378

Browse files
miss-islingtonpicnixzgpsheadfedonman
authored
[3.14] gh-155717: use spawn as the default start method for read-only filesystems (GH-155827) (#155886)
* 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> * Update Lib/test/_test_multiprocessing.py * [3.14] gh-155717: Determine multiprocessing's default start method lazily (GH-159254) gh-155827 made `import multiprocessing` check whether the temporary directory is writeable in order to choose between "forkserver" and "spawn" as the default start method. That check ran at import time and pulled in `multiprocessing.util`, `subprocess`, `tempfile`, `shutil` and `random`, noticeably slowing down the import for every program, including ones that never start a process and every spawned child, the fork server and the resource tracker. The default start method is now determined the first time it is needed instead: on the first `get_context()`, `get_start_method()`, `get_all_start_methods()` or default `Process` start. `multiprocessing.util` goes back to importing `tempfile` only where it is used. Also include the fix from gh-158410 by Vyron Vasileiadis, along with a test adapted from that PR: when `tempfile` finds no usable temporary directory at all, the check now reports that instead of raising `FileNotFoundError`, so that "spawn" is chosen. The regression was reported, and the lazy approach suggested, by Leandro Damascena in #155717 (comment) (cherry picked from commit 48ddda1) Co-authored-by: Vyron Vasileiadis <hi@fedonman.com> * gh-155717: Reword the NEWS entry for the default start method change Say that the change applies when there is no writable temporary directory and that starting a process previously failed there. --------- Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com> Co-authored-by: Gregory P. Smith <68491+gpshead@users.noreply.github.com> Co-authored-by: Vyron Vasileiadis <hi@fedonman.com> Co-authored-by: Gregory P. Smith <greg@krypto.org>
1 parent 3ef92a2 commit 6719378

5 files changed

Lines changed: 172 additions & 11 deletions

File tree

‎Doc/library/multiprocessing.rst‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,11 +160,21 @@ to start a process. These *start methods* are
160160
No unnecessary resources are inherited.
161161

162162
Available on POSIX platforms which support passing file descriptors over
163-
Unix pipes such as Linux. The default on those.
163+
Unix pipes such as Linux. The default on those, unless there is no
164+
writable temporary directory (see :func:`tempfile.gettempdir`), which
165+
the fork server requires; *spawn* is the default in that case.
164166

165167
.. versionchanged:: 3.14
166168
This became the default start method on POSIX platforms.
167169

170+
.. versionchanged:: next
171+
*spawn* is now the default start method when there is no writable
172+
temporary directory, for example on a read-only file system.
173+
Previously *forkserver* remained the default there and starting a
174+
process failed with an :exc:`OSError`. The default is determined
175+
when it is first needed rather than when :mod:`!multiprocessing`
176+
is imported. See :gh:`155717`.
177+
168178
.. versionchanged:: 3.4
169179
*spawn* added on all POSIX platforms, and *forkserver* added for
170180
some POSIX platforms.

‎Lib/multiprocessing/context.py‎

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import functools
12
import os
23
import sys
34
import threading
@@ -236,10 +237,19 @@ def _after_fork():
236237
class DefaultContext(BaseContext):
237238
Process = Process
238239

239-
def __init__(self, context):
240-
self._default_context = context
240+
def __init__(self, context=None):
241+
if context is not None:
242+
self._default_context = context
241243
self._actual_context = None
242244

245+
# The platform default is only determined when first needed, as doing
246+
# so can be costly; importing multiprocessing must stay cheap.
247+
# A cached_property rather than a property so that the attribute can
248+
# still be assigned, deleted and patched like a plain one.
249+
@functools.cached_property
250+
def _default_context(self):
251+
return _get_platform_default_context()
252+
243253
def get_context(self, method=None):
244254
if method is None:
245255
if self._actual_context is None:
@@ -325,13 +335,22 @@ def _check_available(self):
325335
'spawn': SpawnContext(),
326336
'forkserver': ForkServerContext(),
327337
}
328-
# bpo-33725: running arbitrary code after fork() is no longer reliable
329-
# on macOS since macOS 10.14 (Mojave). Use spawn by default instead.
330-
# 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-
_default_context = DefaultContext(_concrete_contexts['forkserver'])
333-
else:
334-
_default_context = DefaultContext(_concrete_contexts['spawn'])
338+
339+
def _get_platform_default_context():
340+
# bpo-33725: running arbitrary code after fork() is no longer reliable
341+
# on macOS since macOS 10.14 (Mojave). Use spawn by default instead.
342+
# gh-84559: We changed everyones default to a thread safeish one in 3.14.
343+
if reduction.HAVE_SEND_HANDLE and sys.platform != 'darwin':
344+
# gh-155717: forkserver requires to write temporary files.
345+
#
346+
# Imported here rather than at the top level as util is costly
347+
# to import and is not otherwise needed to import this module.
348+
from . import util
349+
if util._has_writeable_tempdir():
350+
return _concrete_contexts['forkserver']
351+
return _concrete_contexts['spawn']
352+
353+
_default_context = DefaultContext()
335354

336355
else: # Windows
337356

@@ -354,7 +373,10 @@ class SpawnContext(BaseContext):
354373
_concrete_contexts = {
355374
'spawn': SpawnContext(),
356375
}
357-
_default_context = DefaultContext(_concrete_contexts['spawn'])
376+
def _get_platform_default_context():
377+
return _concrete_contexts['spawn']
378+
379+
_default_context = DefaultContext()
358380

359381
#
360382
# Force the start method

‎Lib/multiprocessing/util.py‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ def is_abstract_socket_namespace(address):
143143
# On Windows platforms, we do not create AF_UNIX sockets.
144144
_SUN_PATH_MAX = None if os.name == 'nt' else 92
145145

146+
146147
def _remove_temp_dir(rmtree, tempdir):
147148
rmtree(tempdir)
148149

@@ -152,6 +153,7 @@ def _remove_temp_dir(rmtree, tempdir):
152153
if current_process is not None:
153154
current_process._config['tempdir'] = None
154155

156+
155157
def _get_base_temp_dir(tempfile):
156158
"""Get a temporary directory where socket files will be created.
157159
@@ -208,10 +210,12 @@ def _get_base_temp_dir(tempfile):
208210
assert len(base_system_tempdir) + 14 + 14 < _SUN_PATH_MAX
209211
return base_system_tempdir
210212

213+
211214
def get_temp_dir():
212215
# get name of a temp directory which will be automatically cleaned up
213216
tempdir = process.current_process()._config.get('tempdir')
214217
if tempdir is None:
218+
# Deferred until first use to keep importing this module cheap.
215219
import shutil, tempfile
216220
base_tempdir = _get_base_temp_dir(tempfile)
217221
tempdir = tempfile.mkdtemp(prefix='pymp-', dir=base_tempdir)
@@ -223,6 +227,34 @@ 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+
# Deferred until first use to keep importing this module cheap.
238+
import tempfile
239+
240+
try:
241+
path = _get_base_temp_dir(tempfile)
242+
except OSError:
243+
# No usable temporary directory at all.
244+
return False
245+
if path is None:
246+
return False
247+
248+
# os.access() is advisory and racy. It can lie on read-only filesystems,
249+
# NFS/network mounts, containers, and immutable-flag files, so we simply
250+
# try to create a file to check if this works and delete it otherwise.
251+
try:
252+
with tempfile.NamedTemporaryFile(dir=path):
253+
return True
254+
except OSError:
255+
return False
256+
257+
226258
#
227259
# Support for reinitialization of objects when bootstrapping a child process
228260
#

‎Lib/test/_test_multiprocessing.py‎

Lines changed: 92 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,97 @@ 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.requires_non_root_user
6057+
@support.subTests("mode", [
6058+
os.R_OK, # read-only directory
6059+
os.R_OK | os.X_OK, # read-only directory
6060+
os.W_OK # write-only directory _without_ permissions for creating files
6061+
])
6062+
def test_forkserver_requires_writeable_tempdir(self, mode):
6063+
# Regression test to ensure that the defualt start method is
6064+
# not 'forkserver' when the temporary directory is not writeable.
6065+
#
6066+
# See https://github.com/python/cpython/issues/155717.
6067+
6068+
cmd = '''if 1:
6069+
import os, tempfile
6070+
# We fake the read-onlyiness of /tmp (which is a fallback when
6071+
# the user-defined TMPDIR is not acceptable) by hardcoding the
6072+
# temporary directory for this specific test.
6073+
tempfile.tempdir = os.environ["TMPDIR"]
6074+
6075+
# Imported after patching 'tempfile' so that the default start
6076+
# method is deduced according to the permissions of TMPDIR.
6077+
import multiprocessing
6078+
if __name__ == "__main__":
6079+
print(multiprocessing.get_start_method())
6080+
'''
6081+
6082+
with support.os_helper.temp_dir() as root:
6083+
TMPDIR = pathlib.Path(root, "TMPDIR")
6084+
TMPDIR.mkdir(mode=mode)
6085+
file = pathlib.Path(TMPDIR, "file")
6086+
self.assertRaises(OSError, file.touch)
6087+
_, out, err = script_helper.assert_python_ok('-c', cmd, TMPDIR=TMPDIR)
6088+
self.assertEqual(out.decode().strip(), "spawn")
6089+
6090+
@unittest.skipIf(os.name == "nt", "requires POSIX")
6091+
@support.requires_non_root_user
6092+
def test_default_start_method_without_usable_tempdir(self):
6093+
# Neither importing multiprocessing nor determining the default
6094+
# start method may fail when tempfile finds no usable temporary
6095+
# directory at all, and the default start method must be 'spawn'.
6096+
#
6097+
# See https://github.com/python/cpython/issues/155717.
6098+
6099+
cmd = '''if 1:
6100+
import os, tempfile
6101+
# Make TMPDIR the only candidate, so that tempfile.gettempdir()
6102+
# raises FileNotFoundError instead of falling back to /tmp.
6103+
tempfile._candidate_tempdir_list = lambda: [os.environ["TMPDIR"]]
6104+
try:
6105+
tempfile.gettempdir()
6106+
except FileNotFoundError:
6107+
pass
6108+
else:
6109+
raise AssertionError("TMPDIR is usable")
6110+
6111+
import multiprocessing
6112+
if __name__ == "__main__":
6113+
print(multiprocessing.get_start_method(),
6114+
multiprocessing.get_all_start_methods()[0])
6115+
'''
6116+
6117+
with support.os_helper.temp_dir() as root:
6118+
TMPDIR = pathlib.Path(root, "TMPDIR")
6119+
TMPDIR.mkdir(mode=os.R_OK | os.X_OK)
6120+
_, out, err = script_helper.assert_python_ok('-c', cmd, TMPDIR=TMPDIR)
6121+
self.assertEqual(out.decode().split(), ["spawn", "spawn"])
6122+
6123+
def test_import_does_not_determine_default_start_method(self):
6124+
# Determining the default start method requires checking that the
6125+
# temporary directory is writeable. That must not happen, nor may
6126+
# the modules it needs be loaded, when importing multiprocessing.
6127+
import_helper.ensure_lazy_imports(
6128+
"multiprocessing", {"tempfile", "shutil", "subprocess"})
6129+
6130+
cmd = '''if 1:
6131+
import sys
6132+
import multiprocessing
6133+
print(multiprocessing.get_start_method(allow_none=True),
6134+
"tempfile" in sys.modules,
6135+
multiprocessing.get_start_method(),
6136+
multiprocessing.get_all_start_methods()[0])
6137+
'''
6138+
# -S: site customizations could import tempfile themselves.
6139+
_, out, err = script_helper.assert_python_ok('-S', '-c', cmd)
6140+
unset, probed, method, first = out.decode().split()
6141+
self.assertEqual(unset, "None")
6142+
self.assertEqual(probed, "False")
6143+
self.assertEqual(method, first)
6144+
self.assertIn(method, multiprocessing.get_all_start_methods())
6145+
60546146

60556147
@unittest.skipIf(sys.platform == "win32",
60566148
"test semantics don't make sense on Windows")
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
:mod:`multiprocessing`'s default start method is now
2+
:ref:`"spawn" <multiprocessing-start-methods>` instead of ``"forkserver"``
3+
when there is no writable temporary directory, such as on a read-only file
4+
system. Previously starting a process failed with an :exc:`OSError` there.
5+
Patch by Bénédikt Tran.

0 commit comments

Comments
 (0)