[issue1068268] subprocess is not EINTR-safe

[Nir Soffer]

Changes by Nir Soffer :


--
nosy: +nirs

___
Python tracker 

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

[issue1068268] subprocess is not EINTR-safe

[David Oxley]

David Oxley  added the comment:

Another instance of a blocking function within subprocess not being protected 
against EINTR 

Python 2.6.4, subprocess.py, Popen function, line 1115:
data = os.read(errpipe_read, 1048576) # Exceptions limited to 1 MB

If a signal arrives while blocked in this read, the EINTR/OSError is passed up 
to whatever called subprocess.Popen. Retrying the Popen doesn't help because 
the child process may already have started but the caller has no way to know 
this nor does the caller have any control over the child process.

===

In the example code, the first subprocess.Popen starts without issue but while 
in the second Popen call, p1's SIGCHLD is received by the parent. 
p2 is never set, but the second copy of /bin/date starts running anyway.

The "preexec_fn = lambda : time.sleep(2)" in the second Popen is a little 
contrived but serves to guarantee that the SIGCHLD will break the Popen for the 
purposes of the demonstration. I have seen this failure mode when using vanilla 
Popen calls although you have to be lucky/unlucky to see it.



This is in python 2.6.4:
> md5sum subprocess.py
2ac8cefe8301eadce87630b230d6fff2  subprocess.py



I expect the fix is equivalent to cmiller's trunk-diff-unified.txt

--
nosy: +mathmodave
Added file: http://bugs.python.org/file15869/bugtest.py

___
Python tracker 

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

[issue1068268] subprocess is not EINTR-safe

[Dave Malcolm]

Changes by Dave Malcolm :


--
nosy: +dmalcolm

___
Python tracker 

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

[issue1068268] subprocess is not EINTR-safe

[Reid Kleckner]

Changes by Reid Kleckner :


--
nosy: +rnk

___
Python tracker 

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

[issue1068268] subprocess is not EINTR-safe

[R. David Murray]

Changes by R. David Murray :


--
nosy: +r.david.murray

___
Python tracker 

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

[issue1068268] subprocess is not EINTR-safe

[Chad Miller]

Chad Miller  added the comment:

File
"/home/cmiller/work/cabzr/desktopcouch/getport-at-call-time/desktopcouch/start_local_couchdb.py",
line 93, in run_couchdb
retcode = subprocess.call(local_exec)
  File "/usr/lib/python2.6/subprocess.py", line 444, in call
return Popen(*popenargs, **kwargs).wait()
  File "/usr/lib/python2.6/subprocess.py", line 1123, in wait
pid, sts = os.waitpid(self.pid, 0)
exceptions.OSError: [Errno 4] Interrupted system call

Now what?  The process started, but I have no way of knowing when it
finishes or the exit value when it does, because I don't have access to
the Popen object.  Nor can I even kill it and try again, because I can't
get he process id.

try/except in my code can never help.  It must be put in the stdlib.

Or, if this is too egregious, then the docs should scream that
subprocess.call can never safely be used, and users should avoid it.



  File
"/home/cmiller/work/cabzr/desktopcouch/getport-at-call-time/desktopcouch/start_local_couchdb.py",
line 93, in run_couchdb
retcode = subprocess.call(local_exec)
  File "/usr/lib/python2.6/subprocess.py", line 444, in call
return Popen(*popenargs, **kwargs).wait()
  File "/usr/lib/python2.6/subprocess.py", line 595, in __init__
errread, errwrite)
  File "/usr/lib/python2.6/subprocess.py", line 1084, in _execute_child
data = os.read(errpipe_read, 1048576) # Exceptions limited to 1 MB
exceptions.OSError: [Errno 4] Interrupted system call


This os.read is a byproduct of something the Popen.__init__
implementation must do, and it is safe for it to continue to get the
information it needs, without the user's knowledge.

The process is started, then this is aborted before the Popen.stdout and
.stderr are set up, leaving the object in a weird state.

--
nosy: +cmiller
Added file: http://bugs.python.org/file14700/trunk-diff-unified.txt

___
Python tracker 

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

[issue1068268] subprocess is not EINTR-safe

[STINNER Victor]

Changes by STINNER Victor :


--
nosy:  -haypo

___
Python tracker 

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

[issue1068268] subprocess is not EINTR-safe

[STINNER Victor]

STINNER Victor  added the comment:

