classification
Title: Bad interaction between signal and subprocess
Type: behavior Stage:
Components: Library (Lib) Versions: Python 2.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: gregory.p.smith Nosy List: amaury.forgeotdarc, gregory.p.smith, jafo, piro
Priority: normal Keywords: patch

Created on 2008-02-14 15:39 by piro, last changed 2008-08-04 03:23 by gregory.p.smith. This issue is now closed.

Files
File name Uploaded Description Edit
subprocess_signal_bug.py piro, 2008-02-14 15:39 Test case for the bug
subprocess_signal_bug.patch piro, 2008-02-14 15:40 Proposed patch to fix the issue
Messages (6)
msg62392 - (view) Author: Daniele Varrazzo (piro) * Date: 2008-02-14 15:39
During Popen.communicate(), if a signal is caught during the select(),
an unhandled exception is raised, and the output gathered is lost.

This means that a long running or hanged process can't be killed after a
timeout (as shown in the attached example, where the output collected
before the signal is valuable)

The bug happens only when stdout and stderr are not merged and is tested
on linux platform.
msg62394 - (view) Author: Daniele Varrazzo (piro) * Date: 2008-02-14 15:56
Just noticed that in the patch "except select.error:" suffices instead
of "except:"...
msg62400 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-02-14 16:50
First, this patch has a possible bug: if select() fails on the first
iteration, wlist is left undefined, and the next instruction "if
self.stdin in wlist" will fail.

Second, I think that is is not a good idea to simply exit the loop on
the first signal. In your script, select() is interrupted because of
SIGALRM (try removing the call to os.kill), and there may be more data
to read.

A possible solution could be:

try:
    ...select.select()...
except select.error, e:
    if e.args[0] == errno.EINTR:
        continue  # try again
    else:
        raise
else:
    ...exchange data...
msg64100 - (view) Author: Sean Reifschneider (jafo) * (Python committer) Date: 2008-03-19 21:08
Daniele: Please provide an updated version of this patch based on the
feedback below.
msg69322 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2008-07-06 07:18
Fixed as Amaury described in trunk r64756.

still needs backporting to release25-maint.
msg70680 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2008-08-04 03:23
fixed in release25-maint r65475.
History
Date User Action Args
2008-08-04 03:23:59gregory.p.smithsetstatus: open -> closed
resolution: fixed
messages: + msg70680
2008-07-06 07:18:19gregory.p.smithsetmessages: + msg69322
2008-03-19 21:08:53jafosetnosy: + gregory.p.smith, jafo
messages: + msg64100
priority: normal
assignee: gregory.p.smith
keywords: + patch
type: crash -> behavior
2008-02-14 16:50:56amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg62400
2008-02-14 15:56:55pirosetmessages: + msg62394
2008-02-14 15:40:52pirosetcomponents: + Library (Lib)
2008-02-14 15:40:06pirosetfiles: + subprocess_signal_bug.patch
2008-02-14 15:39:01pirocreate