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

bpo-40280: Emscripten fork_exec now fails early (GH-32224) #32224

Merged
merged 2 commits into from
Apr 1, 2022
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
10 changes: 8 additions & 2 deletions Lib/subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,13 @@
"CREATE_NO_WINDOW", "DETACHED_PROCESS",
"CREATE_DEFAULT_ERROR_MODE", "CREATE_BREAKAWAY_FROM_JOB"])
else:
import _posixsubprocess
if sys.platform in {"emscripten", "wasi"}:
def _fork_exec(*args, **kwargs):
raise OSError(
errno.ENOTSUP, f"{sys.platform} does not support processes."
)
else:
from _posixsubprocess import fork_exec as _fork_exec
import select
import selectors

Expand Down Expand Up @@ -1777,7 +1783,7 @@ def _execute_child(self, args, executable, preexec_fn, close_fds,
for dir in os.get_exec_path(env))
fds_to_keep = set(pass_fds)
fds_to_keep.add(errpipe_write)
self.pid = _posixsubprocess.fork_exec(
self.pid = _fork_exec(
args, executable_list,
close_fds, tuple(sorted(map(int, fds_to_keep))),
cwd, env_list,
Expand Down
10 changes: 5 additions & 5 deletions Lib/test/test_subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -1805,7 +1805,7 @@ class PopenNoDestructor(subprocess.Popen):
def __del__(self):
pass

@mock.patch("subprocess._posixsubprocess.fork_exec")
@mock.patch("subprocess._fork_exec")
def test_exception_errpipe_normal(self, fork_exec):
"""Test error passing done through errpipe_write in the good case"""
def proper_error(*args):
Expand All @@ -1822,7 +1822,7 @@ def proper_error(*args):
with self.assertRaises(IsADirectoryError):
self.PopenNoDestructor(["non_existent_command"])

@mock.patch("subprocess._posixsubprocess.fork_exec")
@mock.patch("subprocess._fork_exec")
def test_exception_errpipe_bad_data(self, fork_exec):
"""Test error passing done through errpipe_write where its not
in the expected format"""
Expand Down Expand Up @@ -2112,7 +2112,7 @@ def raise_it():
preexec_fn=raise_it)
except subprocess.SubprocessError as e:
self.assertTrue(
subprocess._posixsubprocess,
subprocess._fork_exec,
"Expected a ValueError from the preexec_fn")
except ValueError as e:
self.assertIn("coconut", e.args[0])
Expand Down Expand Up @@ -2600,11 +2600,11 @@ def prepare():
preexec_fn=prepare)
except ValueError as err:
# Pure Python implementations keeps the message
self.assertIsNone(subprocess._posixsubprocess)
self.assertIsNone(subprocess._fork_exec)
self.assertEqual(str(err), "surrogate:\uDCff")
except subprocess.SubprocessError as err:
# _posixsubprocess uses a default message
self.assertIsNotNone(subprocess._posixsubprocess)
self.assertIsNotNone(subprocess._fork_exec)
self.assertEqual(str(err), "Exception occurred in preexec_fn.")
else:
self.fail("Expected ValueError or subprocess.SubprocessError")
Expand Down
3 changes: 3 additions & 0 deletions configure

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -6518,6 +6518,9 @@ AS_CASE([$ac_sys_system/$ac_sys_emscripten_target],
[_curses_panel],
[_dbm],
[_gdbm],
[_multiprocessing],
[_posixshmem],
[_posixsubprocess],
[_scproxy],
[_tkinter],
[_xxsubinterpreters],
Expand Down