fix: stop leaking a twisted log observer per Ros instance

TwistedEventLoopManager.__init__ instantiated PythonLoggingObserver and
started it; the observer was only stopped in manager.terminate(), which
in turn was only invoked by Ros.terminate() (which stops the reactor
permanently). Downstream code that follows the documented
RosClient.__exit__ -> Ros.close() pattern never called terminate(), so
every Ros instance added a permanent observer to twisted's
globalLogPublisher.

In long-running pytest sessions that create many short-lived Ros
instances (e.g. compas_fab's integration suite), this accumulated tens
of observers and slowed every twisted log emission linearly. It also
contributed to occasional RosTimeoutError flakes on the next
Ros.run() after many cycles.

Make the observer a process-wide singleton: TwistedEventLoopManager
lazily starts a single module-level PythonLoggingObserver via a thread-
safe initializer, and terminate() now stops it once and clears the
sentinel. Verified by a new tests/ros1/test_lifecycle.py case that
constructs 10 Ros instances and asserts at most one observer is added.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Author: gonzalocasas

CHANGELOG.rst

@@ -16,6 +16,14 @@ Unreleased
 
 **Fixed**
 
+* ``TwistedEventLoopManager`` no longer leaks one ``PythonLoggingObserver`` per
+  ``Ros`` instance. The observer is now a process-wide singleton — there's only
+  ever one twisted-log → stdlib-logging bridge regardless of how many ``Ros``
+  instances come and go. Long-running pytest sessions that create many
+  short-lived ``Ros`` instances were accumulating one observer per cycle,
+  slowing every log emission linearly and contributing to the occasional
+  ``RosTimeoutError`` flakes that motivated this lifecycle pass.
+
 **Deprecated**
 
 **Removed**

src/roslibpy/comm/comm_autobahn.py

@@ -156,6 +156,37 @@ def set_max_retries(cls, max_retries):
         cls.maxRetries = max_retries
 
 
+# The twisted reactor is a process-wide singleton, so the log observer that
+# fans twisted's log events into the stdlib `logging` module is one too. Any
+# Python process only ever needs one of these registered — registering more
+# means every log event walks an O(n) observer list and ``globalLogPublisher``
+# keeps strong references to every observer for the life of the process.
+#
+# Prior to 2.1, ``TwistedEventLoopManager.__init__`` started a fresh
+# ``PythonLoggingObserver`` per instance, and downstream wrappers that call
+# ``Ros.close()`` (rather than ``Ros.terminate()``, which stops the reactor
+# permanently) never invoked ``manager.terminate()`` and so never stopped the
+# observer. Long-running pytest sessions that create many ``Ros`` instances
+# would accumulate one observer per ``Ros``, slowing every log emission
+# linearly and contributing to occasional ``RosTimeoutError`` flakes on the
+# next ``Ros.run()``.
+_LOG_OBSERVER = None
+_LOG_OBSERVER_LOCK = threading.Lock()
+
+
+def _ensure_log_observer_started():
+    """Lazily start the single process-wide twisted log → stdlib logging bridge."""
+    global _LOG_OBSERVER
+    if _LOG_OBSERVER is not None:
+        return
+    with _LOG_OBSERVER_LOCK:
+        if _LOG_OBSERVER is not None:
+            return
+        observer = log.PythonLoggingObserver()
+        observer.start()
+        _LOG_OBSERVER = observer
+
+
 class TwistedEventLoopManager(object):
     """Manage the main event loop using Twisted reactor.
 
@@ -166,8 +197,11 @@ class TwistedEventLoopManager(object):
     """
 
     def __init__(self):
-        self._log_observer = log.PythonLoggingObserver()
-        self._log_observer.start()
+        # Twisted's reactor is a process singleton — so is the observer that
+        # bridges twisted's log events into stdlib `logging`. See the module-
+        # level comment for the lifecycle details.
+        _ensure_log_observer_started()
+        self._log_observer = _LOG_OBSERVER
 
     def run(self):
         """Kick-starts a non-blocking event loop.
@@ -273,4 +307,13 @@ def terminate(self):
         if self._thread:
             self._thread.join()
 
-        self._log_observer.stop()
+        # The observer is the process-wide singleton (see module top). Stop
+        # it once on terminate — the reactor cannot be restarted after stop,
+        # so no future TwistedEventLoopManager will need the bridge in this
+        # process. Subsequent terminates no-op.
+        global _LOG_OBSERVER
+        with _LOG_OBSERVER_LOCK:
+            if _LOG_OBSERVER is not None:
+                _LOG_OBSERVER.stop()
+                _LOG_OBSERVER = None
+        self._log_observer = None

tests/ros1/test_lifecycle.py

@@ -0,0 +1,50 @@
+"""Lifecycle regression tests for the 2.1 fixes.
+
+Targets bugs that emerged from downstream pytest sessions (``compas_fab``)
+where many sequential ``Ros`` instances are constructed, run, and closed
+in the same Python process.
+
+If these tests flake, the corresponding fix is regressing — diagnose,
+don't retry-loop.
+"""
+
+from __future__ import print_function
+
+from roslibpy import Ros
+
+HOST = "127.0.0.1"
+PORT = 9090
+URL = "ws://%s:%d" % (HOST, PORT)
+
+
+def test_log_observer_does_not_leak():
+    """Constructing many ``Ros`` instances must not leak twisted log observers.
+
+    Before A1 every ``TwistedEventLoopManager.__init__`` (created lazily
+    one-per-``Ros``) called ``PythonLoggingObserver().start()`` and never
+    cleaned up unless ``manager.terminate()`` was explicitly called.
+    Downstream wrappers (compas_fab's ``RosClient.__exit__``) call
+    ``close()``, not ``terminate()`` — by design, since ``terminate()``
+    stops the reactor permanently. So observers piled up forever.
+
+    After A1 there's exactly one process-wide observer regardless of how
+    many ``Ros()`` instances come and go.
+    """
+    from twisted.logger import globalLogPublisher
+
+    initial = len(list(globalLogPublisher._observers))
+
+    for _ in range(10):
+        ros = Ros(URL)
+        ros.run()
+        ros.close()
+
+    final = len(list(globalLogPublisher._observers))
+    # Allow a single-observer drift in case the very first cycle is the one
+    # that registers the singleton. The leak case grows by len(cycles).
+    assert final - initial <= 1, (
+        "Leaked %d twisted log observers across 10 lifecycle cycles "
+        "(expected at most 1)" % (final - initial)
+    )
+
+