From fea0fb41fc7e508ae2417bbf3509be89764cc70c Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 4 Nov 2018 15:42:51 +0000 Subject: [PATCH 1/3] docs: update Changelog; closes #288 --- docs/changelog.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 0ae70a39..3c081d3e 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -219,7 +219,8 @@ Core Library * `#406 `_: connections could leak FDs when a child process failed to start. -* `#406 `_, +* `#288 `_, + `#406 `_, `#417 `_: connections could leave FD wrapper objects that had not been closed lying around to be closed during garbage collection, causing reused FD numbers to be closed at random moments. From 76ec4f201c6bd56d116b950888ae150301a8570b Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 4 Nov 2018 17:49:45 +0000 Subject: [PATCH 2/3] issue #413: paper over harmless duplicate del_route() Ideally it would only be called once, and in future maybe it can, but right now we need to cope with these cases: * Downstream parent notifies us of disconnection (DEL_ROUTE) * We notify ourself of disconnection * We notify ourself and so does downstream parent It's case 3 that causes the error. --- ansible_mitogen/services.py | 5 +---- mitogen/core.py | 4 ++-- mitogen/parent.py | 17 ++++++++++++----- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/ansible_mitogen/services.py b/ansible_mitogen/services.py index c97d71bb..adfd828b 100644 --- a/ansible_mitogen/services.py +++ b/ansible_mitogen/services.py @@ -303,12 +303,9 @@ class ContextService(mitogen.service.Service): longer reachable context. This method runs in the Broker thread and must not to block. """ - # TODO: there is a race between creation of a context and disconnection - # of its related stream. An error reply should be sent to any message - # in _latches_by_key below. self._lock.acquire() try: - LOG.info('Forgetting %r due to stream disconnect', context) + LOG.info('%r: Forgetting %r due to stream disconnect', self, context) self._forget_context_unlocked(context) finally: self._lock.release() diff --git a/mitogen/core.py b/mitogen/core.py index 058d1d3a..58289553 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -2104,8 +2104,8 @@ class Router(object): def _on_del_route(self, msg): """ - Stub DEL_ROUTE handler; fires 'disconnect' events on the corresponding - member of :attr:`_context_by_id`. This handler is replaced by + Stub :data:`DEL_ROUTE` handler; fires 'disconnect' events on the + corresponding :attr:`_context_by_id` member. This is replaced by :class:`mitogen.parent.RouteMonitor` in an upgraded context. """ LOG.error('%r._on_del_route() %r', self, msg) diff --git a/mitogen/parent.py b/mitogen/parent.py index c4e6f621..04f7784e 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -1558,6 +1558,9 @@ class RouteMonitor(object): #: stream; used to cleanup routes during disconnection. self._routes_by_stream = {} + def __repr__(self): + return 'RouteMonitor()' + def _send_one(self, stream, handle, target_id, name): """ Compose and send an update message on a stream. @@ -1644,7 +1647,8 @@ class RouteMonitor(object): Respond to disconnection of a local stream by """ routes = self._routes_by_stream.pop(stream) - LOG.debug('%r is gone; propagating DEL_ROUTE for %r', stream, routes) + LOG.debug('%r: %r is gone; propagating DEL_ROUTE for %r', + self, stream, routes) for target_id in routes: self.router.del_route(target_id) self._propagate_up(mitogen.core.DEL_ROUTE, target_id) @@ -1692,18 +1696,21 @@ class RouteMonitor(object): target_id = int(msg.data) registered_stream = self.router.stream_by_id(target_id) + if registered_stream is None: + return + stream = self.router.stream_by_id(msg.auth_id) if registered_stream != stream: - LOG.error('Received DEL_ROUTE for %d from %r, expected %r', - target_id, stream, registered_stream) + LOG.error('%r: received DEL_ROUTE for %d from %r, expected %r', + self, target_id, stream, registered_stream) return context = self.router.context_by_id(target_id, create=False) if context: - LOG.debug('%r: Firing local disconnect for %r', self, context) + LOG.debug('%r: firing local disconnect for %r', self, context) mitogen.core.fire(context, 'disconnect') - LOG.debug('Deleting route to %d via %r', target_id, stream) + LOG.debug('%r: deleting route to %d via %r', self, target_id, stream) routes = self._routes_by_stream.get(stream) if routes: routes.discard(target_id) From 7a1dfa388ac04d786f8d8ad8dfb3a3dff7c857c3 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 4 Nov 2018 17:51:51 +0000 Subject: [PATCH 3/3] docs: update Changelog; closes #413. --- docs/changelog.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 3c081d3e..93c21370 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -201,7 +201,8 @@ Core Library receivers to wake with :class:`mitogen.core.ChannelError`, even when one participant is not a parent of the other. -* `#387 `_: dead messages include an +* `#387 `_, + `#413 `_: dead messages include an optional reason in their body. This is used to cause :class:`mitogen.core.ChannelError` to report far more useful diagnostics at the point the error occurs that previously would have been buried in debug