fix(api): stop synchronous route work from stalling the whole server

docs/src/content/docs/contributing/blocking-work-in-api-routes.md

@@ -0,0 +1,104 @@
+---
+title: Blocking Work in API Routes
+---
+
+Almost every service in the backend is synchronous — the database layer, the model
+manager, the file stores. The API layer in front of them is asynchronous. Getting the
+boundary between the two wrong does not produce a slow endpoint; it produces a server
+that stops answering entirely.
+
+## The rule
+
+**A route handler that only calls synchronous services must be declared `def`, not
+`async def`.**
+
+```python
+# Correct — Starlette runs this in a worker thread.
+@gallery_router.get("/items/names")
+def get_gallery_item_names(current_user: CurrentUserOrDefault) -> GalleryItemNamesResult:
+    return ApiDependencies.invoker.services.gallery.list_item_names(...)
+```
+
+```python
+# Wrong — the database query runs on the event loop.
+@gallery_router.get("/items/names")
+async def get_gallery_item_names(current_user: CurrentUserOrDefault) -> GalleryItemNamesResult:
+    return ApiDependencies.invoker.services.gallery.list_item_names(...)
+```
+
+The same rule applies to **dependencies**, not just handlers. A dependency declared
+`async def` that performs a synchronous database lookup blocks the loop on every request
+that uses it.
+
+## Why it matters
+
+The server runs as a single process with a single event loop. Anything executed directly
+on that loop has the whole process to itself until it returns. Blocking work on the loop
+therefore does not just delay its own response — for its entire duration the process
+serves **no** other HTTP request and delivers **no** socket.io event. Users do not
+experience this as one slow endpoint; they experience it as the application freezing,
+typically mid-generation, because progress events stop arriving too.
+
+The cost scales with the user's library, not with the developer's. A gallery query that
+returns in milliseconds against a test database can take minutes against a multi-gigabyte
+one — for example a metadata search, which has to read every row's metadata blob.
+
+Declaring the handler `def` makes FastAPI dispatch it to a worker thread instead, leaving
+the loop free to serve everything else.
+
+## When `async def` is right
+
+Use `async def` when the body actually awaits something — streaming a response, awaiting
+another async API, or coordinating tasks. If such a handler *also* performs blocking work,
+that work must be wrapped explicitly:
+
+```python
+from starlette.concurrency import run_in_threadpool
+
+user = await run_in_threadpool(ApiDependencies.invoker.services.users.get, user_id)
+```
+
+`async def` with no `await` in the body is always a mistake: it gains nothing and costs
+the loop.
+
+## What this does not fix
+
+Moving work to the threadpool does not make it faster, and it does not make it parallel.
+The SQLite layer uses a single connection behind a process-wide lock, so database work
+remains serialized regardless of which thread requests it. The benefit is confined to —
+and this is the point — keeping everything *else* responsive while it runs.
+
+It also does not make the responsiveness unbounded. Starlette dispatches `def` handlers
+through anyio's thread limiter, which holds **40 tokens by default**. Forty concurrent
+blocking requests occupy every worker, and the forty-first waits for a free one — as does
+anything else that needs a thread, including the synchronous auth dependency that runs
+before a handler is even reached. So the stall does not vanish past that point, it moves:
+from "one slow request freezes the server" to "the server keeps up until forty of them are
+in flight at once". Note that `test_event_loop_blocking.py` probes `/api/v1/app/version`,
+which has no auth dependency and no database access, so it would not show this.
+
+Getting past that bound is not a matter of raising the token count — the single-connection
+lock below it is the real ceiling. It is the reason a route that can block for minutes
+(model conversion, a git clone) is worth serializing explicitly rather than letting an
+arbitrary number of them pile into the pool.
+
+## Testing it
+
+Two tests cover this, and they do different jobs.
+
+`tests/app/routers/test_no_blocking_async_routes.py` **enforces the rule**: it parses every
+router module and fails if any route handler is `async def` without awaiting anything. This
+is the one that catches a new route — a per-route test cannot, because the route does not
+exist when the test is written.
+
+`tests/app/routers/test_event_loop_blocking.py` **proves the effect** for a few
+representative routes. It stubs a service method to block synchronously, issues a request
+against the route under test, and asserts that an unrelated trivial route still answers
+while that request is in flight.
+
+Note what the second one measures: not the slow request's own duration, which the fix does
+not change, but the latency of other requests during it. A benchmark of the slow endpoint
+alone will show no improvement and is the wrong instrument here.
+
+If you call a route handler directly from a test, call it like the plain function it now is
+— no `await`, no `asyncio.run`.

invokeai/app/api/auth_dependencies.py

@@ -27,7 +27,7 @@ def _validate_token(token: str, invalid_detail: str) -> TokenData:
     return token_data
 
 
-async def get_current_user(
+def get_current_user(
     credentials: Annotated[HTTPAuthorizationCredentials | None, Depends(security)],
 ) -> TokenData:
     """Get current authenticated user from Bearer token.
@@ -76,7 +76,7 @@ async def get_current_user(
     return token_data
 
 
-async def get_current_user_or_default(
+def get_current_user_or_default(
     credentials: Annotated[HTTPAuthorizationCredentials | None, Depends(security)],
 ) -> TokenData:
     """Get current authenticated user from Bearer token, or return a default system user if not authenticated.
@@ -128,7 +128,7 @@ async def get_current_user_or_default(
     return token_data
 
 
-async def get_current_media_user_or_default(
+def get_current_media_user_or_default(
     credentials: Annotated[HTTPAuthorizationCredentials | None, Depends(security)],
     media_token: Annotated[str | None, Cookie(alias=MEDIA_TOKEN_COOKIE)] = None,
 ) -> TokenData:
@@ -146,6 +146,11 @@ async def require_admin(
 ) -> TokenData:
     """Require admin role for the current user.
 
+    Stays `async def`, unlike the dependencies it builds on: this only reads a field off the
+    already-resolved token data. Declaring it `def` would buy a threadpool round-trip per admin
+    request and nothing else. The `users.get` that can block lives in `get_current_user`, which
+    is synchronous for that reason.
+
     Args:
         current_user: The current authenticated user's token data
 
@@ -165,6 +170,8 @@ async def require_admin_or_default(
 ) -> TokenData:
     """Require admin role for the current user, or return default system admin in single-user mode.
 
+    `async def` for the same reason as `require_admin`: it does no blocking work of its own.
+
     This dependency is useful for admin-only endpoints that should work in both single-user and multiuser modes.
 
     When multiuser mode is disabled (default), this always returns a system user with admin privileges.

invokeai/app/api/routers/app_info.py

@@ -52,12 +52,12 @@ class AppVersion(BaseModel):
 
 
 @app_router.get("/version", operation_id="app_version", status_code=200, response_model=AppVersion)
-async def get_version() -> AppVersion:
+def get_version() -> AppVersion:
     return AppVersion(version=__version__)
 
 
 @app_router.get("/app_deps", operation_id="get_app_deps", status_code=200, response_model=dict[str, str])
-async def get_app_deps(current_user: CurrentUserOrDefault) -> dict[str, str]:
+def get_app_deps(current_user: CurrentUserOrDefault) -> dict[str, str]:
     deps: dict[str, str] = {dist.metadata["Name"]: dist.version for dist in distributions()}
     try:
         cuda = getattr(getattr(torch, "version", None), "cuda", None) or "N/A"  # pyright: ignore[reportAttributeAccessIssue]
@@ -72,7 +72,7 @@ async def get_app_deps(current_user: CurrentUserOrDefault) -> dict[str, str]:
 
 
 @app_router.get("/patchmatch_status", operation_id="get_patchmatch_status", status_code=200, response_model=bool)
-async def get_patchmatch_status(current_user: CurrentUserOrDefault) -> bool:
+def get_patchmatch_status(current_user: CurrentUserOrDefault) -> bool:
     return PatchMatch.patchmatch_available()
 
 
@@ -212,7 +212,7 @@ def _redact_config_secrets(config: InvokeAIAppConfig) -> InvokeAIAppConfig:
     status_code=200,
     response_model=list[GenerationDeviceOption],
 )
-async def get_generation_device_options(current_user: CurrentUserOrDefault) -> list[GenerationDeviceOption]:
+def get_generation_device_options(current_user: CurrentUserOrDefault) -> list[GenerationDeviceOption]:
     """List the devices available for generation, for use with the `generation_devices` setting."""
     options: list[GenerationDeviceOption] = []
     if torch.cuda.is_available():
@@ -233,7 +233,7 @@ async def get_generation_device_options(current_user: CurrentUserOrDefault) -> l
 @app_router.get(
     "/runtime_config", operation_id="get_runtime_config", status_code=200, response_model=InvokeAIAppConfigWithSetFields
 )
-async def get_runtime_config(current_admin: AdminUserOrDefault) -> InvokeAIAppConfigWithSetFields:
+def get_runtime_config(current_admin: AdminUserOrDefault) -> InvokeAIAppConfigWithSetFields:
     config = get_config()
     return InvokeAIAppConfigWithSetFields(set_fields=config.model_fields_set, config=_redact_config_secrets(config))
 
@@ -244,7 +244,7 @@ async def get_runtime_config(current_admin: AdminUserOrDefault) -> InvokeAIAppCo
     status_code=200,
     response_model=InvokeAIAppConfigWithSetFields,
 )
-async def update_runtime_config(
+def update_runtime_config(
     _: AdminUserOrDefault,
     changes: UpdateAppGenerationSettingsRequest = Body(description="Writable runtime configuration changes"),
 ) -> InvokeAIAppConfigWithSetFields:
@@ -277,7 +277,7 @@ async def update_runtime_config(
     status_code=200,
     response_model=list[ExternalProviderStatusModel],
 )
-async def get_external_provider_statuses(current_user: CurrentUserOrDefault) -> list[ExternalProviderStatusModel]:
+def get_external_provider_statuses(current_user: CurrentUserOrDefault) -> list[ExternalProviderStatusModel]:
     statuses = ApiDependencies.invoker.services.external_generation.get_provider_statuses()
     return [status_to_model(status) for status in statuses.values()]
 
@@ -288,7 +288,7 @@ async def get_external_provider_statuses(current_user: CurrentUserOrDefault) ->
     status_code=200,
     response_model=list[ExternalProviderConfigModel],
 )
-async def get_external_provider_configs(current_admin: AdminUserOrDefault) -> list[ExternalProviderConfigModel]:
+def get_external_provider_configs(current_admin: AdminUserOrDefault) -> list[ExternalProviderConfigModel]:
     config = get_config()
     return [_build_external_provider_config(provider_id, config) for provider_id in EXTERNAL_PROVIDER_FIELDS]
 
@@ -299,7 +299,7 @@ async def get_external_provider_configs(current_admin: AdminUserOrDefault) -> li
     status_code=200,
     response_model=ExternalProviderConfigModel,
 )
