[gentoo-portage-dev] [PATCH] Use default asyncio event loop implementation in child processes

[gentoo-portage-dev] [PATCH] Use default asyncio event loop implementation in child processes

[Zac Medico]

Use the default asyncio event loop implementation in child
processes, instead of portage's internal EventLoop. After
fork, instantiate a new asyncio.DefaultEventLoopPolicy as
a workaround for https://bugs.python.org/issue22087, which
is necessary for RetryTestCase to succeed.

Bug: https://bugs.gentoo.org/758740
Signed-off-by: Zac Medico <zmedico@gentoo.org>
---
 lib/portage/__init__.py                          | 4 ++++
 lib/portage/util/_eventloop/global_event_loop.py | 7 -------
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/lib/portage/__init__.py b/lib/portage/__init__.py
index 4d4b590a8..2b821e81a 100644
--- a/lib/portage/__init__.py
+++ b/lib/portage/__init__.py
@@ -9,6 +9,7 @@ VERSION = "HEAD"
 # ===========================================================================
 
 try:
+	import asyncio
 	import sys
 	import errno
 	if not hasattr(errno, 'ESTALE'):
@@ -373,6 +374,9 @@ class _ForkWatcher:
 	@staticmethod
 	def hook(_ForkWatcher):
 		_ForkWatcher.current_pid = _os.getpid()
+		# Force instantiation of a new event loop as a workaround for
+		# https://bugs.python.org/issue22087.
+		asyncio.set_event_loop_policy(asyncio.DefaultEventLoopPolicy())
 
 _ForkWatcher.hook(_ForkWatcher)
 
diff --git a/lib/portage/util/_eventloop/global_event_loop.py b/lib/portage/util/_eventloop/global_event_loop.py
index 21a1d1970..413011178 100644
--- a/lib/portage/util/_eventloop/global_event_loop.py
+++ b/lib/portage/util/_eventloop/global_event_loop.py
@@ -2,11 +2,8 @@
 # Distributed under the terms of the GNU General Public License v2
 
 import portage
-from .EventLoop import EventLoop
 from portage.util._eventloop.asyncio_event_loop import AsyncioEventLoop
 
-
-_MAIN_PID = portage.getpid()
 _instances = {}
 
 
@@ -22,10 +19,6 @@ def global_event_loop():
 		return instance
 
 	constructor = AsyncioEventLoop
-	# If the default constructor doesn't support multiprocessing,
-	# then multiprocessing constructor is used in subprocesses.
-	if not constructor.supports_multiprocessing and pid != _MAIN_PID:
-		constructor = EventLoop
 
 	# Use the _asyncio_wrapper attribute, so that unit tests can compare
 	# the reference to one retured from _wrap_loop(), since they should
-- 
2.26.2

[gentoo-portage-dev] [PATCH] Use default asyncio event loop implementation in API consumer threads

[Zac Medico]

Make the _safe_loop function an alias for the global_event_loop
function, so that the default asyncio event loop implementation
will be used in API consumer threads. This is possible because
global_event_loop has been fixed (bug 758740) to always use
AsyncioEventLoop, and that uses asyncio.get_event_loop() which
returns a new event loop for each thread.

Bug: https://bugs.gentoo.org/758755
Signed-off-by: Zac Medico <zmedico@gentoo.org>
---
 lib/portage/util/futures/_asyncio/__init__.py | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/lib/portage/util/futures/_asyncio/__init__.py b/lib/portage/util/futures/_asyncio/__init__.py
index a902ad895..ce3685709 100644
--- a/lib/portage/util/futures/_asyncio/__init__.py
+++ b/lib/portage/util/futures/_asyncio/__init__.py
@@ -39,6 +39,7 @@ portage.proxy.lazyimport.lazyimport(globals(),
 from portage.util._eventloop.asyncio_event_loop import AsyncioEventLoop as _AsyncioEventLoop
 from portage.util._eventloop.global_event_loop import (
 	global_event_loop as _global_event_loop,
+	global_event_loop as _safe_loop,
 )
 # pylint: disable=redefined-builtin
 from portage.util.futures.futures import (
@@ -241,19 +242,3 @@ def _wrap_loop(loop=None):
 	loop = loop or _global_event_loop()
 	return (loop if hasattr(loop, '_asyncio_wrapper')
 		else _AsyncioEventLoop(loop=loop))
-
-
-def _safe_loop():
-	"""
-	Return an event loop that's safe to use within the current context.
-	For portage internal callers, this returns a globally shared event
-	loop instance. For external API consumers, this constructs a
-	temporary event loop instance that's safe to use in a non-main
-	thread (it does not override the global SIGCHLD handler).
-
-	@rtype: asyncio.AbstractEventLoop (or compatible)
-	@return: event loop instance
-	"""
-	if portage._internal_caller:
-		return _global_event_loop()
-	return _EventLoop(main=False)
-- 
2.26.2

[gentoo-portage-dev] Re: [PATCH] Use default asyncio event loop implementation in API consumer threads

[Zac Medico]

[-- Attachment #1.1: Type: text/plain, Size: 618 bytes --]

On 12/6/20 1:46 AM, Zac Medico wrote:
> Make the _safe_loop function an alias for the global_event_loop
> function, so that the default asyncio event loop implementation
> will be used in API consumer threads. This is possible because
> global_event_loop has been fixed (bug 758740) to always use
> AsyncioEventLoop, and that uses asyncio.get_event_loop() which
> returns a new event loop for each thread.

I think we may still need a separate _safe_loop function here,
since global_event_loop returns a separate loop per pid, but
_safe_loop needs to return a separate loop per thread.
-- 
Thanks,
Zac


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 981 bytes --]

[gentoo-portage-dev] [PATCH v2] Use default asyncio event loop implementation in API consumer threads

[Zac Medico]

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.

Bug: https://bugs.gentoo.org/758755
Signed-off-by: Zac Medico <zmedico@gentoo.org>
---
[PATCH v2] fixed _safe_loop function to return a new
AsyncioEventLoop per thread

 lib/portage/util/futures/_asyncio/__init__.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

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()
-- 
2.26.2

[gentoo-portage-dev] [PATCH v3] Use default asyncio event loop implementation in API consumer threads

[Zac Medico]

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 <zmedico@gentoo.org>
---
[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()
-- 
2.26.2

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

[Zac Medico]

[-- Attachment #1.1: Type: text/plain, Size: 3392 bytes --]

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 <zmedico@gentoo.org>
> ---
> [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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 981 bytes --]

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

[Zac Medico]

[-- Attachment #1.1: Type: text/plain, Size: 3465 bytes --]

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 <zmedico@gentoo.org>
> ---
> [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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 981 bytes --]

[gentoo-portage-dev] [PATCH v4] Use default asyncio event loop implementation in API consumer threads

[Zac Medico]

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 separate  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.

For external API consumers calling from a non-main thread, an
asyncio loop must be registered for the current thread, or else an
error will be raised like this:

  RuntimeError: There is no current event loop in thread 'Thread-1'.

In order to avoid this RuntimeError, the external API consumer
is responsible for setting an event loop and managing its lifecycle.
This code will set an event loop for the current thread:

  asyncio.set_event_loop(asyncio.new_event_loop())

In order to avoid a ResourceWarning, the caller should also close
the corresponding loop before the current thread terminates.

Bug: https://bugs.gentoo.org/758755
Signed-off-by: Zac Medico <zmedico@gentoo.org>
---
[PATCH v4] treat external API consumers the same as interal callers
if they call from the main thread, and document asyncio loop
lifecycle management now required for external API consumers
calling from a non-main thread

 .../util/_eventloop/asyncio_event_loop.py     |  6 ++++-
 lib/portage/util/futures/_asyncio/__init__.py | 26 ++++++++++++++-----
 2 files changed, 24 insertions(+), 8 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..0b35c6daf 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 (
@@ -246,14 +245,27 @@ def _wrap_loop(loop=None):
 def _safe_loop():
 	"""
 	Return an event loop that's safe to use within the current context.
-	For portage internal callers, this returns a globally shared event
-	loop instance. For external API consumers, this constructs a
-	temporary event loop instance that's safe to use in a non-main
-	thread (it does not override the global SIGCHLD handler).
+	For portage internal callers or external API consumers calling from
+	the main thread, this returns a globally shared event loop instance.
+
+	For external API consumers calling from a non-main thread, an
+	asyncio loop must be registered for the current thread, or else an
+	error will be raised like this:
+
+	  RuntimeError: There is no current event loop in thread 'Thread-1'.
+
+	In order to avoid this RuntimeError, the external API consumer
+	is responsible for setting an event loop and managing its lifecycle.
+	This code will set an event loop for the current thread:
+
+	  asyncio.set_event_loop(asyncio.new_event_loop())
+
+	In order to avoid a ResourceWarning, the caller should also close the
+	corresponding loop before the current thread terminates.
 
 	@rtype: asyncio.AbstractEventLoop (or compatible)
 	@return: event loop instance
 	"""
-	if portage._internal_caller:
+	if portage._internal_caller or threading.current_thread() is threading.main_thread():
 		return _global_event_loop()
-	return _EventLoop(main=False)
+	return _AsyncioEventLoop()
-- 
2.26.2