[huniu20]

A storage unit has one worker thread. recv_multipart and the final send_multipart in _process_one_worker_request were not guarded, so one failure there killed the worker: the proxy kept queueing requests nobody answered, and shutdown() hung in zmq_context.term(). Catch per request so only that caller times out.

Also warn once per hang (with ip:port) when a metrics probe times out, since it shares that worker.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-23T09:03:32.884712Z	bb01112	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bb0111276c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Replace the worker socket after multipart I/O errors

When recv_multipart() or send_multipart() raises after consuming or emitting only part of a multipart message, this handler logs the exception and reuses the same DEALER socket. PyZMQ performs multipart I/O frame by frame, so unread frames or an outstanding SNDMORE envelope remain on that socket; the next request can then treat a payload frame as its identity or append its response to the previous caller's route, producing malformed replies and stranding an additional caller. Discard and recreate the worker socket (and update the poller), or terminate the unit, rather than continuing on a potentially poisoned socket.

Useful? React with 👍 / 👎.

[0oshowero0]

[AI Review]

[P2] Recreate the worker socket after multipart I/O failures

Logging the exception and continuing with the same DEALER socket can leave an unfinished multipart message. The next response may then be appended to the previous response and routed to the wrong client.

I reproduced this by injecting a MemoryError after the routing identity was sent but before the response body was sent: clients A and B both timed out, and A subsequently received a malformed response containing B’s identity, while the worker remained alive.

Please close and recreate the worker socket after multipart I/O failures and update the poller accordingly. The new test raises only after receiving a complete request, so it does not cover failures between frames. A regression test should inject a failure between response frames and verify that the next client receives its own valid response.

[huniu20]

Thanks for your review. Fixed in latest commit: after a failed request the worker closes its socket and connects a fresh one. The test now fails right after the identity frame and checks that the next client gets its own reply and the stranded one receives nothing.

[0oshowero0]

[AI Review]

CI timeout: the replacement worker socket leaks during shutdown after merging main

I reproduced the shutdown hang on the exact CI merge result, c177a574562c6842d44c211521e36137cfc7ac5d. The Run Tests job hit its 15-minute limit; increasing that limit would leave this hang unresolved.

The root cause is an interaction with the worker-loop refactor now on main:

1. _worker_routine() creates socket A and passes it to _worker_loop().
2. After a request fails, _worker_loop() unregisters and closes A, then replaces its local variable with socket B.
3. When the loop returns, the outer cleanup still calls poller.unregister(A). This raises KeyError because A was already unregistered. B remains open, so unit.shutdown() blocks in zmq_context.term().

The new test_worker_survives_a_reply_failing_between_frames reaches exactly this shutdown hang. I reproduced both the stale-socket KeyError and the blocked Context.term() stack.

Recommended fix

Give _worker_loop() ownership of the complete worker-socket lifecycle: creation, connection, poller registration, replacement after request failures, and closing the current socket in a finally block. Keep _worker_routine() as the wrapper that logs unhandled failures and triggers the existing process-abort behavior. Remove its cleanup of the stale socket reference.

The poller is discarded when the loop exits, so final cleanup only needs to close the current socket; it does not need to unregister it. Keep the unregister operation when replacing a socket while the loop is still running.

Validation: an in-memory change moving the current-socket close into _worker_loop() and removing the stale outer cleanup passed 27 related tests. The complete lifecycle consolidation recommended above still needs to be implemented and verified against the merged tree; testing the PR head alone misses this interaction.

[huniu20]

Thanks — fixed in latest, and rebased on main.