[issue6973] subprocess.Popen.send_signal doesn't check whether the process has terminated

[Roundup Robot]

Roundup Robot added the comment:

New changeset 6bd3c8623bb2 by Gregory P. Smith in branch '3.4':
Fix issue #6973: When we know a subprocess.Popen process has died, do
https://hg.python.org/cpython/rev/6bd3c8623bb2

New changeset bc907c76f054 by Gregory P. Smith in branch '3.5':
Fix issue #6973: When we know a subprocess.Popen process has died, do
https://hg.python.org/cpython/rev/bc907c76f054

New changeset e51c46d12ec1 by Gregory P. Smith in branch 'default':
Fix issue #6973: When we know a subprocess.Popen process has died, do
https://hg.python.org/cpython/rev/e51c46d12ec1

--
nosy: +python-dev

___
Python tracker 

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

[issue6973] subprocess.Popen.send_signal doesn't check whether the process has terminated

[Gregory P. Smith]

Gregory P. Smith added the comment:

I chose to go with the "there should not be an error" approach similar to how 
Python deals with multiple file closes.  Both are technically programming 
errors but the point of Python is make life easier.

Reasoning?  I don't want someone to ever think they could depend on getting an 
exception from send_signal/terminate/kill in this "process has already died and 
someone has called wait or poll to get its return code" situation as that is 
unreliable and potentially impacts other innocent processes.

--
resolution:  -> fixed
status: open -> closed

___
Python tracker 

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

[issue6973] subprocess.Popen.send_signal doesn't check whether the process has terminated

[Gregory P. Smith]

Changes by Gregory P. Smith :


--
assignee:  -> gregory.p.smith
nosy: +gregory.p.smith

___
Python tracker 

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

[issue6973] subprocess.Popen.send_signal doesn't check whether the process has terminated

[Martin Panter]

Martin Panter added the comment:

It seems to me that raising OSError isn’t quite right. Either the error is more 
of an internal programmer error, like how writing to a closed file object 
raises ValueError, or there should not be an error, like when the process has 
terminated but has not been reaped, or when closing an already-closed file.

--
components: +Interpreter Core -Library (Lib)
nosy: +vadmium

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6973
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6973] subprocess.Popen.send_signal doesn't check whether the process has terminated

[Martin Panter]

Changes by Martin Panter vadmium...@gmail.com:


--
components: +Library (Lib) -Interpreter Core

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6973
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6973] subprocess.Popen.send_signal doesn't check whether the process has terminated

[Tshepang Lekhonkhobe]

Changes by Tshepang Lekhonkhobe tshep...@gmail.com:


--
nosy: +tshepang
versions:  -Python 3.2

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6973
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6973] subprocess.Popen.send_signal doesn't check whether the process has terminated

[Tshepang Lekhonkhobe]

Changes by Tshepang Lekhonkhobe tshep...@gmail.com:


--
versions: +Python 3.5 -Python 3.3

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6973
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6973] subprocess.Popen.send_signal doesn't check whether the process has terminated

[Marco Buccini]

Changes by Marco Buccini marcu...@gmail.com:


--
nosy:  -markon

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6973
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6973] subprocess.Popen.send_signal doesn't check whether the process has terminated

[Ezio Melotti]

Changes by Ezio Melotti ezio.melo...@gmail.com:


--
versions: +Python 2.7, Python 3.2, Python 3.3, Python 3.4 -Python 2.6, Python 
3.1

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6973
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6973] subprocess.Popen.send_signal doesn't check whether the process has terminated

[Giampaolo Rodola']

Giampaolo Rodola' g.rod...@gmail.com added the comment:

I totally agree this should be fixed (there was a similar issue for psutil: 
http://code.google.com/p/psutil/issues/detail?id=58) but communicate() and 
wait() should be affected in the same manner.

Probably it would make sense to add an is_running() method and decorate 
send_signal(), kill(), communicate() and wait() methods with a function which 
makes sure that the process is still running, otherwise raises an exception 
instead.

Maybe it would also makes sense to provide a brand new NoSuchProcess exception 
class.

--
nosy: +astrand, brian.curtin, giampaolo.rodola

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6973
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6973] subprocess.Popen.send_signal doesn't check whether the process has terminated

[Ezio Melotti]

Changes by Ezio Melotti ezio.melo...@gmail.com:


--
nosy: +ezio.melotti
priority:  - normal
stage:  - needs patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6973
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6973] subprocess.Popen.send_signal doesn't check whether the process has terminated

