Opened 9 years ago

Closed 6 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: exarkun, itamarst, warner Branch:
Author: Launchpad Bug:

Description


Attachments (1)

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

Download all attachments as: .zip

Change History (8)

Changed 9 years ago by warner

comment:1 Changed 9 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
quirk.

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_process.py
test case (which uses a new helper script called twisted/test/process_sleep.py).
At least the new test case fails without the process.py patch and passes with it.

thanks,
 -Brian

comment:2 Changed 9 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 9 years ago by exarkun

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

comment:4 Changed 8 years ago by itamarst

  • Resolution set to fixed
  • Status changed from new to closed

(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 8 years ago by itamarst

  • Resolution fixed deleted
  • Status changed from closed to reopened

Gah. Not closed just yet.

comment:6 Changed 6 years ago by exarkun

  • Resolution set to fixed
  • Status changed from reopened to closed

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 3 years ago by <automation>

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