Skip to content
Closed
Changes from 1 commit
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
Prev Previous commit
Merge branch 'main' into gh_42127
  • Loading branch information
terryjreedy authored Jun 24, 2023
commit 819ce88c2ae12a5feda659d873ff273295d3eb1c
4 changes: 2 additions & 2 deletions Doc/library/subprocess.rst
Original file line number Diff line number Diff line change
Expand Up @@ -282,8 +282,8 @@ default values. The arguments that are most commonly needed are:
*stdin*, *stdout* and *stderr* specify the executed program's standard input,
standard output and standard error file handles, respectively. Valid values
are :data:`PIPE`, :data:`DEVNULL`, an existing file descriptor (a positive
integer), an existing file object with a valid file descriptor, and ``None``.
:data:`PIPE` indicates that a new pipe to the child should be created.
integer), an existing :term:`file object` with a valid file descriptor, and
``None``. :data:`PIPE` indicates that a new pipe to the child should be created.
:data:`DEVNULL` indicates that the special file :data:`os.devnull` will
be used. With the default settings of ``None``, no redirection will occur;
the child's file handles will be inherited from the parent for console
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wording seems to be Windows specific. There is no such thing as a "console" or "console application" or "window manager" or "graphical shell" having anything to do with file descriptors on POSIX systems.

But I think all this needs is some disambiguation wording:

Turn the ; into a . and start a new Windows specific paragraph:

"On Windows, the child's file handles will be inherited from the parent if the child can connect to the console. Otherwise ..."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description for Windows needs to be corrected and fleshed out.

If the value is None, then no redirection occurs. On POSIX, the child inherits a standard file that is not redirected only if its file descriptor (i.e. 0, 1, or 2) is inheritable. The latter is irrespective of the value of the close_fds parameter.

On Windows, if one or more of the standard files is redirected, then, for each valid standard file that is not redirected, the child inherits a duplicate of the current handle. If the current handle value is None, a handle for a new pipe is substituted. If none of the standard files is redirected and close_fds is false, then the child inherits each standard handle only if it is inheritable. If none of the standard files is redirected, and close_fds is true, the startupinfo handle list is not used explicitly, and the child is a console application that inherits the current console session, then Windows implicitly duplicates the standard handles to the child. Otherwise, the initial standard handle values in the child will be default values. If the child is a console application that is spawned with a new console session, the default standard handles reference console I/O. Else the default values are None.


Beyond just documenting the current behavior in regard to redirecting the standard files, what I'd like to see is new behavior that's more consistent between POSIX and Windows, and behavior that's easier to explain and understand on Windows.

To better match POSIX, the Windows implementation could directly use the current value of a standard handle when it isn't redirected, instead of duplicating an inheritable handle. The pipe fallback for when the current value is None is completely unnecessary behavior. For example, for stdin:

            if stdin is None:
                p2cread = _winapi.GetStdHandle(_winapi.STD_INPUT_HANDLE)
                if p2cread is not None:
                    try:
                        if not os.get_handle_inheritable(p2cread):
                            p2cread = None
                    except OSError:
                        p2cread = None
            else:
                if stdin == PIPE:
                    # FIXME: _winapi.CreatePipe(None, max(self.pipesize, 0))
                    p2cread, p2cwrite = _winapi.CreatePipe(None, 0)
                    p2cread, p2cwrite = Handle(p2cread), Handle(p2cwrite)
                elif stdin == DEVNULL:
                    p2cread = msvcrt.get_osfhandle(self._get_devnull())
                elif isinstance(stdin, int):
                    p2cread = msvcrt.get_osfhandle(stdin)
                else:
                    p2cread = msvcrt.get_osfhandle(stdin.fileno())
                p2cread = self._make_inheritable(p2cread)

If the handle list is used in _exec_child(), then 0 and None values would have to be filtered out via handle_list[:] = [int(h) for h in handle_list if h].

There's also a behavior difference if none of the standard files is redirected when close_fds is true. It could be implemented to always inherit each standard file if the handle is inheritable, as is implemented on POSIX. This entails removing the check in _get_handles() for when none of the standard files is redirected. Instead, when none of the standard files is redirected, it should return the current values in p2cread, c2pwrite, and errwrite. With this change, Popen() would always use STARTF_USESTDHANDLES.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of all this, I would have added a separate new paragraph, after the text "into the same file handle as for stdout" in the original, which explicitly noted that it was Windows specific. Something like:

On Windows, the default settings result in the subprocess output going to your code's standard output. If your code is running as a GUI application (which has no console, and hence no standard output) then the OS will automatically open a console window to display the subprocess output.

This is deliberately a little vague, and only approximates the truth. That's because the actual underlying behaviour is subtle, but it's (IMO) not the job of the Python documentation to explain the nuances of Windows (and the whole console vs GUI application thing).

Honestly, though, I'm not sure this warrants a change in any case. The original issue seems to be arguing for completeness and precision, but doesn't offer any motivating real-world situation where knowing this level of detail is important. I suspect that's either because there isn't one, or there is, but anyone who encounters it has access to far better (more detailed and complete) sources of information1 than we can offer here.

Footnotes

  1. Such as MSDN.

Expand Down
You are viewing a condensed version of this merge commit. You can view the full changes here.