Skip to content

bpo-41586: Add pipesize parameter to subprocess. Add F_GETPIPE_SZ and F_SETPIPE_SZ to fcntl. #21921

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

Merged
merged 17 commits into from
Oct 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
1e51d96
Add F_SETPIPE_SZ and F_GETPIPE_SZ to fcntl module
rhpvorderman Aug 18, 2020
a218c39
Add pipesize parameter for Popen class
rhpvorderman Aug 18, 2020
02b1d43
Add test for F_SETPIPE_SZ and F_GETPIPE_SZ
rhpvorderman Aug 18, 2020
af05503
Ensure cross-platform compatibility for pipesize in subprocess.Popen
rhpvorderman Aug 18, 2020
493d6c3
Add linux test for pipesize in subprocess
rhpvorderman Aug 18, 2020
aad24c6
Enable subprocess pipesize test for all platforms
rhpvorderman Aug 19, 2020
acf9ae4
Do not set pipesizes for Popen on Windows, as this can not be tested …
rhpvorderman Aug 19, 2020
6815aaa
Add F_GETPIPE_SZ, F_SETPIPE_SZ and subprocess.Popen pipesize argument…
rhpvorderman Aug 19, 2020
6bfd6a5
Add Ruben Vorderman to ACKS for work on the pipesize parameter in sub…
rhpvorderman Aug 19, 2020
cf97235
Add blurb for bpo 41586
rhpvorderman Aug 19, 2020
389e4e9
Simplify test fore F_GETPIPE_SZ and F_SETPIPE_SZ so no file cleanup i…
rhpvorderman Aug 19, 2020
e601174
Test F_SETPIPE_SZ and F_GETPIPE_SZ on all supported platforms
rhpvorderman Aug 19, 2020
24f061b
Intentionally exclude fcntl from subprocess.__all__ test.
rhpvorderman Aug 19, 2020
58925f3
Properly format the pipesize argument in the subprocess docs.
rhpvorderman Aug 19, 2020
15b759e
Determine default dynamically
rhpvorderman Aug 24, 2020
d4d0886
Add test for pipesize default
rhpvorderman Aug 24, 2020
c129ca8
Add a note to the documentation about only Linux supporting the pipes…
rhpvorderman Aug 24, 2020
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
5 changes: 5 additions & 0 deletions Doc/library/fcntl.rst
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ descriptor.
On Linux(>=3.15), the fcntl module exposes the ``F_OFD_GETLK``, ``F_OFD_SETLK``
and ``F_OFD_SETLKW`` constants, which working with open file description locks.

.. versionchanged:: 3.10
On Linux >= 2.6.11, the fcntl module exposes the ``F_GETPIPE_SZ`` and
``F_SETPIPE_SZ`` constants, which allow to check and modify a pipe's size
respectively.

The module defines the following functions:


Expand Down
10 changes: 9 additions & 1 deletion Doc/library/subprocess.rst
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ functions.
startupinfo=None, creationflags=0, restore_signals=True, \
start_new_session=False, pass_fds=(), \*, group=None, \
extra_groups=None, user=None, umask=-1, \
encoding=None, errors=None, text=None)
encoding=None, errors=None, text=None, pipesize=-1)

Execute a child program in a new process. On POSIX, the class uses
:meth:`os.execvp`-like behavior to execute the child program. On Windows,
Expand Down Expand Up @@ -625,6 +625,14 @@ functions.
* :data:`CREATE_DEFAULT_ERROR_MODE`
* :data:`CREATE_BREAKAWAY_FROM_JOB`

*pipesize* can be used to change the size of the pipe when
:data:`PIPE` is used for *stdin*, *stdout* or *stderr*. The size of the pipe
is only changed on platforms that support this (only Linux at this time of
writing). Other platforms will ignore this parameter.

.. versionadded:: 3.10
The ``pipesize`` parameter was added.

Popen objects are supported as context managers via the :keyword:`with` statement:
on exit, standard file descriptors are closed, and the process is waited for.
::
Expand Down
19 changes: 18 additions & 1 deletion Lib/subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@
import grp
except ImportError:
grp = None
try:
import fcntl
except ImportError:
fcntl = None


