`loop.call_soon_threadsafe()` behaves as non-thread-safe in free-threading

[x42005e1f]

When I tried running rogerbinns/python-async-bench on free-threaded Python 3.14.2 (Linux, dual-core), I found it getting stuck on the uvloop (0.22.1) tests. Below is the code to reproduce the issue and its possible output.

#!/usr/bin/env python3

import asyncio
import threading
import time

from queue import SimpleQueue

import uvloop

START_TIME = time.monotonic()


def callback(future):
    future.set_result(True)
    print(f"time: {time.monotonic() - START_TIME:.10f} (callback)")


def notify_all(loop, queue):
    while (future := queue.get()) is not None:
        loop.call_soon_threadsafe(callback, future)


async def main():
    loop = asyncio.get_running_loop()
    queue = SimpleQueue()
    thread = threading.Thread(target=notify_all, args=[loop, queue])
    thread.start()

    try:
        async with asyncio.timeout(60):
            while True:
                future = loop.create_future()

                print(f"time: {time.monotonic() - START_TIME:.10f}")

                try:
                    async with asyncio.timeout(3):
                        outer_future = asyncio.shield(future)
                        queue.put(future)
                        await outer_future
                except TimeoutError:
                    print("[timeout] future:", future)
                    raise
    except TimeoutError:
        pass
    finally:
        queue.put(None)

    await asyncio.to_thread(thread.join)


if __name__ == "__main__":
    uvloop.run(main())

...
time: 0.1763970610
time: 0.1764364019 (callback)
time: 0.1764616738
time: 0.1765011698 (callback)
time: 0.1765267970
time: 3.1786725279 (callback)
[timeout] future: <Future finished result=True>

This looks like a race condition. As we can see, the handle was successfully added, but was only processed by the _on_idle() method after the timeout. It is probably related to the fact that access to the _ready_len attribute is performed non-atomically from different threads (or, in a simpler case, self._ready_len = len(self._ready) from the main thread competes with self._ready_len += 1 from the worker thread). This also applies to loop.run_in_executor() and asyncio.to_thread() running on top of it, but it is much more difficult to reproduce the issue with them (due to the greater delay between threads).

Related: #408.

[fantix]

Right, that's a valid guess! We could probably use atomic primitives to fix counters, but I will verify the root cause of the race condition later.

[x42005e1f]

But is there any real point in having the separate self._ready_len, which is updated along with self._ready anyway? Would it not be more reliable to always use len(self._ready)?

[x42005e1f]

I removed _ready_len in favor of len(self._ready) locally, compiled, tested, and the issue was fixed. Of course, before doing so, I also made sure that the issue could be reproduced without any changes. Even if there is some undocumented reason why _ready_len must exist (it was added in 8098eb9), the fact that this solution works may indicate that the issue is indeed caused by a race condition on _ready_len.

[fantix]

Hmm, that's interesting, good job in pinning down the issue! I believe _ready_len was added as an optimization to avoid calling len(self._ready) over and over again. However, given that 1) len(deque) is cheap enough, and 2) its only consumer Loop._on_wake() is not on the hot path (_on_wake is only called on signals and call_soon_threadsafe), I think it's okay to drop _ready_len.

[x42005e1f]

I believe _ready_len was added as an optimization to avoid calling len(self._ready) over and over again

I should note that even so, its impact on execution time is minimal. First, libuv optimizes async handle's calls. Second, it is mainly called only between two _on_idle() calls, that is, between the current batch of callbacks and the new batch of callbacks (which will be scheduled by the current batch or in parallel). In other words, its greatest impact will only be seen when there are no active tasks at all.

However, considering the context, its impact is even lower. Before each self.handler_async.send() call, a handle is added to the queue, which means there are two cases:

1. If _on_idle() starts executing before _on_wake(), and the handle is being processed, then _on_wake() will be redundant. But _on_idle() already makes two len(self._ready) calls, which means that the similar call from _on_wake() will take no more than a third of the execution time of both method calls (in reality, it will most likely be much less, not least because of other operations).
2. Otherwise, _on_wake() will schedule (if not already scheduled) one _on_idle() call. We get the same calculations.

If we add to this model the cost of context switching between threads, the overhead of iterating through the event loop, and the execution time of callbacks (cancellation of which is a very rare occurrence), then the relative time of len(self._ready) will effectively tend toward zero. So, I guess in practice no one will notice the difference.

[x42005e1f]

Below is a benchmark that measures how many iterations the call_soon_threadsafe() call chain can perform in a given time:

#!/usr/bin/env python3

import uvloop

LOOPS = 1
DELAY = 1


def main():
    loop = uvloop.new_event_loop()

    def callback():
        nonlocal iterations

        loop.call_soon_threadsafe(callback)

        iterations += 1

    loop.call_soon(callback)

    for _ in range(LOOPS):
        iterations = 0

        loop.call_later(DELAY, loop.stop)
        loop.run_forever()

        print(iterations)

    loop.close()


if __name__ == "__main__":
    main()

After running this benchmark multiple times for two different builds (with and without _ready_len), I found that the difference in the number of iterations does not exceed the margin of error: 209157±2996 vs 208835±2390. For comparison, for the asyncio event loop, I got 88829±1481.

I also noticed that replacing call_soon_threadsafe() with call_soon() increases the number of iterations by a factor of 5/2. So calling len(self._ready) is far from being a bottleneck here.

[fantix]

Oh, that's beautiful. Let's just get rid of _ready_len. Great work!