Serialport connection lost not handled.

[mdamen]

When you disconnect a device connected to the serial port, twisted reports an unhandled error. Here is the traceback:

Traceback (most recent call last):
  File "C:\Python26\Lib\site-packages\twisted\python\log.py", line 84, in callWithLogger
    return callWithContext({"system": lp}, func, *args, **kw)
  File "C:\Python26\Lib\site-packages\twisted\python\log.py", line 69, in callWithContext
    return context.call({ILogContext: newCtx}, func, *args, **kw)
  File "C:\Python26\Lib\site-packages\twisted\python\context.py", line 59, in callWithContext
    return self.currentContext().callWithContext(ctx, func, *args, **kw)
  File "C:\Python26\Lib\site-packages\twisted\python\context.py", line 37, in callWithContext
    return func(*args,**kw)
--- <exception caught here> ---
  File "C:\Python26\Lib\site-packages\twisted\internet\win32eventreactor.py", line 211, in _runAction
    closed = getattr(fd, action)()
  File "C:\Python26\Lib\site-packages\twisted\internet\_win32serialport.py", line 68, in serialReadEvent
    n = win32file.GetOverlappedResult(self._serial.hComPort, self._overlappedRead, 0)
pywintypes.error: (995, 'GetOverlappedResult', 'The I/O operation has been aborted because of either a thread exit or an application request.')

There should be some kind of error handling to prevent this, I had a look at it but can't figure out what should be done. Here's a basic example to reproduce the problem:

from twisted.internet import protocol
from twisted.internet.serialport import SerialPort
from sys import platform
if platform == 'win32':
    from twisted.internet import win32eventreactor
    win32eventreactor.install()
from twisted.internet import reactor

class TestProto(protocol.Protocol):
    def connectionMade(self):
        print "connection made"
        
    def connectionLost(self):
        print "connection lost"

SerialPort(TestProto(),2, reactor, '115200')        
reactor.run()

[mdamen]

Forgot to mention the twisted version: 9.0.0 on python 2.6.

[exarkun]

For actually testing the serial port code, there's #2462.

[itamarst]

(In [32929]) Branching to 'serialport-connectionLost-4249'

[itamar]

The original ticket for this, #3690, has evolved into a testing ticket; I will change the description there if this ticket is approved.

For this issue, I just did a minimal test that doesn't actually depend on actual having a serial port (fake or otherwise).

Build results, mostly from penultimate revision of branch, except for winxp32-py2.5 which is the only windows slave that seems to have pyserial installed, which I reran with fix for windows, and debian64-py2.4-select which also has pyserial and also has run from final revision:

 http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/serialport-connectionLost-4249

[itamar]

As far as the original reporter's problem in this ticket, my expectation is that that the Failure will get passed to the protocol's connectionLost where the user could then deal with it (the sample code he gives is wrong, though, connectionLost needs to accept a reason argument).

[exarkun]

I don't think #3690 is the "original" of this ticket. The GetOverlappedResult error in the traceback in the description doesn't seem to be related to whether the protocol's connectionLost method is called.

Sadly, I cannot reproduce the error with my Windows setup. In fact, I can't trigger a disconnected serial port at all.

Still, it seems like the fix here is just a duplicate of the fix attempted for #3690, not a fix for this issue. Please correct me if I am mistaken.

[itamar]

My interpretation is that some unexpected error occurred -- which happens, and is perhaps unsolvable if it's hard to reproduce -- it got logged but it never got reported in connectionLost so no handling could be done. So maybe this bug is both #3690 and a separate issue? And my fix covers #3690 only, yes.

Perhaps I should rename this branch to be a #3690 branch, and rename the #3690 branch to be a #2462 branch? My basic goal is to get a fix for no connectionLost merged, and the current branch for #3690 is stuck in limbo since it's covering the much more ambitious requirements of #2462 (i.e. general testing).

[exarkun]

Yea, that sounds like a good plan.

[itamar]

OK, branches were moved.

[itamar]

I am not qualified to deal with the Windows issue.