[Marco Buccini]

Marco Buccini marcu...@gmail.com added the comment:

I agree with Milko. 

However, I think Popen.send_signal should poll() before sending any
signals, without resetting any variables to zero (or None).
In this way, if you poll() before sending a signal, if the return code
is None, the child is still running. If the poll return code is
different than None, the child has been terminated, so you must not send
anything to the child process.

In this way, instead of polling before sending signals to check if the
child has been terminated, we can make Python do the work for us.

I've provided a patch. All the tests pass.
A new test has been added: test_terminate_already_terminated_child.
This method asserts `terminate()` (through `send_signal`) raises an
OSError exception. 

However, even though you try the old version of subprocess.py all the
tests pass. This happens because the method `send_signal` calls
os.kill() (or Terminate for Windows systems) that can raise an OSError
exception if the process to which it send the signal does not exist. 
`send_signal` has been fixed to be a safer method, so that it raises an
OSError exception if the child has been terminated.

--
keywords: +patch
nosy: +markon
Added file: http://bugs.python.org/file15135/subprocess.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6973
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6973] subprocess.Popen.send_signal doesn't check whether the process has terminated

[Marco Buccini]

Changes by Marco Buccini marcu...@gmail.com:


Added file: http://bugs.python.org/file15136/test_subprocess.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6973
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6973] subprocess.Popen.send_signal doesn't check whether the process has terminated

[Marco Buccini]

Changes by Marco Buccini marcu...@gmail.com:


Removed file: http://bugs.python.org/file15136/test_subprocess.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6973
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6973] subprocess.Popen.send_signal doesn't check whether the process has terminated

[Marco Buccini]

Changes by Marco Buccini marcu...@gmail.com:


Added file: http://bugs.python.org/file15137/test_subprocess.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6973
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6973] subprocess.Popen.send_signal doesn't check whether the process has terminated

[Marco Buccini]

Changes by Marco Buccini marcu...@gmail.com:


Removed file: http://bugs.python.org/file15135/subprocess.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6973
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6973] subprocess.Popen.send_signal doesn't check whether the process has terminated

[Marco Buccini]

Changes by Marco Buccini marcu...@gmail.com:


Added file: http://bugs.python.org/file15141/subprocess.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6973
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6973] subprocess.Popen.send_signal doesn't check whether the process has terminated

[Marco Buccini]

Changes by Marco Buccini marcu...@gmail.com:


Removed file: http://bugs.python.org/file15141/subprocess.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6973
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6973] subprocess.Popen.send_signal doesn't check whether the process has terminated

[Marco Buccini]

Changes by Marco Buccini marcu...@gmail.com:


Removed file: http://bugs.python.org/file15137/test_subprocess.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6973
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6973] subprocess.Popen.send_signal doesn't check whether the process has terminated

[Marco Buccini]

Changes by Marco Buccini marcu...@gmail.com:


Added file: http://bugs.python.org/file15142/test_subprocess.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6973
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6973] subprocess.Popen.send_signal doesn't check whether the process has terminated

[Marco Buccini]

Changes by Marco Buccini marcu...@gmail.com:


Added file: http://bugs.python.org/file15143/subprocess.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6973
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue6973] subprocess.Popen.send_signal doesn't check whether the process has terminated

[Milko Krachounov]

New submission from Milko Krachounov pyt...@milko.3mhz.net:

When subprocess.Popen.send_signal is called, it simply calls
os.kill(self.pid, ...) without checking whether the child has already
terminated. If the child has been terminated, and Popen.wait() or
Popen.poll() have been called, a process with PID self.pid no longer
exists, and what's worse, could belong to a totally unrelated process.

A better behavior would be to raise an exception when self.returncode is
not None. Alternatively, self.pid might be set to None after the process
has been terminated, as it is no longer meaningful. Or to another value
that would be invalid pid and invalid argument to os.kill and os.wait,
but unlike None would still evaluate to True.

--
components: Library (Lib)
messages: 93022
nosy: milko.krachounov
severity: normal
status: open
title: subprocess.Popen.send_signal doesn't check whether the process has 
terminated
type: behavior
versions: Python 2.6, Python 3.1

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6973
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com