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

subprocess.Popen(stderr=STDOUT) fails to redirect subprocess stderr to stdout #66470

Closed
4kir4 mannequin opened this issue Aug 25, 2014 · 7 comments
Closed

subprocess.Popen(stderr=STDOUT) fails to redirect subprocess stderr to stdout #66470

4kir4 mannequin opened this issue Aug 25, 2014 · 7 comments
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@4kir4
Copy link
Mannequin

4kir4 mannequin commented Aug 25, 2014

BPO 22274
Nosy @gpshead, @vstinner, @bitdancer, @4kir4, @vadmium, @MojoVampire
Files
  • subprocess-stderr_redirect_with_no_stdout_redirect.diff
  • subprocess-stderr_redirect_with_no_stdout_redirect-2.diff: print -> .write
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2016-05-13.10:00:07.217>
    created_at = <Date 2014-08-25.21:46:01.972>
    labels = ['type-bug', 'library']
    title = 'subprocess.Popen(stderr=STDOUT) fails to redirect subprocess stderr to stdout'
    updated_at = <Date 2016-05-13.10:00:07.215>
    user = 'https://github.com/4kir4'

    bugs.python.org fields:

    activity = <Date 2016-05-13.10:00:07.215>
    actor = 'martin.panter'
    assignee = 'none'
    closed = True
    closed_date = <Date 2016-05-13.10:00:07.217>
    closer = 'martin.panter'
    components = ['Library (Lib)']
    creation = <Date 2014-08-25.21:46:01.972>
    creator = 'akira'
    dependencies = []
    files = ['36472', '42777']
    hgrepos = []
    issue_num = 22274
    keywords = ['patch']
    message_count = 7.0
    messages = ['225898', '225907', '226020', '265136', '265139', '265451', '265460']
    nosy_count = 7.0
    nosy_names = ['gregory.p.smith', 'vstinner', 'r.david.murray', 'akira', 'python-dev', 'martin.panter', 'josh.r']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue22274'
    versions = ['Python 2.7', 'Python 3.5', 'Python 3.6']

    @4kir4
    Copy link
    Mannequin Author

    4kir4 mannequin commented Aug 25, 2014

    The following command should not produce any output but it does:

      $ ./python >/dev/null -c 'import subprocess as S, sys; S.call([sys.executable, "-c", "import sys; print(42, file=sys.stderr)"], stderr=S.STDOUT)'

    Its stdout is redirected to /dev/null. It starts a subprocess with its
    stderr redirected to stdout. See "Redirect subprocess stderr to
    stdout" [1] on Stackoverflow.

    [1] http://stackoverflow.com/questions/11495783/redirect-subprocess-stderr-to-stdout

    I've uploaded a patch that fixes the issue on POSIX.

    Please, run the provided test (in the patch), to see whether the code
    should be fixed on Windows too (it might work as is there).

    No documentation changes are required.

    Please, review.

    @4kir4 4kir4 mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Aug 25, 2014
    @MojoVampire
    Copy link
    Mannequin

    MojoVampire mannequin commented Aug 25, 2014

    Okay, dumb question: Is there a reason the Windows code explicitly initializes c2pwrite in the "stdout not passed" case, while the Linux code leaves it as -1? Windows doesn't look like it would have the problem (because c2pwrite is always set to a non-default value), and it seems like the fix for Linux could just mimic the Windows approach; the code that sets errwrite wouldn't change, but instead of a "pass", when stdout is None, we'd explicitly set it to os.STDOUT_FILENO, and the stderr=subprocess.STDOUT (stdout unset) case would work automatically, and the code would be more similar.

    Haven't explored the negative consequences of that change, if any.

    @4kir4
    Copy link
    Mannequin Author

    4kir4 mannequin commented Aug 28, 2014

    Josh, on Windows, if at least one standard stream is replaced; all three
    hStdInput, hStdOutput, hStdError handles are provided
    (all-or-nothing). On POSIX, standard streams stdin (0), stdout (1),
    stderr (2) are always inherited from the parent. Each stream can be
    manipulated independently. c2pwrite=-1 is different from providing
    c2pwrite=1 (STDOUT_FILENO) explicitly e.g., set_inheritable() call is
    made after the fork() in the latter case.

    My patch leads to dup2(fileno(stdout), STDERR_FILENO) when stdout is
    None and stderr=STDOUT on POSIX i.e., it redirects stderr to the
    inherited stdout (like 2>&1 in the shell). It has no effect otherwise.

    sys.__stdout__ is used so that the call fails sooner without fork() if
    fileno(stdout) is not a valid file descriptor when python initializes
    (daemon, GUI). __stdout__-based solution doesn't support a case when
    fileno(stdout) is changed later in the program e.g., using freopen() on
    some systems.

    @4kir4
    Copy link
    Mannequin Author

    4kir4 mannequin commented May 8, 2016

    Updated the patch to address vadmium's review comments.

    @vadmium
    Copy link
    Member

    vadmium commented May 8, 2016

    I think this patch is pretty good. I will try to commit it in the next few days.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 13, 2016

    New changeset 642933771fa5 by Martin Panter in branch '3.5':
    Issue bpo-22274: Redirect stderr=STDOUT when stdout not redirected, by Akira Li
    https://hg.python.org/cpython/rev/642933771fa5

    New changeset 5979e7aadd59 by Martin Panter in branch 'default':
    Issue bpo-22274: Merge stderr=STDOUT fix from 3.5
    https://hg.python.org/cpython/rev/5979e7aadd59

    New changeset 5f46ecaf8c6e by Martin Panter in branch '2.7':
    Issue bpo-22274: Redirect stderr=STDOUT when stdout not redirected, by Akira Li
    https://hg.python.org/cpython/rev/5f46ecaf8c6e

    @vadmium
    Copy link
    Member

    vadmium commented May 13, 2016

    Thanks for the patch Akira.

    @vadmium vadmium closed this as completed May 13, 2016
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant