[issue31446] _winapi.CreateProcess (used by subprocess) is not thread-safe

2017-09-13 Thread Evan Andrews

New submission from Evan Andrews:

The method used for spawning subprocesses on Windows is not thread-safe under 
certain circumstances. The following example demonstrates how this manifests:

>>> import threading
>>> import subprocess
>>> for i in range(100):
... threading.Thread(
... target=subprocess.Popen,
... args=('ping localhost',),
... kwargs={'stdout': subprocess.DEVNULL},
... ).start()
...
Exception in thread Thread-1202:
Traceback (most recent call last):
  File "C:\Program Files\Python36\lib\threading.py", line 916, in 
_bootstrap_inner
self.run()
  File "C:\Program Files\Python36\lib\threading.py", line 864, in run
self._target(*self._args, **self._kwargs)
  File "C:\Program Files\Python36\lib\subprocess.py", line 707, in __init__
restore_signals, start_new_session)
  File "C:\Program Files\Python36\lib\subprocess.py", line 990, in 
_execute_child
startupinfo)
ValueError: embedded null character

Exception in thread Thread-1206:
Traceback (most recent call last):
  File "C:\Program Files\Python36\lib\threading.py", line 916, in 
_bootstrap_inner
self.run()
  File "C:\Program Files\Python36\lib\threading.py", line 864, in run
self._target(*self._args, **self._kwargs)
  File "C:\Program Files\Python36\lib\subprocess.py", line 707, in __init__
restore_signals, start_new_session)
  File "C:\Program Files\Python36\lib\subprocess.py", line 990, in 
_execute_child
startupinfo)
ValueError: embedded null character

>>>

subprocess.Popen calls down to _winapi.CreateProcess, which calls 
CreateProcessW. When args is passed as a fixed string, the result of the 
argument conversion is attached to the object and shared by future calls into C 
code. However, the documentation for CreateProcess states:

"The Unicode version of this function, CreateProcessW, can modify the contents 
of this string. Therefore, this parameter cannot be a pointer to read-only 
memory (such as a const variable or a literal string). If this parameter is a 
constant string, the function may cause an access violation." (Source: 
https://msdn.microsoft.com/en-us/library/windows/desktop/ms682425(v=vs.85).aspx)

It appears CreateProcessW is briefly inserting null characters into the buffer, 
causing errors if that buffer is used elsewhere before it is changed back.

The call to CreateProcessW using the shared buffer can be seen here: 
https://github.com/python/cpython/blob/b8f4163da30e16c7cd58fe04f4b17e38d53cd57e/Modules/_winapi.c#L879

Note that this error does not occur when passing args as a list, as 
subprocess.list2cmdline creates a new (though identical) string for each 
invocation.

One potential solution is to allocate a copy of command_line (the shared 
buffer) instead of using the original.

--
components: Library (Lib)
messages: 302045
nosy: evan_
priority: normal
severity: normal
status: open
title: _winapi.CreateProcess (used by subprocess) is not thread-safe
versions: Python 3.3, Python 3.4, Python 3.5, Python 3.6, Python 3.7

___
Python tracker 
<https://bugs.python.org/issue31446>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue28595] shlex.shlex should not augment wordchars

2017-06-10 Thread Evan Andrews

Evan Andrews added the comment:

Thanks, Vinay. I understand you're busy, and I'm in no particular rush to have 
this looked at, so feel free to come back to it when you have more time. I've 
submitted the changes as a PR.

I also have an additional concern - even with this change, there is no way to 
tell whether a token should be considered special or text:

>>> import shlex
>>> def split(line):
... s = shlex.shlex(line, posix=True, punctuation_chars=True)
... s.whitespace_split = True
... return list(s)
...
>>> split('a&&b')
['a', '&&', 'b']
>>> split('a "&&" b')
['a', '&&', 'b']

I feel this can be addressed after this as a separate issue but wanted to 
mention it now so you're aware.

--

___
Python tracker 
<http://bugs.python.org/issue28595>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue28595] shlex.shlex should not augment wordchars

2017-06-09 Thread Evan Andrews

Changes by Evan Andrews :


--
pull_requests: +2134

___
Python tracker 
<http://bugs.python.org/issue28595>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue28595] shlex.shlex should not augment wordchars

2017-05-13 Thread Evan Andrews

Evan Andrews added the comment:

Do I need to create a pull request for this?

--

___
Python tracker 
<http://bugs.python.org/issue28595>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue28595] shlex.shlex should not augment wordchars

2017-01-20 Thread Evan Andrews

Evan Andrews added the comment:

Unfortunately shlex.shlex's defaults are probably going to remain that way for 
a long time in order to avoid breaking backwards compatibility. Presumably 
shlex.split was added so you didn't have to remember to set posix and 
whitespace_split yourself.

The particular problem I'm addressing in this issue is that the new 
punctuation_chars argument doesn't currently work with whitespace_split.

>>> def split(text, ws=False, pc=False):
... s = shlex.shlex(text, posix=True, punctuation_chars=pc)
... s.whitespace_split = ws
... return list(s)
...
>>> split('foo,bar>baz')
['foo', ',', 'bar', '>', 'baz']
>>> split('foo,bar>baz', ws=True)
['foo,bar>baz']
>>> split('foo,bar>baz', pc=True)
['foo', ',', 'bar', '>', 'baz']
>>> split('foo,bar>baz', ws=True, pc=True)
['foo,bar>baz']

With my patch, the last example outputs ['foo,bar', '>', 'baz'].

Before the release of 3.6 I was arguing that punctuation_chars should not 
attempt to augment wordchars at all, since the idea of wordchars is inherently 
incorrect as you point out. Now I think it's too late to change, hence my patch 
treats this as a new feature in 3.7.

--

___
Python tracker 
<http://bugs.python.org/issue28595>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue28595] shlex.shlex should not augment wordchars

2017-01-19 Thread Evan Andrews

Evan Andrews added the comment:

Attaching an updated patch now that the two linked issues are resolved.

--
Added file: http://bugs.python.org/file46335/shlex2.diff

___
Python tracker 
<http://bugs.python.org/issue28595>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com