Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Relocate test suite hack for dealing with leaked AsyncioSelectReactor state #11694

Closed
exarkun opened this issue Sep 27, 2022 · 0 comments · Fixed by #11695
Closed

Relocate test suite hack for dealing with leaked AsyncioSelectReactor state #11694

exarkun opened this issue Sep 27, 2022 · 0 comments · Fixed by #11695

Comments

@exarkun
Copy link
Member

exarkun commented Sep 27, 2022

test_cleanUpThreadPoolEvenBeforeReactorIsRun has a special case for AsyncioSelectReactor:

        if reactor.__class__.__name__ == "AsyncioSelectorReactor":
            self.assertIsNone(reactor.threadpool)
            # ReactorBase.__init__ sets self.crash as a 'shutdown'
            # event, which in turn calls stop on the underlying
            # asyncio event loop, which in turn sets a _stopping
            # attribute on it that's only unset after an iteration of
            # the loop.  Subsequent tests can only reuse the asyncio
            # loop if it's allowed to run and unset that _stopping
            # attribute.
            self.runReactor(reactor)
        else:
            gc.collect()
            self.assertIsNone(threadPoolRef())

This seems to be a consequence of the choice to prefer to re-use the global loop for AsyncioSelectReactor. There are a couple weird things going on here:

  1. The test shuts down a reactor without ever starting it. This is not a realistic usage pattern so we start in a strange place.
  2. The test suite makes many AsyncioSelectReactor instances and they all share the same loop. This is not a realistic usage pattern so we get to an even stranger place.

We should get rid of (1) but that's a separate task and it's not clear how many other obstacles are blocking it. We should figure out how to fix (2) - ideally by not propagating the "there's a global loop" idea further - but that's probably an interface design question for someone who works with asyncio more than I do.

Until we deal with (1) and (2) let's at least clean up this application code by moving the work-around out of this test and into infrastructure code. That way maintainers don't need to concern themselves with this state leakage, either when maintaining this test or writing new tests.

This is in the way of #11685

exarkun added a commit that referenced this issue Sep 28, 2022
Give each AsyncioSelectorReactor its own event loop when running the tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant