spawnl crash on windows7

[srinim]

BPO	28732
Nosy	@pfmoore, @vstinner, @tjguk, @zware, @eryksun, @zooba

^{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 = 'https://github.com/zooba'
closed_at = <Date 2016-11-20.05:40:20.192>
created_at = <Date 2016-11-18.11:22:35.319>
labels = ['interpreter-core', '3.7', 'OS-windows', 'type-crash']
title = 'spawnl crash on windows7'
updated_at = <Date 2016-11-20.05:40:20.190>
user = 'https://bugs.python.org/srinim'

bugs.python.org fields:

activity = <Date 2016-11-20.05:40:20.190>
actor = 'steve.dower'
assignee = 'steve.dower'
closed = True
closed_date = <Date 2016-11-20.05:40:20.192>
closer = 'steve.dower'
components = ['Interpreter Core', 'Windows']
creation = <Date 2016-11-18.11:22:35.319>
creator = 'srinim'
dependencies = []
files = []
hgrepos = []
issue_num = 28732
keywords = []
message_count = 13.0
messages = ['281095', '281096', '281097', '281111', '281116', '281181', '281239', '281240', '281241', '281242', '281243', '281246', '281248']
nosy_count = 8.0
nosy_names = ['paul.moore', 'vstinner', 'tim.golden', 'python-dev', 'zach.ware', 'eryksun', 'steve.dower', 'srinim']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue28732'
versions = ['Python 3.5', 'Python 3.6', 'Python 3.7']

[srinim]

in window7 (using python 3.4.3,3.5.2) following script crashes

import os
os.spawnl( os.P_NOWAIT, 'C:/Tcl/bin/tclsh.exe' )"

Note: similar issue is already exist https://bugs.python.org/issue8036

[srinim]

small correction (removed " )

import os
os.spawnl( os.P_NOWAIT, 'C:/Tcl/bin/tclsh.exe' )

[vstinner]

Just to make sure: a crash means that the Python process is killed and you get a popup or something like that?

Can you please try to run the script on Python 3.6 using:

python.exe -X faulthandler script.py

It should display the Windows error code at least.

[eryksun]

spawnl is implemented by calling _spawnv 1, which is documented to invoke the invalid parameter handler if argv points to a NULL pointer. It wants the program name, or whatever, as long as argv[0] isn't NULL or an empty string, e.g. os.spawnl(os.P_NOWAIT, 'C:/Tcl/bin/tclsh.exe', 'tclsh').

The invalid parameter handler should be disabled (_Py_BEGIN_SUPPRESS_IPH) when calling _spawnv[e], which in this case would lead to an EINVAL OSError instead of crashing the process.

For example:

    import os, sys
    from ctypes import *

    ucrt = CDLL('ucrtbase')

    @CFUNCTYPE(None, c_wchar_p, c_wchar_p, c_wchar_p, c_int, c_void_p)
    def iph(*args):
        pass

    ucrt._set_thread_local_invalid_parameter_handler(iph)

    >>> os.spawnl(os.P_NOWAIT, sys.executable)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "C:\Program Files\Python35\lib\os.py", line 977, in spawnl
        return spawnv(mode, file, args)
    OSError: [Errno 22] Invalid argument

Also, in this case a descriptive ValueError would be friendlier, given an empty argv or argv[0] that's an empty string -- at least on Windows.

[vstinner]

I like the idea of using _Py_BEGIN_SUPPRESS_IPH, but also the idea of
implementing most obvious checks on arguments. Sadly, I don't have
access to Windows yet to write such patch. Eryk, if you write such
patch, I would be happy to review it ;-) Especially if it includes
unit tests :-D

[zooba]

Looks like a few functions in os module need this. os_execve_impl also doesn't release the GIL at any point, but I don't see why it shouldn't.

I'll try and get to this over the weekend if nobody else comes up with a patch first.

[python-dev]

New changeset 02f416441def by Steve Dower in branch '3.5':
Issue bpo-28732: Fix crash in os.spawnv() with no elements in args
https://hg.python.org/cpython/rev/02f416441def

[eryksun]

A ValueError should also be raised when argv[0] is an empty string.

[python-dev]

New changeset 1a9e4b465497 by Steve Dower in branch '3.6':
Issue bpo-28732: Raise ValueError when os.spawn*() is passed an empty tuple of arguments
https://hg.python.org/cpython/rev/1a9e4b465497

New changeset 75824899f0dd by Steve Dower in branch 'default':
Issue bpo-28732: Raise ValueError when os.spawn*() is passed an empty tuple of arguments
https://hg.python.org/cpython/rev/75824899f0dd

[python-dev]

New changeset e076ace7b0ff by Steve Dower in branch '3.5':
Issue bpo-28732: Raise ValueError when argv[0] is empty.
https://hg.python.org/cpython/rev/e076ace7b0ff

New changeset af78b33704af by Steve Dower in branch '3.6':
Issue bpo-28732: Raise ValueError when argv[0] is empty
https://hg.python.org/cpython/rev/af78b33704af

New changeset fc6f757e53de by Steve Dower in branch 'default':
Issue bpo-28732: Raise ValueError when argv[0] is empty
https://hg.python.org/cpython/rev/fc6f757e53de

[zooba]

ValueError should also be raised when argv[0] is an empty string.

Added that too.

Python 3.5 is missing the tests for these functions completely, so I only added those to 3.6 and later. Also the original issue was already resolved in 3.6, but I tidied up a few other functions that were missing proper handling.

[python-dev]

New changeset 2e1fb851dfb4 by Steve Dower in branch '3.6':
Issue bpo-28732: Adds new errors to spawnv emulation for platforms that only have fork and execv
https://hg.python.org/cpython/rev/2e1fb851dfb4

New changeset ac6de11fbd50 by Steve Dower in branch 'default':
Issue bpo-28732: Adds new errors to spawnv emulation for platforms that only have fork and execv
https://hg.python.org/cpython/rev/ac6de11fbd50

[zooba]

Buildbots seem to be happy now so I'm closing the issue.

Feel free to reopen if anyone spots anything in commit review.