Opened 13 years ago

Closed 10 years ago

#1120 defect closed fixed (fixed)

OS-X vs PTYProcess, OSError vs IOError

Reported by: warner Owned by:
Priority: high Milestone:
Component: core Keywords:
Cc: Jean-Paul Calderone, itamarst, warner Branch:


Attachments (1)

osx.diff (1.8 KB) - added by warner 13 years ago.

Download all attachments as: .zip

Change History (8)

Changed 13 years ago by warner

Attachment: osx.diff added

comment:1 Changed 13 years ago by warner

I've experienced some failures in the buildbot unit test suite that occur under
OS-X but not under other unices. The scenario is:

 open a PTYProcess to a long-running child
 decide the child is stuck, kill it off:
  first do transport.signalProcess("KILL") to try and kill it
  then immediately do transport.loseConnection() so we stop seeing its output

It appears that this causes PTYProcess.writeSomeData() to be called, and it
tries to os.write() an empty string to the doomed fd. (I don't know whether the
fd is closed by this point or not, but I believe it is closed). I believe the
write is superfluous: the point is to get notified *when* the fd becomes
writable because that's how we know the pipe is closed.

Anyway, that os.write raises "exceptions.OSError: [Errno 5] Input/output error"
under OS-X (whereas linux raises an IOError). I think this is an OS-X -specific

There is code in Process.writeSomeData to cause OSErrors in addition to
IOErrors, but this code is not present in PTYProcess.writeSomeData. I copied and
modified this code (to treat all OSErrors other than EAGAIN as CONNECTION_LOST),
and my buildbot test now passes.

I've attached a patch which fixes this (I think) and includes a
test case (which uses a new helper script called twisted/test/
At least the new test case fails without the patch and passes with it.


comment:2 Changed 13 years ago by itamarst

We should make a single, correct writeSomeData function in t.i.fdesc and make
Process, ProcessPTY and t.i.stdio use it.

comment:3 Changed 13 years ago by Jean-Paul Calderone

Also, PTYProcess and Process should not contain such massive code duplication
between each other.

comment:4 Changed 11 years ago by itamarst

Resolution: fixed
Status: newclosed

(In [19174]) merged code that writes to fds into twisted.internet.fdesc

should fix #1120, but I couldn't get warner's test to fail, so who knows.

comment:5 Changed 11 years ago by itamarst

Resolution: fixed
Status: closedreopened

Gah. Not closed just yet.

comment:6 Changed 10 years ago by Jean-Paul Calderone

Resolution: fixed
Status: reopenedclosed

Some code got committed (the #2341 branch got merged). It's too bad Itamar couldn't reproduce the issue at the time, but no one has complained since and I can't remember seeing anything that might be attributed to this problem on our own buildbot. If the problem still exists, I expect someone will file a new ticket or they can re-open this one.

comment:7 Changed 7 years ago by <automation>

Owner: itamarst deleted
Note: See TracTickets for help on using tickets.