__all__ = ["Popen", "PIPE", "STDOUT", "call", "check_call", "getstatusoutput",
"getoutput", "check_output", "run", "CalledProcessError", "DEVNULL",
Expand Down Expand Up @@ -756,7 +761,7 @@ def __init__(self, args, bufsize=-1, executable=None,
startupinfo=None, creationflags=0,
restore_signals=True, start_new_session=False,
pass_fds=(), *, user=None, group=None, extra_groups=None,
encoding=None, errors=None, text=None, umask=-1):
encoding=None, errors=None, text=None, umask=-1, pipesize=-1):
"""Create new Popen instance."""
_cleanup()
# Held while anything is calling waitpid before returncode has been
Expand All @@ -773,6 +778,11 @@ def __init__(self, args, bufsize=-1, executable=None,
if not isinstance(bufsize, int):
raise TypeError("bufsize must be an integer")

if pipesize is None:
pipesize = -1 # Restore default
if not isinstance(pipesize, int):
raise TypeError("pipesize must be an integer")

if _mswindows:
if preexec_fn is not None:
raise ValueError("preexec_fn is not supported on Windows "
Expand All @@ -797,6 +807,7 @@ def __init__(self, args, bufsize=-1, executable=None,
self.returncode = None
self.encoding = encoding
self.errors = errors
self.pipesize = pipesize

# Validate the combinations of text and universal_newlines
if (text is not None and universal_newlines is not None
Expand Down Expand Up @@ -1575,6 +1586,8 @@ def _get_handles(self, stdin, stdout, stderr):
pass
elif stdin == PIPE:
p2cread, p2cwrite = os.pipe()
if self.pipesize > 0 and hasattr(fcntl, "F_SETPIPE_SZ"):
fcntl.fcntl(p2cwrite, fcntl.F_SETPIPE_SZ, self.pipesize)
elif stdin == DEVNULL:
p2cread = self._get_devnull()
elif isinstance(stdin, int):
Expand All @@ -1587,6 +1600,8 @@ def _get_handles(self, stdin, stdout, stderr):
pass
elif stdout == PIPE:
c2pread, c2pwrite = os.pipe()
if self.pipesize > 0 and hasattr(fcntl, "F_SETPIPE_SZ"):
fcntl.fcntl(c2pwrite, fcntl.F_SETPIPE_SZ, self.pipesize)
elif stdout == DEVNULL:
c2pwrite = self._get_devnull()
elif isinstance(stdout, int):
Expand All @@ -1599,6 +1614,8 @@ def _get_handles(self, stdin, stdout, stderr):
pass
elif stderr == PIPE:
errread, errwrite = os.pipe()
if self.pipesize > 0 and hasattr(fcntl, "F_SETPIPE_SZ"):
fcntl.fcntl(errwrite, fcntl.F_SETPIPE_SZ, self.pipesize)
elif stderr == STDOUT:
if c2pwrite != -1:
errwrite = c2pwrite
Expand Down
13 changes: 13 additions & 0 deletions Lib/test/test_fcntl.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,19 @@ def test_fcntl_f_getpath(self):
res = fcntl.fcntl(self.f.fileno(), fcntl.F_GETPATH, bytes(len(expected)))
self.assertEqual(expected, res)

@unittest.skipIf(not (hasattr(fcntl, "F_SETPIPE_SZ") and hasattr(fcntl, "F_GETPIPE_SZ")),
"F_SETPIPE_SZ and F_GETPIPE_SZ are not available on all unix platforms.")
def test_fcntl_f_pipesize(self):
test_pipe_r, test_pipe_w = os.pipe()
# Get the default pipesize with F_GETPIPE_SZ
pipesize_default = fcntl.fcntl(test_pipe_w, fcntl.F_GETPIPE_SZ)
# Multiply the default with 2 to get a new value.
fcntl.fcntl(test_pipe_w, fcntl.F_SETPIPE_SZ, pipesize_default * 2)
self.assertEqual(fcntl.fcntl(test_pipe_w, fcntl.F_GETPIPE_SZ), pipesize_default * 2)
os.close(test_pipe_r)
os.close(test_pipe_w)


def test_main():
run_unittest(TestFcntl)

Expand Down
47 changes: 46 additions & 1 deletion Lib/test/test_subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@
except ImportError:
grp = None

try:
import fcntl
except:
fcntl = None

if support.PGO:
raise unittest.SkipTest("test is not helpful for PGO")

Expand Down Expand Up @@ -661,6 +666,46 @@ def test_stdin_devnull(self):
p.wait()
self.assertEqual(p.stdin, None)

def test_pipesizes(self):
# stdin redirection
pipesize = 16 * 1024
p = subprocess.Popen([sys.executable, "-c",
'import sys; sys.stdin.read(); sys.stdout.write("out"); sys.stderr.write("error!")'],
stdin=subprocess.PIPE,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
pipesize=pipesize)
# We only assert pipe size has changed on platforms that support it.
if sys.platform != "win32" and hasattr(fcntl, "F_GETPIPE_SZ"):
for fifo in [p.stdin, p.stdout, p.stderr]:
self.assertEqual(fcntl.fcntl(fifo.fileno(), fcntl.F_GETPIPE_SZ), pipesize)
# Windows pipe size can be acquired with the GetNamedPipeInfoFunction
# https://docs.microsoft.com/en-us/windows/win32/api/namedpipeapi/nf-namedpipeapi-getnamedpipeinfo
# However, this function is not yet in _winapi.
p.stdin.write(b"pear")
p.stdin.close()
p.wait()

def test_pipesize_default(self):
p = subprocess.Popen([sys.executable, "-c",
'import sys; sys.stdin.read(); sys.stdout.write("out");'
' sys.stderr.write("error!")'],
stdin=subprocess.PIPE,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
pipesize=-1)
# UNIX tests using fcntl
if sys.platform != "win32" and hasattr(fcntl, "F_GETPIPE_SZ"):
fp_r, fp_w = os.pipe()
default_pipesize = fcntl.fcntl(fp_w, fcntl.F_GETPIPE_SZ)
for fifo in [p.stdin, p.stdout, p.stderr]:
self.assertEqual(
fcntl.fcntl(fifo.fileno(), fcntl.F_GETPIPE_SZ), default_pipesize)
# On other platforms we cannot test the pipe size (yet). But above code
# using pipesize=-1 should not crash.
p.stdin.close()
p.wait()

def test_env(self):
newenv = os.environ.copy()
newenv["FRUIT"] = "orange"
Expand Down Expand Up @@ -3503,7 +3548,7 @@ def test_getoutput(self):

def test__all__(self):
"""Ensure that __all__ is populated properly."""
intentionally_excluded = {"list2cmdline", "Handle", "pwd", "grp"}
intentionally_excluded = {"list2cmdline", "Handle", "pwd", "grp", "fcntl"}
exported = set(subprocess.__all__)
possible_exports = set()
import types
Expand Down
1 change: 1 addition & 0 deletions Misc/ACKS
Original file line number Diff line number Diff line change
Expand Up @@ -1805,6 +1805,7 @@ Johannes Vogel
Michael Vogt
Radu Voicilas
Alex Volkov
Ruben Vorderman
Guido Vranken
Martijn Vries
Sjoerd de Vries
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Add F_SETPIPE_SZ and F_GETPIPE_SZ to fcntl module. Allow setting pipesize on
subprocess.Popen.
8 changes: 8 additions & 0 deletions Modules/fcntlmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,14 @@ all_ins(PyObject* m)
if (PyModule_AddIntMacro(m, F_SHLCK)) return -1;
#endif

/* Linux specifics */
#ifdef F_SETPIPE_SZ
if (PyModule_AddIntMacro(m, F_SETPIPE_SZ)) return -1;
#endif
#ifdef F_GETPIPE_SZ
if (PyModule_AddIntMacro(m, F_GETPIPE_SZ)) return -1;
#endif

/* OS X specifics */
#ifdef F_FULLFSYNC
if (PyModule_AddIntMacro(m, F_FULLFSYNC)) return -1;
Expand Down