[Bug] Large sandbox RPC responses can starve Uvicorn worker healthchecks
- Dominant language
- Python
- Stars
- 485
- Forks
- 81
- Avg merge
- 16h 12m
- Merged PRs (30d)
- 8
Description
**Bug Description**
In a multi-worker Admin proxy deployment, an `/execute`, `/read_file`, or `/run_in_session` request can return an unbounded response from Rocklet. The proxy currently buffers the complete HTTP response and calls `response.json()`. FastAPI then serializes the resulting `RockResponse` again.
For a sufficiently large command output, the `json.loads()` and `json.dumps()` stages can hold the Python GIL long enough to starve Uvicorn's `always_pong` healthcheck thread. Uvicorn 0.38.0 waits five seconds by default and then sends `SIGKILL` to the worker. The in-flight user request is terminated without a response.
`SIGKILL` also bypasses FastAPI lifespan shutdown and Python finalizers. This can leave worker-owned child processes behind. One observed example is the legacy Nacos client in `nacos-sdk-python==2.0.9`: the first config watcher creates a `multiprocessing.Manager()` server, but a killed worker cannot call the Manager finalizer. The Manager is reparented to PID 1 and remains alive.
The affected response path currently has no byte limit:
1. Rocklet captures the complete command `stdout` and `stderr`.
2. Rocklet serializes the complete result as JSON.
3. HTTPX buffers the complete RPC response in the Admin worker.
4. `response.json()` decodes the complete body.
5. FastAPI/Starlette serializes the complete outer `RockResponse` again.
**Steps to Reproduce**
1. Start the Admin proxy with more than one worker and Uvicorn 0.38.0.
2. Create or select a sandbox.
3. Execute a command that writes a very large amount of data to stdout, for example:
```shell
python -c 'import sys; sys.stdout.write("x" * (1024 ** 3))'
```
4. Observe the Uvicorn master and the request result.
5. Optionally trace the master process with:
```shell
strace -tt -T -p -e trace=poll,kill,wait4
```
6. Profile the affected worker with `py-spy --gil --threads`.
**Expected Behavior**
- A single large response must not cause a healthy worker to be mistaken for an unresponsive worker.
- Oversized Rocklet RPC responses should fail with a clear size-limit error before JSON decoding and re-encoding.
- Other requests handled by the worker should not be terminated because one response is too large.
- Worker replacement should not leave worker-owned child processes running indefinitely.
**Actual Behavior**
- The worker healthcheck pong becomes progressively slower and eventually exceeds the default five-second timeout.
- The Uvicorn master sends `SIGKILL` to the worker and starts a replacement.
- The in-flight request is interrupted and produces no normal response log.
- `json.loads()` and `json.dumps()` dominate GIL samples immediately before the kill.
- Worker child processes that depend on Python finalizers, such as the Nacos `multiprocessing.Manager()` server, may remain orphaned under PID 1.
**Error Logs**
Typical Uvicorn replacement messages are:
```text
INFO: Waiting for child process []
INFO: Child process [] died
```
The Uvicorn messages alone do not distinguish a worker that exited by itself from a healthcheck timeout. In the reproduced case, `strace` showed the master polling the worker health pipe for approximately five seconds, followed by:
```text
kill(, SIGKILL) = 0
```
A concurrent GIL profile was almost entirely in these two paths:
```text
httpx.Response.json -> json.loads
Starlette JSONResponse.render -> json.dumps
```
**Environment Information**
- **OS**: Linux container
- **Python Version**: 3.11.14
- **ROCK Version**: current `master`
- **Installation Method**: source installation in a container image
- **Docker Version**: N/A for the Admin proxy process
- **Deployment Type**: distributed, multi-worker Admin proxy
**ROCK Configuration**
- **Runtime Environment Type**: Python virtual environment
- **Sandbox Image**: custom sandbox image
- **Resource Allocation**: multiple Uvicorn proxy workers per pod
- **Uvicorn Version**: 0.38.0
- **Nacos SDK Version**: 2.0.9 when dynamic configuration is enabled
**Component Affected**
- [x] Sandbox
- [ ] Actions
- [ ] Deployments
- [x] SDK & API
- [ ] Envhub
- [ ] CLI
- [x] Performance & Optimization
- [ ] Documentation & Examples
**Proposed Fix**
1. Set Uvicorn's `timeout_worker_healthcheck` to 30 seconds for the ROCK Admin server. This provides operational headroom but is not a substitute for bounding payload size.
2. Add `ROCK_PROXY_MAX_RPC_RESPONSE_BYTES`, defaulting to 128 MiB (`134217728` bytes).
3. After HTTPX returns and before any call to `response.json()`, calculate `len(response.content)`. If the response exceeds the configured limit, log the RPC path, actual byte count, and limit, then return a clear failure.
4. Add tests proving that a response exactly at the limit is accepted and an oversized response is rejected without invoking `response.json()`.
The byte check prevents the expensive Admin-side JSON decode and encode, but HTTPX has already buffered the body at that point. A follow-up improvement should stream the Rocklet response, reject an oversized `Content-Length` early, enforce a cumulative byte limit for chunked responses, and bound stdout/stderr at the Rocklet command-execution source.
The Nacos Manager lifecycle is a separate follow-up: graceful shutdown alone cannot run after `SIGKILL`. The legacy config watcher should avoid creating the unnecessary `multiprocessing.Manager()` server, or explicitly shut it down during normal lifespan teardown while retaining a supervisor-level fallback for forced termination.
**Acceptance Criteria**
- Uvicorn worker healthchecks use a 30-second timeout.
- The default Rocklet RPC response limit is 128 MiB and can be overridden with `ROCK_PROXY_MAX_RPC_RESPONSE_BYTES`.
- Responses larger than the configured limit fail before `response.json()` is called.
- The error and server log include the actual response size and configured limit without logging response contents.
- Existing responses at or below the limit continue to work.
- Unit tests cover the default, environment override, boundary value, and oversized-response path.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start at the Admin proxy handlers for /execute, /read_file, and /run_in_session, then inspect the HTTPX response path before response.json() and the Uvicorn Admin server configuration. Add unit coverage for the default and environment-configured limit, boundary-sized responses, and oversized responses that avoid JSON decoding. Done means the timeout and limit criteria pass without logging response contents.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- fastapi, python
- Domain
- api, backend, performance
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100