Repository navigation
fix: make repeated RemDial calls in one process safe - #14
Merged
Merged
Conversation
A second RemDial in the same process must never wedge the caller or corrupt the first channel: - the FFI RemDial and the REM proxy client waited on agent Init with no exit condition; a HandlerInit failure (e.g. the local endpoint is already bound by another agent in the process) closes the agent without ever setting Init, hanging the caller forever. Both now wait via agent.WaitInit, which also fails on agent close and on timeout. - the handler-exit path deleted the agent's registry entry unconditionally, which could evict a same-alias successor registered after Close ran; it now uses CompareAndDelete. Add runner-level regression tests for the second-dial contract: a dial whose alias is still registered is refused with a controlled error instead of panicking in newAgent, and a re-dial after the first agent closes re-establishes the channel cleanly. Add WaitInit unit tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug: Calling
RemDial(FFI) or otherwise dialing a second agent console in the same process can kill or permanently wedge the caller.agent.Agentsregistry (e.g. a re-dial reusing the same alias) makesagent.NewAgentreturn(nil, err).runner.(*Console).newAgentignored the error and immediately assigneda.Recoveron the nil agent — a nil-pointer panic that takes down the entire host process. (Already mitigated on master by honoring the error; this PR adds regression coverage pinning that contract.)RemDialexport and theREM://proxy client — wait for the dialed agent to finish synchronous init withfor { if a.Init { break } }. AHandlerInitfailure (for example, the local listener endpoint requested by the second dial is already bound by the first agent in the same process) closes the agent without ever settingInit, so the wait loops forever and the calling thread/task is never answered.Agents.Map.Delete(a.ID). If a same-alias successor registered afterCloseran, this evicts the live successor's registry entry.Fix:
agent.(*Agent).WaitInit(timeout): blocks untilInit, agent close, or timeout, and reports whetherInitwas reached. Both wait sites now fail the dial (with a bounded 30s backstop) instead of waiting forever, closing the agent on timeout.DeletewithCompareAndDeleteso only the exiting agent's own entry is removed.Tests:
runner/redial_test.go: a second dial while the first agent's alias is still registered is refused with a controlled error (no panic; the incumbent keeps its registry entry and stays open) — this test panics with the original nil dereference when run against the pre-fixnewAgent; and a re-dial with the same alias after the first agent closes re-establishes the channel end-to-end against a live server console.agent/waitinit_test.go:WaitInitreturns on init, on close-before-init (the former hang), and on timeout.