Pull requests / #1563

#1563 fix(serve): release requests after engine READY timeout

open · @hulkbig · 0 comentarios · En GitHub

Server & APIWindowsLinux

Descripción

Fixes #1527

## Summary

The READY timeout kills the shell wrapper in the issue's reproducer, but its `sleep` child can keep stdout open. The reader thread remains blocked in `readline()`, and the request then blocks in `stdout.close()` waiting for that reader's lock. This prevents the existing retry/error path from returning a response.

## What changed

- Start the Strata engine in an owned session on POSIX and clean up its process group before reaping the leader. Group signaling is gated by the unreaped leader and serialized with Popen's wait lock, so a recycled process-group ID cannot be targeted.
- Bound the reader join and defer stdout closure when the reader is still alive. Deferred cleanup captures the old process and reader, so it cannot close a replacement engine's stdout.
- Initialize the existing admission state before the lazy-start return and preserve it across restarts. Raise `EngineDied` for startup failure so HTTP returns 503 with the actual timeout reason rather than 400 with a missing `slot_cv` error.
- Add 16 startup/lifecycle regressions covering timeout cleanup, inherited writers, process-group safety, late READY, retries, HTTP error propagation and recovery. Update the existing lifecycle mock to specify that its reader has stopped.

The existing retry count and backoff are unchanged.

## Validation

On Linux CPU, using the issue's exact bash/`sleep 3600` stub and `STRATA_ENGINE_READY_S=8`:

- Baseline reproduced the hang in stdout closure after the wrapper was killed.
- Fixed cold startup returned the timeout error in 8.002 s.
- A real HTTP request returned 503 with the timeout reason in 54.015 s: three 8-second startup attempts plus two 15-second backoffs.
- After replacing the stub with a working one, the next request returned 200.

Focused tests:

```sh
.venv/bin/python -m unittest serve.test_startup_timeout serve.test_lifecycle serve.test_restart_waiters serve.test_fatal_recovery serve.test_control_cancel serve.test_quiet_cancel serve.test_server.UntimedReads -v
```

55 tests passed. Compile and diff whitespace checks also passed.

Full discovery:

```sh
.venv/bin/python -m unittest discover -s serve -p 'test_*.py'
```

609 tests ran, 11 skipped, with one error in `test_server.AmdTelemetry.test_readings`: this environment returns `None` from `psutil.disk_io_counters()`, which unchanged `serve/telemetry.py` dereferences. The same error reproduced on untouched base `fb58e0d`.

As a supplemental environment-adjusted check, full discovery was repeated with an in-process shim that preserves real disk counters when available and supplies zero read/write counters only when they are `None`. The unchanged rerun passed all 609 tests with 11 skips. Its first run had a sampler-thread-count timing failure in unchanged `test_responses.NoLeakedSampler.test_server_close_ends_the_sampler`. No repository source or test edits were made for the shim. The raw full suite is therefore not a clean pass in this environment.

## Limits

- No GPU, Windows or macOS runtime validation was performed.
- If the leader was already reaped, or Popen does not expose the wait lock, group signaling is conservatively skipped. Deferred pipe cleanup still releases the caller, but surviving descendants may remain until they exit.
- This addresses the startup cleanup/error path; it does not change the stall watchdog or retry policy.

En el sitio

Enlaces a install, modelos, releases.