Opened 4 years ago

Last modified 3 years ago

#4249 defect new

Serialport connection lost not handled.

Reported by: mdamen Owned by:
Priority: normal Milestone:
Component: core Keywords:
Cc: jason.heeris@… Branch:
Author: itamar Launchpad Bug:

Description

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()

Change History (14)

comment:1 Changed 4 years ago by mdamen

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

comment:2 Changed 4 years ago by exarkun

  • Keywords serialport added
  • Owner changed from glyph to PenguinOfDoom

comment:3 Changed 4 years ago by exarkun

  • Keywords win32 added

comment:4 Changed 4 years ago by exarkun

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

comment:5 Changed 3 years ago by <automation>

  • Owner PenguinOfDoom deleted

comment:6 Changed 3 years ago by detly

  • Cc jason.heeris@… added

comment:7 Changed 3 years ago by itamarst

  • Author set to itamarst
  • Branch set to branches/serialport-connectionLost-4249

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

comment:8 Changed 3 years ago by itamar

  • Author changed from itamarst to itamar
  • Keywords review added; serialport win32 removed

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

comment:9 Changed 3 years ago by 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).

comment:10 Changed 3 years ago by exarkun

  • Keywords review removed
  • Owner set to itamar

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.

comment:11 Changed 3 years ago by 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).

comment:12 Changed 3 years ago by exarkun

Yea, that sounds like a good plan.

comment:13 Changed 3 years ago by itamar

  • Branch branches/serialport-connectionLost-4249 deleted

OK, branches were moved.

comment:14 Changed 3 years ago by itamar

  • Owner itamar deleted

I am not qualified to deal with the Windows issue.

Note: See TracTickets for help on using tickets.