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

os.execv executes in background on Windows #63323

Open
techtonik mannequin opened this issue Sep 29, 2013 · 15 comments
Open

os.execv executes in background on Windows #63323

techtonik mannequin opened this issue Sep 29, 2013 · 15 comments
Labels
3.10 only security fixes extension-modules C modules in the Modules dir OS-windows stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@techtonik
Copy link
Mannequin

techtonik mannequin commented Sep 29, 2013

BPO 19124
Nosy @terryjreedy, @pfmoore, @tjguk, @jwilk, @zware, @eryksun, @zooba
Files
  • testexecvchild.py
  • 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 = None
    created_at = <Date 2013-09-29.10:46:42.091>
    labels = ['extension-modules', 'type-bug', 'library', '3.10', 'OS-windows']
    title = 'os.execv executes in background on Windows'
    updated_at = <Date 2021-03-30.18:18:06.766>
    user = 'https://bugs.python.org/techtonik'

    bugs.python.org fields:

    activity = <Date 2021-03-30.18:18:06.766>
    actor = 'eryksun'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Extension Modules', 'Library (Lib)', 'Windows']
    creation = <Date 2013-09-29.10:46:42.091>
    creator = 'techtonik'
    dependencies = []
    files = ['31905']
    hgrepos = []
    issue_num = 19124
    keywords = []
    message_count = 15.0
    messages = ['198578', '198579', '198599', '198613', '198616', '198627', '198628', '198638', '198974', '316754', '389227', '389737', '389802', '389838', '389843']
    nosy_count = 9.0
    nosy_names = ['terry.reedy', 'paul.moore', 'techtonik', 'tim.golden', 'jwilk', 'docs@python', 'zach.ware', 'eryksun', 'steve.dower']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = 'needs patch'
    status = 'open'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue19124'
    versions = ['Python 3.10']

    @techtonik
    Copy link
    Mannequin Author

    techtonik mannequin commented Sep 29, 2013

    os.execv() starts process in background on Windows. Because it inherits stdin/stdout handlers from the process that launched Python interpreter, this becomes a source of numerous weird bugs, from polluting the stdout stream of parent to completely blocking its input.

    Example session on Windows. Open cmd.exe and run attached testexecvchild.py. It starts child process with execv(). Child pauses for 2 seconds during which I type 'echo "Hello"' and hit Enter.

    With Python 3 is pollutes parent output after 3 seconds:
        >python testexecvchild.py
        
        >echo "Hello"
        "Hello"
        
        >Traceback (most recent call last):
        File "testexecvchild.py", line 7, in <module>
        raw_input('xxx')
        NameError: name 'raw_input' is not defined

    With Python 2 the stdin of cmd.exe is blocked:
    >py testexecvchild.py

    \>echo "Hello"
    "Hello"
    
    \>xxxecho "Hello"
    "Hello"
    
    \>echo "Hello"
      testexecvchild.py
      passed
    echo "Hello"
    "Hello"
    

    The same behavior on Linux:

        $ python testexecvchild.py
        echo "Hello"
        xxx  testexecvchild.py
          passed

    @techtonik techtonik mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Sep 29, 2013
    @techtonik
    Copy link
    Mannequin Author

    techtonik mannequin commented Sep 29, 2013

    s/same behavior/same command/

    @sbt
    Copy link
    Mannequin

    sbt mannequin commented Sep 29, 2013

    As I wrote in http://bugs.python.org/issue19066, on Windows execv() is equivalent to

        os.spawnv(os.P_NOWAIT, ...)
        os._exit(0)

    This means that control is returned to cmd when the child process *starts* (and afterwards you have cmd and the child connected to the same console).

    On Unix control is returned to the shell only once the child process *ends*.

    Although it might be less memory efficient, you would actually get something closer to Unix behaviour by replacing os.execv(...) with

        sts = os.spawnv(os.P_WAIT, ...)
        _exit(sts)

    or

        sts = subprocess.call(...)
        _exit(sts)

    This is why I said that execv() is useless on Windows and that you should just use subprocess instead.

    @techtonik
    Copy link
    Mannequin Author

    techtonik mannequin commented Sep 29, 2013

    On Sun, Sep 29, 2013 at 6:39 PM, Richard Oudkerk <[email protected]> wrote:

    Richard Oudkerk added the comment:

    As I wrote in http://bugs.python.org/issue19066, on Windows execv() is equivalent to

    os.spawnv(os.P_NOWAIT, ...)
    os.\_exit(0)
    

    Where did you get that info? MSDN is silent about that.
    http://msdn.microsoft.com/en-us/library/886kc0as(v=vs.90).aspx

    This means that control is returned to cmd when the child process *starts* (and afterwards you have cmd and the child connected to the same console).

    On Unix control is returned to the shell only once the child process *ends*.

    That was my conclusion also.

    Although it might be less memory efficient, you would actually get something closer to Unix behaviour by replacing os.execv(...) with

    sts = os.spawnv(os.P_WAIT, ...)
    \_exit(sts)
    

    or

    sts = subprocess.call(...)
    \_exit(sts)
    

    This is why I said that execv() is useless on Windows and that you should just use subprocess instead.

    The problem is not in what I should or should not use. The problem
    that existing scripts that work on Unix and use os.execv() to launch
    interactive scripts, on Windows behave absolutely weird and unusable
    behavior. I previously experienced this with SCons, but couldn't get
    the reason. Now I experience this with basic Android development tools
    and dug down to this. It is clearly a big mess from this side of
    Windows.

    @sbt
    Copy link
    Mannequin

    sbt mannequin commented Sep 29, 2013

    Where did you get that info? MSDN is silent about that.
    http://msdn.microsoft.com/en-us/library/886kc0as(v=vs.90).aspx

    Reading the source code for the C runtime included with Visual Studio.

    The problem is not in what I should or should not use. The problem
    that existing scripts that work on Unix and use os.execv() to launch
    interactive scripts, on Windows behave absolutely weird and unusable
    behavior. I previously experienced this with SCons, but couldn't get
    the reason. Now I experience this with basic Android development tools
    and dug down to this. It is clearly a big mess from this side of
    Windows.

    As said before (more than once), os.exec*() is useless on Windows: just use subprocess.

    @sbt sbt mannequin closed this as completed Sep 29, 2013
    @techtonik
    Copy link
    Mannequin Author

    techtonik mannequin commented Sep 29, 2013

    On Sun, Sep 29, 2013 at 8:53 PM, Richard Oudkerk <[email protected]> wrote:

    Richard Oudkerk added the comment:

    > Where did you get that info? MSDN is silent about that.
    > http://msdn.microsoft.com/en-us/library/886kc0as(v=vs.90).aspx

    Reading the source code for the C runtime included with Visual Studio.

    Visual Studio 10+ ? Is it available somewhere for a reference?

    > The problem is not in what I should or should not use. The problem
    > that existing scripts that work on Unix and use os.execv() to launch
    > interactive scripts, on Windows behave absolutely weird and unusable
    > behavior. I previously experienced this with SCons, but couldn't get
    > the reason. Now I experience this with basic Android development tools
    > and dug down to this. It is clearly a big mess from this side of
    > Windows.

    As said before (more than once), os.exec*() is useless on Windows: just use subprocess.

    I value your expert opinion, but to increase the bus factor, I can not
    leave it without asking for reasons.

    Have you tried to run examples provided by MSDN - do they exhibit the
    same behavior as Python script I attached earlier and described in the
    first message?

    @techtonik
    Copy link
    Mannequin Author

    techtonik mannequin commented Sep 29, 2013

    I can't use subprocess. These are official "business suite" scripts for Android development from Google.

    @techtonik techtonik mannequin reopened this Sep 29, 2013
    @sbt
    Copy link
    Mannequin

    sbt mannequin commented Sep 29, 2013

    @terryjreedy
    Copy link
    Member

    In general, os module functions lightly wrap the corresponding operating system calls. It does not mask differences between OSes, or between versions of an OS. So the unix-windows difference is not a bug and the behavior will not change.

    The difference could, however, be described succinctly in the doc. In the first paragraph, after "On Unix, the new executable is loaded into the current process, and will have the same process id as the caller.", we could add something like

    On Windows, the new process is executed in the background but can send output to a console if the original process was started in a console.

    This has to be worded carefully because the process could instead have been started from an icon, including in Start Menu or Windows Explorer.

    @terryjreedy terryjreedy added docs Documentation in the Doc dir and removed stdlib Python modules in the Lib dir labels Oct 5, 2013
    @eryksun
    Copy link
    Contributor

    eryksun commented May 16, 2018

    The exec functions provided by the Windows C runtime really are practically useless, due to creating an orphaned process, disrupting synchronous operation, and returning the wrong status code. It might be more useful for Python 3.7.x to implement an internal win32_execv[e] function that calls _wspawnv[e] with _P_NOWAIT mode. Assign the child process to a silent-breakaway, kill-on-close Job. Wait for it to end, and exit with the child's status code.

    @eryksun eryksun added 3.7 (EOL) end of life 3.8 (EOL) end of life OS-windows labels May 16, 2018
    @eryksun
    Copy link
    Contributor

    eryksun commented Mar 21, 2021

    On Windows, the new process is executed in the background but
    can send output to a console if the original process was
    started in a console.

    C execve is a mess for console applications. The child process is competing for console input with an ancestor process in the console session, which is typically a CLI shell such as CMD or PowerShell. This is dysfunctional. It's worth either fixing or deprecating. It's not worth documenting since it's way off spec compared to exec() in POSIX systems. exec() is supposed to overlay the current process with a new image, not terminate the current process. That cannot be implemented in Windows, but we could do our best to emulate it.

    The implementation could leave the current Python process running in much the same way as a launcher functions, i.e. wait for the child process to exit and proxy its exit status, and set the child process in a kill-on-close job object.

    It would be better at this point to use subprocess.Popen() to implement os.exec*, considering it can work concurrently with itself, without race conditions involving inheritable handles. nt.spawnve, nt.waitpid(), and nt.system() could also be implemented with subprocess.

    @eryksun eryksun added extension-modules C modules in the Modules dir stdlib Python modules in the Lib dir 3.10 only security fixes and removed docs Documentation in the Doc dir 3.7 (EOL) end of life 3.8 (EOL) end of life labels Mar 21, 2021
    @zooba
    Copy link
    Member

    zooba commented Mar 29, 2021

    nt.spawnve, nt.waitpid(), and nt.system() could also be implemented with subprocess.

    I like the idea, but we shouldn't invert the dependencies like that. nt/os is a lower-level library, and should provide its own implementation (that perhaps subprocess could then use).

    @eryksun
    Copy link
    Contributor

    eryksun commented Mar 30, 2021

    Steve, what do you think about os.exec*()? Should it be emulated better or deprecated?

    We can't hide the reality that it's a new process with a different process ID and parent process ID -- against what POSIX requires. But emulating exec() is probably good enough for most cases, especially if code expects to work in Windows.

    I like the idea, but we shouldn't invert the dependencies like that.
    nt/os is a lower-level library, and should provide its own
    implementation (that perhaps subprocess could then use).

    The precedent I had in mind is os.popen(), which uses subprocess.

    There wouldn't be much need to implement system() and spawnv[e] if ucrt used PROC_THREAD_ATTRIBUTE_HANDLE_LIST in the common spawn code (exec\spawnv.cpp). Any chance that's in development or planned?

    If Python implements its own system() and spawnve functions, support for inheritable file descriptors [1] would probably have to be dropped. That's not a great loss, IMO, but I'm sure someone will be unhappy about it. The problem is that the flags for an open fd (e.g. FDEV, FPIPE, FAPPEND, FTEXT, FNOINHERIT), which the CRT copies to the STARTUPINFO.lpReserved2 buffer, are not public data. In particular, the FAPPEND flag for an O_APPEND open can't be determined or inferred.

    ---

    [1] Making an fd inheritable currently requires two steps in Windows Python: fd2 = os.dup(fd); os.set_inheritable(fd2, True). os.dup(fd) creates a duplicate fd that's not flagged FNOINHERIT, but the underlying OS handle isn't inheritable. os.set_inheritable(fd, True) makes the OS handle inheritable, but it can't remove the FNOINHERIT fd flag.

    @zooba
    Copy link
    Member

    zooba commented Mar 30, 2021

    emulating exec() is probably good enough for most cases, especially if code expects to work in Windows.

    I think good-enough emulation is fine, but we should update the docs to clarify that non-Unix platforms may create a subprocess with a new PID, and callers should avoid relying on POSIX semantics if they may run on non-compliant platforms.

    If Python implements its own system() and spawnve functions, support for inheritable file descriptors [1] would probably have to be dropped

    I don't see how its behaviour would change at all, unless we're going well out of our way to override the CRT. Which I wouldn't want to see us do.

    So let's at least fix the current issue of having the child process break away immediately, by waiting for it and then exiting. Even if that's just swapping the execv call for spawnv(_P_WAIT) and then exiting, that's fine by me (all the CRT calls go through the same path, so they'll launch it consistently regardless of mode).

    @eryksun
    Copy link
    Contributor

    eryksun commented Mar 30, 2021

    Even if that's just swapping the execv call for spawnv(_P_WAIT)
    and then exiting, that's fine by me

    Maybe call SuspendThread() on other threads, at least the ones that can be enumerated with the threading module.

    I don't see how its behaviour would change at all, unless we're going
    well out of our way to override the CRT. Which I wouldn't want to see
    us do.

    The suggestion was for Python to implement system() and spawnve using subprocess or _winapi. That would eliminate the problem of leaked handles when subprocess.Popen() is called concurrently with os.system() and os.spawn*(). However, inheritance of file descriptors cannot be reasonably implemented in that case, not without accessing private CRT data (i.e. the internal flags of each file descriptor).

    If os.spawn*() has to continue supporting inheritance of file descriptors, then the idea is probably a non-starter. All we can do is hope that the CRT's common spawn code will eventually use PROC_THREAD_ATTRIBUTE_HANDLE_LIST. This feature only protects against leaked handles (particularly important for pipes) if it's used by all concurrent CreateProcessW() calls that inherit handles.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.10 only security fixes extension-modules C modules in the Modules dir OS-windows stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants