Skip to content

fix(tp): acknowledge shm reads so rank0 cannot corrupt in-flight RPC (#246)#248

Open
Anai-Guo wants to merge 1 commit into
GeeeekExplorer:mainfrom
Anai-Guo:fix/tp-shm-ack-246
Open

fix(tp): acknowledge shm reads so rank0 cannot corrupt in-flight RPC (#246)#248
Anai-Guo wants to merge 1 commit into
GeeeekExplorer:mainfrom
Anai-Guo:fix/tp-shm-ack-246

Conversation

@Anai-Guo

Copy link
Copy Markdown
Contributor

Summary

Fixes #246.

In tensor-parallel mode (tensor_parallel_size > 1), rank0 dispatches method calls to worker ranks through a shared-memory buffer plus a per-worker multiprocessing.Event. The protocol only had a forward (rank0 → worker) notification: there was no acknowledgement that a worker had finished reading the previous command.

As a result rank0 could return from one call() and immediately write_shm() the next RPC, overwriting the shared buffer while a worker was still inside pickle.loads(self.shm.buf[...]) for the previous command. That corrupts the payload (_pickle.UnpicklingError); once one worker rank dies, the surviving ranks hang in the next NCCL collective until timeout.

Fix

Add a per-worker acknowledgement event that closes the handshake:

  • read_shm (worker): after fully reading and unpickling the payload, self.ack_event.set().
  • write_shm (rank0): before overwriting the buffer, wait() + clear() every worker's ack event, so the buffer is only reused once all workers have consumed the previous command.
  • The ack events are pre-set in LLMEngine so the very first command isn't blocked waiting on a read that hasn't happened yet.

This is the ping-pong the buffer needs: rank0 clears each ack before setting the forward event, and a worker sets its ack only after the read completes, so no signal is lost and no read races an overwrite. No change to the public LLM/LLMEngine API; only ModelRunner's internal constructor gains an ack_event argument, and its sole caller (LLMEngine) is updated.

Testing

  • python -m py_compile on both changed modules.
  • Handshake reasoning verified against the exit path (call("exit") still drains cleanly) and the bootstrap case (pre-set acks let the first write_shm proceed).

🤖 Generated with Claude Code

…eeeekExplorer#246)

In tensor-parallel mode the rank0->worker command channel only had a
forward notification event, so rank0 could overwrite the shared-memory
buffer with the next RPC while a worker was still reading the previous
pickled payload, causing _pickle.UnpicklingError and a subsequent NCCL
hang.

Add a per-worker acknowledgement event: each worker sets it after fully
reading a command, and rank0 waits for all acks before reusing the
buffer. Ack events are pre-set so the first command is not blocked.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tensor parallel shared-memory RPC lacks worker acknowledgement and can corrupt commands

1 participant