aio-libs / aio-libs/aiojobs

Scheduler.wait_and_close() livelocks at 100% CPU when a shielded future is already done (Python 3.12+)

Aperta
#623 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub
Lingua principale
Python
Stelle
929
Fork
68
Merge medio
1m
PR unite (30g)
2

Descrizione

## Summary

On Python 3.12 and newer, `Scheduler.wait_and_close()` can spin forever without ever yielding to the event loop. It never returns, the timeout guard cannot fire, and the process pegs one core. The window is narrow but entirely reachable in normal use: it needs a future in `_shields` that has *finished*, at a moment when its `_shields.discard` done-callback is queued but has not run yet.

For an aiohttp app this means graceful shutdown can hang indefinitely whenever a shielded background task happens to finish in the loop iteration just before the cleanup context drains.

## Reproducer

Self-contained, no aiohttp:

```python
import asyncio

from aiojobs import Scheduler

async def main():
scheduler = Scheduler()
ev = asyncio.Event()

async def background():
ev.set() # queues main()'s wakeup *ahead of* this task's own done-callbacks
return "ok"

scheduler.shield(background())
await ev.wait()

# background() has finished, but `_shields.discard` is still sitting in the
# ready queue, so the finished future is still registered:
assert len(scheduler._shields) == 1

await scheduler.wait_and_close() # never returns on Python 3.12+
print("clean shutdown")

asyncio.run(main())
```

| Python | result |
|---|---|
| 3.10 | `clean shutdown` |
| 3.11 | `clean shutdown` |
| 3.12 | hangs at 100% CPU |
| 3.13 | hangs at 100% CPU |
| 3.14 | hangs at 100% CPU |

(aiojobs 1.4.0 in every case.)

To confirm it is a livelock rather than a wait, schedule `loop.call_later(1.0, cb)` just before `wait_and_close()` — `cb` never runs. A watchdog thread calling `faulthandler.dump_traceback(all_threads=True)` shows the main thread inside `_scheduler.py:180-181` on every sample.

## Cause

The drain loop:

```python
while self._jobs or self._shields:
gather = asyncio.gather(
*(job._wait() for job in self._jobs),
*self._shields,
return_exceptions=True,
)
await asyncio.shield(gather)
```

The loop's exit condition depends on `_shields.discard`, which is a `call_soon` done-callback and therefore only runs if the body yields to the event loop. The body is not guaranteed to yield:

- CPython 3.12 made `asyncio.gather()` complete **synchronously** when all of its awaitables are already done (python/cpython#104138, gh-104144). Before 3.12 the returned future was always pending at construction.
- `asyncio.shield()` has a long-standing `if inner.done(): return inner` shortcut, so it hands that already-done gather straight back.
- `await` on an already-done future does not suspend.

So on 3.12+ every iteration completes without a single loop turn. The queued `discard` never runs, `_shields` never empties, and the `while` condition stays true forever.

Measured across versions:

```
py3.10: gather.done() right after construction=False shield returned gather itself=False await suspends=True
py3.11: gather.done() right after construction=False shield returned gather itself=False await suspends=True
py3.12: gather.done() right after construction=True shield returned gather itself=True await suspends=False
py3.13: gather.done() right after construction=True shield returned gather itself=True await suspends=False
py3.14: gather.done() right after construction=True shield returned gather itself=True await suspends=False
```

Two consequences worth calling out:

- **The `wait_timeout` guard cannot save it.** `asyncio.timeout()` fires from the event loop, and the loop never gets a turn. The hang is unbounded, not 60 seconds.
- **`_jobs` is immune.** `job._wait()` builds a fresh coroutine on every iteration, so the gather always has at least one not-yet-done child and the `await` is a real suspension point. Only `_shields`, which stores the same future across iterations, can pre-complete the gather.

## Why it is easy to miss

The documented example for `Scheduler.shield()` awaits the returned future. In that shape the bug cannot fire: `_shields.discard` is registered before `_inner_done_callback`, so it always runs before the awaiter is resumed. Reaching the bad state requires the fire-and-forget shape — register the work, do not await it, let something else trigger shutdown later — which is exactly what `aiojobs.aiohttp.shield()` is for, and the only shape available to a handler that must respond immediately and keep working in the background.

## Suggested fix

Either guarantee a loop turn per iteration:

```diff
await asyncio.shield(gather)
+ await asyncio.sleep(0)
```

or make the exit condition independent of pending callbacks by dropping finished futures directly:

```diff
await asyncio.shield(gather)
+ for f in [f for f in self._shields if f.done()]:
+ self._shields.discard(f)
```

I verified both on 3.12/3.13/3.14: each fixes the reproducer above, and each still waits properly for a shielded task that has real work left to do (shutdown takes the full 0.2 s and the task runs to completion). I am not in a position to put a PR together, so I am just reporting it — please treat the diffs as starting points rather than a proposal.

## Environment

- aiojobs 1.4.0
- CPython 3.10.x / 3.11.x / 3.12.13 / 3.13.x / 3.14.x
- macOS (arm64); the logic is not platform-specific, but I only tested macOS

Guida per i contributori

Apri la guida per i contributori

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.