sibson / vncdotool

A command line VNC client and python library
Other
451 stars 120 forks source link

1.1.0 client triggers twisted.internet.defer.AlreadyCalledError on disconnect() #256

Closed reticulatedpines closed 1 year ago

reticulatedpines commented 1 year ago

vncdotool version 1.1.0

VNC server and version Qemu 4.2.1 via -vnc. Hopefully not relevant.

Steps to reproduce

#!/usr/bin/env python3

import vncdotool
from vncdotool import api

print(vncdotool.__version__)
client = vncdotool.api.connect(":1234")
client.keyPress("m") # just to test the connection really established
client.disconnect() # throws UnhandledError and twisted AlreadyCalledError with 1.1.0,
                    # silent with 1.0.0
vncdotool.api.shutdown()

Expected result Clean disconnect with no error when calling disconnect(). Unless there's some API change with 1.1.0 and I'm doing it wrong?

Which erroneous result did you get instead UnhandledError and twisted AlreadyCalledError.

Additional information

I did some digging, and notice that command.py does this:

    def clientConnectionLost(self, connector: IConnector, reason: Failure) -> None:
        if reason.type == ConnectionDone:
            self.done(0)
        else:
            self.error(reason)

If I change client.py so it does similar, I don't see the error... but I definitely don't understand this code well enough to know if that's an appropriate fix or I'm just hiding something:

    def clientConnectionLost(self, connector: IConnector, reason: Failure) -> None:
        #self.deferred.errback(reason)
        if reason.type == ConnectionDone:
            pass
        else:
            self.deferred.errback(reason)

It does seem sensible that ConnectionDone is a special case error, and currently client.py isn't special casing it.

reticulatedpines commented 1 year ago

Slightly bad report, my mistake. The error doesn't trigger on the disconnect(). It triggers on shutdown(). And without the shutdown(), the script never exits, similar to issue 255. But there's no explicit multiprocessing or threads in this one.

sibson commented 1 year ago

fixed in 1.2.0