Re: [PATCH v3] Use default asyncio event loop implementation in API consumer threads

Zac Medico <[email protected]> Sun, 6 Dec 2020 15:19:27 -0800
Newsgroups	gmane.linux.gentoo.portage.devel
Message-ID	<[email protected]>
Accidentally encrypted the last email. Here's an unencrypted version.

On 12/6/20 2:14 PM, Zac Medico wrote:
> Make the _safe_loop function return an AsyncioEventLoop instance,
> so that the default asyncio event loop implementation will be used
> in API consumer threads. This is possible because the underlying
> asyncio.get_event_loop() function returns a new event loop for
> each thread. The AsyncioEventLoop _run_until_complete method will
> now appropriately handle a ValueError from signal.set_wakeup_fd(-1)
> if it is not called in the main thread.
> 
> Bug: https://bugs.gentoo.org/758755
> Signed-off-by: Zac Medico <[email protected]>
> ---
> [PATCH v3] fixed AsyncioEventLoop _run_until_complete method to
> handle ValueError from signal.set_wakeup_fd(-1)
> 
>  lib/portage/util/_eventloop/asyncio_event_loop.py | 6 +++++-
>  lib/portage/util/futures/_asyncio/__init__.py     | 3 +--
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/portage/util/_eventloop/asyncio_event_loop.py b/lib/portage/util/_eventloop/asyncio_event_loop.py
> index 836f1c30a..4d7047ae8 100644
> --- a/lib/portage/util/_eventloop/asyncio_event_loop.py
> +++ b/lib/portage/util/_eventloop/asyncio_event_loop.py
> @@ -121,4 +121,8 @@ class AsyncioEventLoop(_AbstractEventLoop):
>  		try:
>  			return self._loop.run_until_complete(future)
>  		finally:
> -			self._wakeup_fd = signal.set_wakeup_fd(-1)
> +			try:
> +				self._wakeup_fd = signal.set_wakeup_fd(-1)
> +			except ValueError:
> +				# This is intended to fail when not called in the main thread.
> +				pass
> diff --git a/lib/portage/util/futures/_asyncio/__init__.py b/lib/portage/util/futures/_asyncio/__init__.py
> index a902ad895..12013be00 100644
> --- a/lib/portage/util/futures/_asyncio/__init__.py
> +++ b/lib/portage/util/futures/_asyncio/__init__.py
> @@ -34,7 +34,6 @@ import portage
>  portage.proxy.lazyimport.lazyimport(globals(),
>  	'portage.util.futures.unix_events:_PortageEventLoopPolicy',
>  	'portage.util.futures:compat_coroutine@_compat_coroutine',
> -	'portage.util._eventloop.EventLoop:EventLoop@_EventLoop',
>  )
>  from portage.util._eventloop.asyncio_event_loop import AsyncioEventLoop as _AsyncioEventLoop
>  from portage.util._eventloop.global_event_loop import (
> @@ -256,4 +255,4 @@ def _safe_loop():
>  	"""
>  	if portage._internal_caller:
>  		return _global_event_loop()
> -	return _EventLoop(main=False)
> +	return _AsyncioEventLoop()
> 

This fails if an event loop has not been created for the current thread:

  File "/usr/lib/python3.8/asyncio/events.py", line 639, in get_event_loop
    raise RuntimeError('There is no current event loop in thread %r.'
RuntimeError: There is no current event loop in thread 'Thread-1'.

However, if we automatically instantiate a loop for the current thread
then we will be responsible for closing it as well, or else we'll
eventually see a ResourceWarning like this:

/usr/lib/python3.8/asyncio/base_events.py:654: ResourceWarning: unclosed
event loop <_UnixSelectorEventLoop running=False closed=False debug=False>
  _warn(f"unclosed event loop {self!r}", ResourceWarning, source=self)
ResourceWarning: Enable tracemalloc to get the object allocation traceback

So, I think it's probably best if we force the API consumer to manage
the lifecycle of an asyncio loop for each thread that it uses to call
the portage API.
-- 
Thanks,
Zac
signature.asc (application/pgp-signature, 981 B)