Гейт на ВСЕ 34 проводки + запрет вложенных бюджетов (ревью #3460)
All checks were successful
CI Trade-In / changes (pull_request) Successful in 9s
CI Trade-In / browser-tests (pull_request) Has been skipped
CI / changes (pull_request) Successful in 11s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 5m14s
All checks were successful
CI Trade-In / changes (pull_request) Successful in 9s
CI Trade-In / browser-tests (pull_request) Has been skipped
CI / changes (pull_request) Successful in 11s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 5m14s
Сценарный тест ловил одну проводку из 34 — ту, через которую сам и шёл (`geocoder._cache_get`). Мутационный прогон ревьюера: возврат голого `asyncio.to_thread` в 5 из 6 других мест тест НЕ краснит, то есть регресс «кто-то вернул вызов в голый вид» прошёл бы мимо CI в 33 случаях из 34. `test_no_bare_to_thread_over_request_session` читает исходники geocoder и estimator (через `module.__file__`, не по относительному пути — он зависел бы от cwd прогона) и требует нуля живых `asyncio.to_thread(`. Оба модуля сейчас на нуле, поэтому гейт без списка исключений. Фальсификация — голый `to_thread` у `_fetch_anchor_comps` (estimator:4973, сценарным тестом не покрыт): гейт краснеет с номером строки. Второе: защита `run_db_thread` одноразовая — `except asyncio.CancelledError` ловит ОДНУ отмену, вторая вылетает из самого `asyncio.wait([step])`, и поток остаётся сиротой. Живых путей нет (`_with_budget` нигде не вложен, Starlette не отменяет задачу на дисконнекте, uvicorn стартует без `--timeout-graceful-shutdown`), поэтому кода не трогаю — фиксирую инвариант «не вкладывать бюджеты» в докстринге `_with_budget`, чтобы вложение не завезли как безобидное. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
c251c02f1e
commit
be9aa2f907
2 changed files with 33 additions and 0 deletions
|
|
@ -3092,6 +3092,12 @@ async def _with_budget(coro: Any, budget_s: float, *, label: str) -> Any:
|
|||
blowing the gateway read timeout (#654: opaque Caddy 502/504).
|
||||
|
||||
budget_s <= 0 disables the guard (await directly) — escape hatch via config.
|
||||
|
||||
НЕ ВКЛАДЫВАТЬ бюджеты друг в друга: защита `run_db_thread` (#3449) одноразовая
|
||||
— она ловит ОДНУ отмену, а вторая, прилетевшая пока шаг БД дожидается своего
|
||||
потока, вылетает из самого ожидания, и поток остаётся сиротой в чужой `Session`
|
||||
(ровно то, ради чего защита и заведена). Сейчас ни один вызов `_with_budget` не
|
||||
обёрнут другим — это инвариант, а не совпадение.
|
||||
"""
|
||||
if budget_s is None or budget_s <= 0:
|
||||
return await coro
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@ from __future__ import annotations
|
|||
|
||||
import asyncio
|
||||
import os
|
||||
import pathlib
|
||||
import threading
|
||||
import time
|
||||
from typing import Any
|
||||
|
|
@ -86,3 +87,29 @@ async def test_geocode_budget_cancel_does_not_leave_orphan_in_session(
|
|||
"персист вошёл в сессию, пока осиротевший поток геокодера ещё работал в ней: "
|
||||
"два потока в одной Session → «another operation is in progress» на персисте"
|
||||
)
|
||||
|
||||
|
||||
def test_no_bare_to_thread_over_request_session() -> None:
|
||||
"""Source-гейт: в геокодере и эстиматоре не осталось голых `asyncio.to_thread(`.
|
||||
|
||||
Тест выше ловит ОДНУ проводку — ту, через которую идёт сценарий. Остальные 33
|
||||
(`_cache_put`, `_fetch_anchor_comps`, персист, …) он не видит: возврат любой из
|
||||
них в голый вид прошёл бы мимо CI. Оба модуля сейчас на нуле по живым вызовам,
|
||||
поэтому гейт — ровно «ноль», без списка исключений. Понадобится вызов со СВОЕЙ
|
||||
сессией (как `user_events.record_event`) — заводить его в отдельном модуле или
|
||||
менять этот тест осознанно.
|
||||
|
||||
Читаем через `module.__file__`: относительный путь зависел бы от cwd прогона.
|
||||
"""
|
||||
for module in (geo, est):
|
||||
src = pathlib.Path(module.__file__ or "").read_text(encoding="utf-8")
|
||||
bare = [
|
||||
f"{i}: {line.strip()}"
|
||||
for i, line in enumerate(src.splitlines(), 1)
|
||||
if "asyncio.to_thread(" in line and not line.lstrip().startswith("#")
|
||||
]
|
||||
assert not bare, (
|
||||
f"{module.__name__}: голый asyncio.to_thread по сессии запроса — "
|
||||
f"отмена оставит сироту в чужой Session (#3449), нужен run_db_thread:\n"
|
||||
+ "\n".join(bare)
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue