Merge pull request #390 from meejah/389.hang-getinfo

Increase line-length limit (o prevent GETINFO issues)

docs/releases.rst

@@ -18,6 +18,7 @@ See also :ref:`api_stability`.
 `git main <https://github.com/meejah/txtorcon>`_ *will likely become v23.6.0*
 
  * Fix test-failures on Python 3.12
+ * Particular GETINFO hanging (`#389 <https://github.com/meejah/txtorcon/issues/389>`_)
 
 
 v23.5.0

test/test_torcontrolprotocol.py

@@ -226,7 +226,7 @@ def it_was_called(arg):
         it_was_called.yes = False
 
         d = self.protocol.when_disconnected()
-        d.addCallback(it_was_called)
+        d.addBoth(it_was_called)
         f = failure.Failure(error.ConnectionDone("It's all over"))
         self.protocol.connectionLost(f)
         self.assertTrue(it_was_called.yes)
@@ -284,6 +284,31 @@ def it_was_called(f):
         self.protocol.connectionLost(f)
         self.assertEqual(it_was_called.count, 2)
 
+    def test_disconnect_subsequent_commands(self):
+        """
+        commands issued after disconnect should errback
+        """
+
+        def it_was_called(f):
+            str(f)
+            it_was_called.count += 1
+            return None
+        it_was_called.count = 0
+
+        # one outstanding command
+        d0 = self.protocol.queue_command("some command0")
+        d0.addErrback(it_was_called)
+        self.protocol.on_disconnect.addErrback(lambda _: None)
+
+        f = failure.Failure(RuntimeError("The thing didn't do the stuff."))
+        self.protocol.connectionLost(f)
+
+        # one command issued _after_ we've disconnected
+        d1 = self.protocol.queue_command("some command1")
+        d1.addErrback(it_was_called)
+
+        self.assertEqual(it_was_called.count, 2)
+
 
 class ProtocolTests(unittest.TestCase):
 

txtorcon/torcontrolprotocol.py

@@ -232,6 +232,12 @@ class TorControlProtocol(LineOnlyReceiver):
     :class:`txtorcon.TorState`, which is also the place to go if you
     wish to add your own stream or circuit listeners.
     """
+    # override Twisted's LineOnlyReceiver maximum line-length. At
+    # least "GETINFO md/id/X" for some Xse exceeds 16384 (2**14, the
+    # default) and thus causes the control connection to
+    # fail. control.c defines MAX_COMMAND_LINE_LENGTH as 1024*1024 so
+    # we use that
+    MAX_LENGTH = 2 ** 20
 
     def __init__(self, password_function=None):
         """
@@ -274,11 +280,6 @@ def __init__(self, password_function=None):
         :func:`when_disconnected` instead)
         """
 
-        self._when_disconnected = SingleObserver()
-        """
-        Internal use. A :class:`SingleObserver` for when_disconnected()
-        """
-
         self._when_disconnected = SingleObserver()
         """
         Private. See :func:`.when_disconnected`
@@ -356,7 +357,7 @@ def setup(proto):
         self.stop_debug()
 
     def start_debug(self):
-        self.debuglog = open('txtorcon-debug.log', 'w')
+        self.debuglog = open('txtorcon-debug.log', 'wb')
 
     def stop_debug(self):
         def noop(*args, **kw):
@@ -692,10 +693,14 @@ def connectionMade(self):
     def connectionLost(self, reason):
         "Protocol API"
         txtorlog.msg('connection terminated: ' + str(reason))
-        if reason.check(ConnectionDone):
-            self._when_disconnected.fire(self)
-        else:
-            self._when_disconnected.fire(reason)
+        self._when_disconnected.fire(
+            Failure(
+                TorDisconnectError(
+                    text="Tor connection terminated",
+                    error=reason,
+                )
+            )
+        )
 
         # ...and this is why we don't do on_disconnect = Deferred() :(
         # and instead should have had on_disconnect() method that
@@ -712,8 +717,10 @@ def connectionLost(self, reason):
             else:
                 self.on_disconnect.errback(reason)
         self.on_disconnect = None
-        self._when_disconnected.fire(self)
+
         outstanding = [self.command] + self.commands if self.command else self.commands
+        self.command = None
+        self.defer = None
         for d, cmd, cmd_arg in outstanding:
             if not d.called:
                 d.errback(
@@ -754,6 +761,10 @@ def _maybe_issue_command(self):
         if len(self.commands):
             self.command = self.commands.pop(0)
             (d, cmd, cmd_arg) = self.command
+
+            if self._when_disconnected.already_fired(d):
+                return
+
             self.defer = d
 
             self.debuglog.write(cmd + b'\n')

txtorcon/util.py

@@ -473,6 +473,19 @@ def __init__(self):
         self._observers = []
         self._fired = self._NotFired
 
+    def has_fired(self):
+        return self._fired is not self._NotFired
+
+    def already_fired(self, d):
+        """
+        If we have already fired, callback `d` with our result.
+        :returns bool: True if we already fired, False otherwise
+        """
+        if self.has_fired():
+            d.callback(self._fired)
+            return True
+        return False
+
     def when_fired(self):
         d = defer.Deferred()
         if self._fired is not self._NotFired: