_wrap_loop: Prevent redundant AsyncioEventLoop instances #1372
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zmedico
force-pushed
the
bug_938127_wrap_loop_gc_fix
branch
2 times, most recently
from
8cc14d1
to
6f4eea6
Compare
zmedico
force-pushed
the
bug_938127_wrap_loop_gc_fix
branch
3 times, most recently
from
4c7602c
to
5201eb4
Compare
Ultimately the loop arguments that necessitate the _wrap_loop function can be removed, because our aim since bug 761538 should be to eliminate them. Meanwhile, we don't want _wrap_loop to return redundant AsyncioEventLoop instances if we can easily prevent it. Therefore, use _safe_loop(create=False) to look up the AsyncioEventLoop instance associated with the current thread, and avoid creating redundant instances. This serves to guard against garbage collection of AsyncioEventLoop instances which may have _coroutine_exithandlers added by the atexit_register function since commit cb0c09d from bug 937740. If _safe_loop(create=False) fails to associate a loop with the current thread, raise an AssertionError for portage internal API consumers. It's not known whether external API consumers will trigger this case, so if it happens then emit a UserWarning and return a temporary AsyncioEventLoop instance. Fixes: cb0c09d ("Support coroutine exitfuncs for non-main loops") Bug: https://bugs.gentoo.org/938127 Bug: https://bugs.gentoo.org/937740 Bug: https://bugs.gentoo.org/761538 Signed-off-by: Zac Medico <zmedico@gentoo.org>
zmedico
force-pushed
the
bug_938127_wrap_loop_gc_fix
branch
from
5201eb4
to
a62faf9
Compare
thesamesam
approved these changes
Aug 19, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ultimately the loop arguments that necessitate the _wrap_loop function can be removed, because our aim since bug 761538 should be to eliminate them. Meanwhile, we don't want _wrap_loop to return redundant AsyncioEventLoop instances if we can easily prevent it.
Therefore, use
_safe_loop(create=False)to look up the AsyncioEventLoop instance associated with the current thread, and avoid creating redundant instances. This serves to guard against garbage collection of AsyncioEventLoop instances which may have _coroutine_exithandlers added by the atexit_register function since commit cb0c09d from bug 937740.If
_safe_loop(create=False)fails to associate a loop with the current thread, raise an AssertionError for portage internal API consumers. It's not known whether external API consumers will trigger this case, so if it happens then emit a UserWarning and return a temporary AsyncioEventLoop instance.Fixes: #1368 cb0c09d ("Support coroutine exitfuncs for non-main loops")
Bug: https://bugs.gentoo.org/938127
Bug: https://bugs.gentoo.org/937740
Bug: https://bugs.gentoo.org/761538