Since Python 2.5 only accept security fixes, you should update your 
patch to Python trunk.

___
Python tracker 

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

[issue1068268] subprocess is not EINTR-safe

[Matteo Bertini]

Matteo Bertini  added the comment:

no EINTR patch upgraded to 25-maint r65475 that protects:

*) all direct calls
*) all returned fd

I hope :P

Added file: 
http://bugs.python.org/file12438/no-EINTR-subprocess.py-25-maint-r65475.patch

___
Python tracker 

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

[issue1068268] subprocess is not EINTR-safe

[Matteo Bertini]

Changes by Matteo Bertini :


Removed file: 
http://bugs.python.org/file11511/subprocess-eintr-safety-25maint-r65475.patch

___
Python tracker 

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

[issue1068268] subprocess is not EINTR-safe

[Matteo Bertini]

Changes by Matteo Bertini :


Removed file: 
http://bugs.python.org/file11818/subprocess-retry-on-EINTR-std-in-out-err.diff

___
Python tracker 

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

[issue1068268] subprocess is not EINTR-safe

[STINNER Victor]

STINNER Victor  added the comment:

Oh, the new patch (subprocess-retry-on-EINTR-std-in-out-err.diff) has 
already a generic function (_no_intr) which is a little bit different 
than mine (looks like a decorator). Can you delete your old patch?

___
Python tracker 

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

[issue1068268] subprocess is not EINTR-safe

[STINNER Victor]

STINNER Victor  added the comment:

Instead of define a method for each "syscall", you can write a generic 
function that retry at OSError(EINTR):

def no_intr(self, func, *args, **kw):
  while True:
try:
  return func(*args, **kw)
except OSError, e:
  if e.errno == errno.EINTR:
continue
  else:
raise

x=_waitpid_no_intr(pid, options) becomes x=no_intr(os.waitpid, pid, 
options).

___
Python tracker 

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

[issue1068268] subprocess is not EINTR-safe

[Matteo Bertini]

Matteo Bertini  added the comment:

Please have a look at the proposed patch:

http://bugs.python.org/file11511/subprocess-eintr-safety-25maint-
r65475.patch

the list is more or less the patch itself.

___
Python tracker 

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

[issue1068268] subprocess is not EINTR-safe

[Martin v. Löwis]

Changes by Martin v. Löwis :


--
versions:  -Python 2.4, Python 2.5, Python 2.5.3

___
Python tracker 

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

[issue1068268] subprocess is not EINTR-safe

[STINNER Victor]

STINNER Victor  added the comment:

naufraghi> there are a lot of other places where EINTR 
naufraghi> can cause and error.

Can you write a list of these places?

___
Python tracker 

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

[issue1068268] subprocess is not EINTR-safe

[STINNER Victor]

Changes by STINNER Victor :


--
nosy: +haypo

___
Python tracker 

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

[issue1068268] subprocess is not EINTR-safe

[Matteo Bertini]

Matteo Bertini  added the comment:

Python 2.5.3 is near but the I think the fix in
http://svn.python.org/view?rev=65475&view=rev
is not enough, there are a lot of other places where EINTR can cause and 
error.

--
versions: +Python 2.5.3

___
Python tracker 

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

[issue1068268] subprocess is not EINTR-safe

[Matteo Bertini]

Matteo Bertini <[EMAIL PROTECTED]> added the comment:

Ups, forgot a _no_intr around select.select

Index: subprocess.py
===
--- subprocess.py   (revisione 19645)
+++ subprocess.py   (copia locale)
@@ -1178,7 +1178,7 @@
 
 input_offset = 0
 while read_set or write_set:
-rlist, wlist, xlist = select.select(read_set, 
write_set, [])
+rlist, wlist, xlist = _no_intr(select.select)(read_set, 
write_set, [])
 
 if self.stdin in wlist:
 # When select has indicated that the file is 
writable,

___
Python tracker <[EMAIL PROTECTED]>

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

[issue1068268] subprocess is not EINTR-safe

[Matteo Bertini]

Matteo Bertini <[EMAIL PROTECTED]> added the comment:

Factorized try-except code, merged r65475 from 25-maint.
Protetect std[in|out|err] read and write too.

Added file: 
http://bugs.python.org/file11818/subprocess-retry-on-EINTR-std-in-out-err.diff

___
Python tracker <[EMAIL PROTECTED]>

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

