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

Unsubscribe will not actually cancel the background task #154

Closed
elupus opened this issue Nov 21, 2022 · 3 comments · Fixed by #158
Closed

Unsubscribe will not actually cancel the background task #154

elupus opened this issue Nov 21, 2022 · 3 comments · Fixed by #158

Comments

@elupus
Copy link

elupus commented Nov 21, 2022

The problem

Event unsubscribe will not actually stop the subscription, only the call_later handle will be cancelled, which has likely already executed and create the task.

Noticed in: home-assistant/core#82475 and more specifically when adding: home-assistant/core@c7f0e11

Environment

  • Operating system: Any
  • Python version: Any

Problem-relevant code

Traceback/Error logs

    @pytest.fixture(autouse=True)
    def verify_cleanup(event_loop: asyncio.AbstractEventLoop):
        """Verify that the test has cleaned up resources correctly."""
        threads_before = frozenset(threading.enumerate())
        tasks_before = asyncio.all_tasks(event_loop)
        yield
    
        event_loop.run_until_complete(event_loop.shutdown_default_executor())
    
        if len(INSTANCES) >= 2:
            count = len(INSTANCES)
            for inst in INSTANCES:
                inst.stop()
            pytest.exit(f"Detected non stopped instances ({count}), aborting test run")
    
        threads = frozenset(threading.enumerate()) - threads_before
        for thread in threads:
            assert isinstance(thread, threading._DummyThread)
    
        tasks = asyncio.all_tasks(event_loop) - tasks_before
>       assert not tasks
E       AssertionError: assert not {<Task pending name='Task-19' coro=<_GitHubEventsBaseNamespace.subscribe.<locals>._subscriber() running at /Users/joak...aiogithubapi/namespaces/events.py:132> wait_for=<Future pending cb=[<TaskWakeupMethWrapper object at 0x116c73d60>()]>>}

Additional information

@ludeeus
Copy link
Owner

ludeeus commented Dec 4, 2022

It does cancel all future requests, but it does not force-stop any ongoing requests.

@elupus
Copy link
Author

elupus commented Dec 4, 2022

Yup, meaning during tests these tended to leak over logs to next test. We now stop them manually. But would be better if it cleaned up fully when unloaded.

Its not causing any issues now though, so its up to you if you want to fix it or not.

@ludeeus
Copy link
Owner

ludeeus commented Dec 4, 2022

Ill look into adding a new async method to stop all and only return when everything is done.
That should be added to __aexit__ anyway for those that uses async with ....

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.

2 participants