-async def set_external_provider_config(
+def set_external_provider_config(
     _: AdminUserOrDefault,
     provider_id: str = Path(description="The external provider identifier"),
     update: ExternalProviderConfigUpdate = Body(description="External provider configuration settings"),
@@ -330,7 +330,7 @@ async def set_external_provider_config(
     status_code=200,
     response_model=ExternalProviderConfigModel,
 )
-async def reset_external_provider_config(
+def reset_external_provider_config(
     _: AdminUserOrDefault,
     provider_id: str = Path(description="The external provider identifier"),
 ) -> ExternalProviderConfigModel:
@@ -439,7 +439,7 @@ def _remove_external_models_for_provider(provider_id: str) -> None:
     responses={200: {"description": "The operation was successful"}},
     response_model=LogLevel,
 )
-async def get_log_level(current_admin: AdminUserOrDefault) -> LogLevel:
+def get_log_level(current_admin: AdminUserOrDefault) -> LogLevel:
     """Returns the log level"""
     return LogLevel(ApiDependencies.invoker.services.logger.level)
 
@@ -450,7 +450,7 @@ async def get_log_level(current_admin: AdminUserOrDefault) -> LogLevel:
     responses={200: {"description": "The operation was successful"}},
     response_model=LogLevel,
 )
-async def set_log_level(
+def set_log_level(
     current_admin: AdminUserOrDefault,
     level: LogLevel = Body(description="New log verbosity level"),
 ) -> LogLevel:
@@ -464,7 +464,7 @@ async def set_log_level(
     operation_id="clear_invocation_cache",
     responses={200: {"description": "The operation was successful"}},
 )
-async def clear_invocation_cache(current_admin: AdminUserOrDefault) -> None:
+def clear_invocation_cache(current_admin: AdminUserOrDefault) -> None:
     """Clears the invocation cache"""
     ApiDependencies.invoker.services.invocation_cache.clear()
 
@@ -474,7 +474,7 @@ async def clear_invocation_cache(current_admin: AdminUserOrDefault) -> None:
     operation_id="enable_invocation_cache",
     responses={200: {"description": "The operation was successful"}},
 )
-async def enable_invocation_cache(current_admin: AdminUserOrDefault) -> None:
+def enable_invocation_cache(current_admin: AdminUserOrDefault) -> None:
     """Clears the invocation cache"""
     ApiDependencies.invoker.services.invocation_cache.enable()
 
@@ -484,7 +484,7 @@ async def enable_invocation_cache(current_admin: AdminUserOrDefault) -> None:
     operation_id="disable_invocation_cache",
     responses={200: {"description": "The operation was successful"}},
 )
-async def disable_invocation_cache(current_admin: AdminUserOrDefault) -> None:
+def disable_invocation_cache(current_admin: AdminUserOrDefault) -> None:
     """Clears the invocation cache"""
     ApiDependencies.invoker.services.invocation_cache.disable()
 
@@ -494,6 +494,6 @@ async def disable_invocation_cache(current_admin: AdminUserOrDefault) -> None:
     operation_id="get_invocation_cache_status",
     responses={200: {"model": InvocationCacheStatus}},
 )
-async def get_invocation_cache_status(current_admin: AdminUserOrDefault) -> InvocationCacheStatus:
+def get_invocation_cache_status(current_admin: AdminUserOrDefault) -> InvocationCacheStatus:
     """Clears the invocation cache"""
     return ApiDependencies.invoker.services.invocation_cache.get_status()