[issue1068268] subprocess is not EINTR-safe

[Matteo Bertini]

Matteo Bertini <[EMAIL PROTECTED]> added the comment:

Upgrade subprocess.py patch to 25-maint r65475
(apply cleanly with http://bugs.python.org/issue2113 fixed)

--
keywords: +patch
Added file: 
http://bugs.python.org/file11511/subprocess-eintr-safety-25maint-r65475.patch

___
Python tracker <[EMAIL PROTECTED]>

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

[issue1068268] subprocess is not EINTR-safe

[Gregory P. Smith]

Gregory P. Smith <[EMAIL PROTECTED]> added the comment:

its too late in the release process for subprocess internals being EINTR
safe to make it into 2.6 but it is reasonable for 2.6.1.

--
priority: low -> normal
type:  -> behavior
versions: +Python 2.5, Python 2.6

___
Python tracker <[EMAIL PROTECTED]>

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

[issue1068268] subprocess is not EINTR-safe

[Amaury Forgeot d'Arc]

Amaury Forgeot d'Arc <[EMAIL PROTECTED]> added the comment:

Two remarks:
1 - The part of the patch around the call to select.select() is already
in trunk since r64756, almost in the same form. good.

2 - the patch seems to replace all calls to os.write, os.read and
os.waipid. But it is based on a very old version of subprocess, and
r38169 added a new call to os.waitpid. I don't know if it should be
replaced as well.

--
nosy: +amaury.forgeotdarc

___
Python tracker <[EMAIL PROTECTED]>

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

[issue1068268] subprocess is not EINTR-safe

[Matteo Bertini]

Matteo Bertini <[EMAIL PROTECTED]> added the comment:

I'd like to suggest to rise the priority of this bug.
Till this bus is around, no way using any module using subprocess.Popen 
form a PyQt app (and I suppose PyGtk and wxPython too).

--
nosy: +naufraghi

___
Python tracker <[EMAIL PROTECTED]>

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

[issue1068268] subprocess is not EINTR-safe

[Jesse Noller]

Jesse Noller <[EMAIL PROTECTED]> added the comment:

I think this should be resolved in 2.6/3.0 if possible. Especially if 
distributions like Ubuntu are self-patching the fix into place. For 
reference, see: http://mg.pov.lt/blog/subprocess-in-2.4

--
nosy: +jnoller

___
Python tracker <[EMAIL PROTECTED]>

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

[issue1068268] subprocess is not EINTR-safe

[Gregory P. Smith]

Gregory P. Smith <[EMAIL PROTECTED]> added the comment:

fyi - To fix issue #2113 I added handling of a select.error errno.EINTR
being raised during the select.select call in r64756.

___
Python tracker <[EMAIL PROTECTED]>

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

[issue1068268] subprocess is not EINTR-safe

[Jeffrey Yasskin]

Jeffrey Yasskin <[EMAIL PROTECTED]> added the comment:

I think the proper behavior on EINTR may depend on which subprocess call
we're in. For example, the user can easily loop on .wait() herself if
she wants to ignore EINTR. But it's a lot harder to loop on Popen() if
the read() in _execute_child is interrupted. So my inclination would be
to let EINTR be raised in the first case, and use a loop to handle it in
the second.

Regarding the patch, a wrapper function called as
"retry_on_eintr(obj.write, data)" might be a cleaner way to do it.

--
nosy: +jyasskin

_
Tracker <[EMAIL PROTECTED]>

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

[issue1068268] subprocess is not EINTR-safe

[Gregory P. Smith]

Changes by Gregory P. Smith <[EMAIL PROTECTED]>:


--
nosy: +gregory.p.smith

_
Tracker <[EMAIL PROTECTED]>

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

[issue1068268] subprocess is not EINTR-safe

[Ralf Schmitt]

Ralf Schmitt <[EMAIL PROTECTED]> added the comment:

Of course the signal handler may raise an exception, so my last argument
isn't that good.

_
Tracker <[EMAIL PROTECTED]>

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

[issue1068268] subprocess is not EINTR-safe

[Ralf Schmitt]

Ralf Schmitt added the comment:

In normal application code that signal.alarm is called for a reason. And
the caller most probably expects it to interrupt the read calls. So I
think the proposed patch is wrong. 

What was the signal interrupting the read calls? maybe SIGPIPE?

--
nosy: +schmir

_
Tracker <[EMAIL PROTECTED]>

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