Skip to content

gh-128002: use _PyObject_SetMaybeWeakref when creating tasks in asyncio #128885

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

Merged
merged 5 commits into from
Jan 24, 2025

Conversation

kumaraditya303
Copy link
Contributor

@kumaraditya303 kumaraditya303 commented Jan 15, 2025

@colesbury
Copy link
Contributor

Can you add a test for this? I think something like the following would catch the bug:

  1. Create some tasks in the main thread
  2. Call all_tasks() in a different thread
  3. Check that you get all the tasks

@kumaraditya303
Copy link
Contributor Author

I think the following code appears to catch that bug:

import asyncio
import threading


def func():
    loop = asyncio.EventLoop()

    async def coro():
        await asyncio.sleep(0.01)

    lock = threading.Lock()
    count = 0
    async def main():
        nonlocal count
        while True:
            with lock:
                asyncio.create_task(coro())
                count = len(asyncio.all_tasks(loop))

    th = threading.Thread(target=loop.run_until_complete, args=(main(),))
    th.start()

    def check():
        with lock:
            assert count == len(asyncio.all_tasks(loop))

    threads = [threading.Thread(target=check) for _ in range(10)]
    for t in threads:
        t.start()

    for t in threads:
        t.join()


func()

Without the fix, I see many assertion errors and with it there isn't any.

@kumaraditya303
Copy link
Contributor Author

@colesbury Added test based on the above code.

@kumaraditya303
Copy link
Contributor Author

I have tweaked the test to be a little less strict when comparing, it still tests the bug.

Without fix:

./python -m test test_asyncio.test_free_threading -F
Using random seed: 2944425156
0:00:00 load avg: 0.61 Run tests sequentially in a single process
0:00:00 load avg: 0.61 [  1] test_asyncio.test_free_threading
Warning -- Uncaught thread exception: AssertionError
Exception in thread Thread-5 (check):
Warning -- Uncaught thread exception: AssertionError
Exception in thread Thread-2 (check):
Traceback (most recent call last):
Traceback (most recent call last):
  File "/home/realkumaraditya/cpython/Lib/threading.py", line 1054, in _bootstrap_inner
    self.run()
    ~~~~~~~~^^
  File "/home/realkumaraditya/cpython/Lib/threading.py", line 996, in run
    self._target(*self._args, **self._kwargs)
    ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/realkumaraditya/cpython/Lib/test/test_asyncio/test_free_threading.py", line 86, in check
    self.assertSetEqual(tasks & self.all_tasks(loop), tasks)
    ~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/realkumaraditya/cpython/Lib/unittest/case.py", line 1176, in assertSetEqual
    self.fail(self._formatMessage(msg, standardMsg))
    ~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/realkumaraditya/cpython/Lib/unittest/case.py", line 741, in fail
    raise self.failureException(msg)
AssertionError: Items in the second set but not the first:
<Task pending name='Task-1' coro=<TestFreeThreading.test_all_tasks_different_thread.<locals>.main() running at /home/realkumaraditya/cpython/Lib/test/test_asyncio/test_free_threading.py:77> cb=[_run_until_complete_cb() at /home/realkumaraditya/cpython/Lib/asyncio/base_events.py:181]>

@kumaraditya303 kumaraditya303 merged commit 8e0b360 into python:main Jan 24, 2025
40 checks passed
@kumaraditya303 kumaraditya303 deleted the maybe-weakref branch January 24, 2025 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants