diff mbox series

asyncrpc: Close the client event loop when the client is collected

Message ID 20260806161258.3815426-1-fjpedrazag@gmail.com
State New
Headers show
Series asyncrpc: Close the client event loop when the client is collected | expand

Commit Message

Francisco Pedraza Aug. 6, 2026, 4:12 p.m. UTC
Client creates its own event loop and installs it with
asyncio.set_event_loop(), so a client dropped without close() being
called keeps the loop, and any transport that loop owns, alive. The
loop only becomes collectable once a later client replaces the global
reference, and asyncio then reports the transport and the loop as
unclosed:

  ResourceWarning: unclosed transport <asyncio.sslproto._SSLProtocolTransport object>
  ResourceWarning: unclosed event loop <_UnixSelectorEventLoop running=False closed=False>

This is why the warning shows up at an unrelated later point rather
than where the client was leaked.

SignatureGeneratorUniHashMixIn caches its client and only closes it
from reset() and exit(), so any teardown path missing those leaks it.

Register a weakref.finalize() to close the loop when the client is
collected. close() detaches the finalizer first, so the explicit path
does the same work as before.

Reproduced with two clients where the first is never closed, on python
3.10 with websockets 10.4 and on python 3.14 with websockets 17.0.1.
Both report the warnings without this change and neither does with it.
bitbake-selftest hashserv.tests passes.

Fixes [YOCTO #16236]

Signed-off-by: Francisco Pedraza <fjpedrazag@gmail.com>
---
 lib/bb/asyncrpc/client.py | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

Comments

Joshua Watt Aug. 6, 2026, 4:32 p.m. UTC | #1
On Thu, Aug 6, 2026 at 10:13 AM Francisco Pedraza via lists.openembedded.org
<fjpedrazag=gmail.com@lists.openembedded.org> wrote:

> Client creates its own event loop and installs it with
> asyncio.set_event_loop(), so a client dropped without close() being
> called keeps the loop, and any transport that loop owns, alive. The
> loop only becomes collectable once a later client replaces the global
> reference, and asyncio then reports the transport and the loop as
> unclosed:
>
>   ResourceWarning: unclosed transport
> <asyncio.sslproto._SSLProtocolTransport object>
>   ResourceWarning: unclosed event loop <_UnixSelectorEventLoop
> running=False closed=False>
>
> This is why the warning shows up at an unrelated later point rather
> than where the client was leaked.
>
> SignatureGeneratorUniHashMixIn caches its client and only closes it
> from reset() and exit(), so any teardown path missing those leaks it.
>
> Register a weakref.finalize() to close the loop when the client is
> collected. close() detaches the finalizer first, so the explicit path
> does the same work as before.
>
> Reproduced with two clients where the first is never closed, on python
> 3.10 with websockets 10.4 and on python 3.14 with websockets 17.0.1.
> Both report the warnings without this change and neither does with it.
> bitbake-selftest hashserv.tests passes.
>

LGTM Thanks

Reviewed-by: Joshua Watt <JPEWhacker@gmail.com>


>
> Fixes [YOCTO #16236]
>
> Signed-off-by: Francisco Pedraza <fjpedrazag@gmail.com>
> ---
>  lib/bb/asyncrpc/client.py | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/lib/bb/asyncrpc/client.py b/lib/bb/asyncrpc/client.py
> index 17b72033b..115acd456 100644
> --- a/lib/bb/asyncrpc/client.py
> +++ b/lib/bb/asyncrpc/client.py
> @@ -12,6 +12,7 @@ import socket
>  import sys
>  import re
>  import contextlib
> +import weakref
>  from threading import Thread
>  from .connection import StreamConnection, WebsocketConnection,
> DEFAULT_MAX_CHUNK
>  from .exceptions import ConnectionClosedError, InvokeError
> @@ -224,8 +225,29 @@ class Client(object):
>          # required (but harmless) with it.
>          asyncio.set_event_loop(self.loop)
>
> +        # The loop and its transports must be closed even if the caller
> never
> +        # calls close(). Since set_event_loop() above replaces the
> reference
> +        # held by the previous client, an unclosed loop only becomes
> reachable
> +        # for collection once another client is created, at which point
> +        # asyncio reports "unclosed transport" and "unclosed event loop"
> +        # ResourceWarnings. The finalizer is detached by close() so that
> the
> +        # normal path is unaffected.
> +        self._finalizer = weakref.finalize(
> +            self, self._close_loop, self.loop, self.client
> +        )
> +
>          self._add_methods("connect_tcp", "ping")
>
> +    @staticmethod
> +    def _close_loop(loop, client):
> +        if loop.is_closed():
> +            return
> +        try:
> +            loop.run_until_complete(client.close())
> +            loop.run_until_complete(loop.shutdown_asyncgens())
> +        finally:
> +            loop.close()
> +
>      @abc.abstractmethod
>      def _get_async_client(self):
>          pass
> @@ -258,9 +280,8 @@ class Client(object):
>
>      def close(self):
>          if self.loop:
> -            self.loop.run_until_complete(self.client.close())
> -            self.loop.run_until_complete(self.loop.shutdown_asyncgens())
> -            self.loop.close()
> +            self._finalizer.detach()
> +            self._close_loop(self.loop, self.client)
>          self.loop = None
>
>      def __enter__(self):
> --
> 2.55.0
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#19904):
> https://lists.openembedded.org/g/bitbake-devel/message/19904
> Mute This Topic: https://lists.openembedded.org/mt/120629140/3616693
> Group Owner: bitbake-devel+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [
> JPEWhacker@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
Anibal Limon Aug. 6, 2026, 5:43 p.m. UTC | #2
Only a small comment below.


On Thu, Aug 6, 2026 at 10:13 AM Francisco Pedraza via lists.openembedded.org
<fjpedrazag=gmail.com@lists.openembedded.org> wrote:

> Client creates its own event loop and installs it with
> asyncio.set_event_loop(), so a client dropped without close() being
> called keeps the loop, and any transport that loop owns, alive. The
> loop only becomes collectable once a later client replaces the global
> reference, and asyncio then reports the transport and the loop as
> unclosed:
>
>   ResourceWarning: unclosed transport
> <asyncio.sslproto._SSLProtocolTransport object>
>   ResourceWarning: unclosed event loop <_UnixSelectorEventLoop
> running=False closed=False>
>
> This is why the warning shows up at an unrelated later point rather
> than where the client was leaked.
>
> SignatureGeneratorUniHashMixIn caches its client and only closes it
> from reset() and exit(), so any teardown path missing those leaks it.
>
> Register a weakref.finalize() to close the loop when the client is
> collected. close() detaches the finalizer first, so the explicit path
> does the same work as before.
>
> Reproduced with two clients where the first is never closed, on python
> 3.10 with websockets 10.4 and on python 3.14 with websockets 17.0.1.
> Both report the warnings without this change and neither does with it.
> bitbake-selftest hashserv.tests passes.
>
> Fixes [YOCTO #16236]
>
> Signed-off-by: Francisco Pedraza <fjpedrazag@gmail.com>
> ---
>  lib/bb/asyncrpc/client.py | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/lib/bb/asyncrpc/client.py b/lib/bb/asyncrpc/client.py
> index 17b72033b..115acd456 100644
> --- a/lib/bb/asyncrpc/client.py
> +++ b/lib/bb/asyncrpc/client.py
> @@ -12,6 +12,7 @@ import socket
>  import sys
>  import re
>  import contextlib
> +import weakref
>  from threading import Thread
>  from .connection import StreamConnection, WebsocketConnection,
> DEFAULT_MAX_CHUNK
>  from .exceptions import ConnectionClosedError, InvokeError
> @@ -224,8 +225,29 @@ class Client(object):
>          # required (but harmless) with it.
>          asyncio.set_event_loop(self.loop)
>
> +        # The loop and its transports must be closed even if the caller
> never
> +        # calls close(). Since set_event_loop() above replaces the
> reference
> +        # held by the previous client, an unclosed loop only becomes
> reachable
> +        # for collection once another client is created, at which point
> +        # asyncio reports "unclosed transport" and "unclosed event loop"
> +        # ResourceWarnings. The finalizer is detached by close() so that
> the
> +        # normal path is unaffected.
> +        self._finalizer = weakref.finalize(
> +            self, self._close_loop, self.loop, self.client
> +        )
> +
>          self._add_methods("connect_tcp", "ping")
>
> +    @staticmethod
> +    def _close_loop(loop, client):
> +        if loop.is_closed():
> +            return
> +        try:
> +            loop.run_until_complete(client.close())
> +            loop.run_until_complete(loop.shutdown_asyncgens())
>

Maybe it's a good idea to add an exception to handle certain types of
errors and log when they happen.


> +        finally:
> +            loop.close()
> +
>      @abc.abstractmethod
>      def _get_async_client(self):
>          pass
> @@ -258,9 +280,8 @@ class Client(object):
>
>      def close(self):
>          if self.loop:
> -            self.loop.run_until_complete(self.client.close())
> -            self.loop.run_until_complete(self.loop.shutdown_asyncgens())
> -            self.loop.close()
> +            self._finalizer.detach()
> +            self._close_loop(self.loop, self.client)
>          self.loop = None
>
>      def __enter__(self):
> --
> 2.55.0
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#19904):
> https://lists.openembedded.org/g/bitbake-devel/message/19904
> Mute This Topic: https://lists.openembedded.org/mt/120629140/8181911
> Group Owner: bitbake-devel+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [
> anibal@limonsoftware.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
diff mbox series

Patch

diff --git a/lib/bb/asyncrpc/client.py b/lib/bb/asyncrpc/client.py
index 17b72033b..115acd456 100644
--- a/lib/bb/asyncrpc/client.py
+++ b/lib/bb/asyncrpc/client.py
@@ -12,6 +12,7 @@  import socket
 import sys
 import re
 import contextlib
+import weakref
 from threading import Thread
 from .connection import StreamConnection, WebsocketConnection, DEFAULT_MAX_CHUNK
 from .exceptions import ConnectionClosedError, InvokeError
@@ -224,8 +225,29 @@  class Client(object):
         # required (but harmless) with it.
         asyncio.set_event_loop(self.loop)
 
+        # The loop and its transports must be closed even if the caller never
+        # calls close(). Since set_event_loop() above replaces the reference
+        # held by the previous client, an unclosed loop only becomes reachable
+        # for collection once another client is created, at which point
+        # asyncio reports "unclosed transport" and "unclosed event loop"
+        # ResourceWarnings. The finalizer is detached by close() so that the
+        # normal path is unaffected.
+        self._finalizer = weakref.finalize(
+            self, self._close_loop, self.loop, self.client
+        )
+
         self._add_methods("connect_tcp", "ping")
 
+    @staticmethod
+    def _close_loop(loop, client):
+        if loop.is_closed():
+            return
+        try:
+            loop.run_until_complete(client.close())
+            loop.run_until_complete(loop.shutdown_asyncgens())
+        finally:
+            loop.close()
+
     @abc.abstractmethod
     def _get_async_client(self):
         pass
@@ -258,9 +280,8 @@  class Client(object):
 
     def close(self):
         if self.loop:
-            self.loop.run_until_complete(self.client.close())
-            self.loop.run_until_complete(self.loop.shutdown_asyncgens())
-            self.loop.close()
+            self._finalizer.detach()
+            self._close_loop(self.loop, self.client)
         self.loop = None
 
     def __enter__(self):