Retain the chunk-received trace task in read_nowait

[noron12234]

What

read_nowait schedules the chunk-received trace hook and drops the task, which the TODO on that line already flags:

chunk = self._read_nowait(n)
if chunk and (cb := self._on_chunk_received) is not None:
    # read_nowait is sync but the hook is async; schedule it so the
    # observability event still fires.
    # TODO: Save and await this task.
    asyncio.create_task(cb(chunk))  # type: ignore[unused-awaitable]

The loop only keeps a weak reference, so the task can be garbage collected before the hook runs. Chunk tracing then loses events with no error and no log line — the failure is a missing datapoint, which is the hardest kind to notice, and it gets more likely exactly when the loop is busiest.

What this PR does — and does not — do

This is only the "save" half of that TODO.

The async paths already handle this properly through _fire_chunk_received, which also bounds a hung handler with the per-stream timer:

async def _fire_chunk_received(self, chunk: bytes) -> None:
    cb = self._on_chunk_received
    assert cb is not None
    # Run under the same per-stream timer that _wait() uses, so a hung
    # trace handler is bounded by sock_read just like a hung socket read would be.
    with self._timer:
        await cb(chunk)

read_nowait cannot use it because it is synchronous. Awaiting the hook there would need a sync/async split of read_nowait, and the fire-and-forget task also bypasses the self._timer bound the async paths get. Both are design calls for the maintainers, not something to decide in a drive-by PR — so the TODO is narrowed rather than removed, and now names the remaining piece:

# TODO: this still bypasses the `self._timer` bound that
# _fire_chunk_received applies; awaiting it needs a sync/async split.
task = asyncio.create_task(cb(chunk))
self._chunk_received_tasks.add(task)
task.add_done_callback(self._chunk_received_tasks.discard)

_chunk_received_tasks is added to __slots__ and initialised next to _on_chunk_received. discard rather than remove so a double callback cannot raise; the set stays bounded by the number of in-flight hooks.

The # type: ignore[unused-awaitable] is gone, since the result is bound now.

Verification

$ black --check aiohttp/streams.py     # black 26.5.1, pinned in .pre-commit-config.yaml
1 file would be left unchanged.
$ isort --check-only aiohttp/streams.py
$ python -m py_compile aiohttp/streams.py

No test: the failure is a garbage-collection race, so a test would have to force a GC at a chosen moment and assert a task did not vanish — flaky by construction. Happy to add one if you have a shape in mind.

Found with an AST scan for create_task / ensure_future results discarded as bare expression statements (excluding TaskGroup.create_task, which does hold strong references). This was the only hit in the package.

[noron12234]

Pushed 2ef5141 with two follow-ups.

The holding set is now allocated lazily. A StreamReader is constructed per response, but _on_chunk_received is only set when client tracing is enabled, so an eager set() in __init__ charged every response for a feature most of them never use. On this machine that is 46ns per construction, roughly +40% on the object construction itself:

set()  : 160.7 ns/construction
None   : 114.8 ns/construction

It is now built on first use, so the common path allocates nothing.

Added two tests, both of which fail against the current upstream streams.py and pass with this branch:

• test_read_nowait_holds_the_task_then_releases_it — the task is strongly referenced while pending and dropped once it finishes, so keeping the reference cannot become a leak. Worth noting the discard lands one tick after completion, because add_done_callback is delivered via call_soon.
• test_no_hook_does_not_allocate_the_task_set — pins the lazy behaviour so it cannot regress back to an eager allocation.

tests/test_streams.py is 138 passed locally, black --check clean, and mypy aiohttp/streams.py reports nothing for the file.

On the red Benchmark check — that job is not a performance regression. The run completed 84 passed, 4931 deselected in 445.45s and then failed in the Uploading results step with Request failed after 3 retries, so no comparison against base was ever produced. Happy to rerun it if that is useful.

[codecov]

Codecov Report

❌ Patch coverage is 95.45455% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 98.98%. Comparing base (073ee84) to head (2ef5141).
⚠️ Report is 2 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
aiohttp/streams.py	83.33%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #13300      +/-   ##
==========================================
- Coverage   98.98%   98.98%   -0.01%     
==========================================
  Files         132      132              
  Lines       49023    49076      +53     
  Branches     2551     2553       +2     
==========================================
+ Hits        48526    48578      +52     
  Misses        373      373              
- Partials      124      125       +1     

Flag	Coverage Δ
Autobahn	22.12% <9.09%> (+<0.01%)	⬆️
CI-GHA	98.90% <95.45%> (-0.01%)	⬇️
OS-Linux	98.67% <95.45%> (-0.01%)	⬇️
OS-Windows	97.02% <95.45%> (-0.01%)	⬇️
OS-macOS	97.92% <95.45%> (-0.02%)	⬇️
Py-3.10	98.11% <95.45%> (-0.01%)	⬇️
Py-3.11	98.37% <95.45%> (-0.01%)	⬇️
Py-3.12	98.46% <95.45%> (-0.01%)	⬇️
Py-3.13	98.44% <95.45%> (-0.01%)	⬇️
Py-3.14	98.46% <95.45%> (-0.01%)	⬇️
Py-3.14t	97.55% <95.45%> (-0.01%)	⬇️
Py-pypy-3.11	97.39% <95.45%> (-0.01%)	⬇️
VM-macos	97.92% <95.45%> (-0.02%)	⬇️
VM-ubuntu	98.67% <95.45%> (-0.01%)	⬇️
VM-windows	97.02% <95.45%> (-0.01%)	⬇️
cython-coverage	37.95% <4.54%> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.