Retry daemon socket acquisition after shutdown tail - #1274
Retry daemon socket acquisition after shutdown tail#1274sylvesterkaczmarek wants to merge 3 commits into
Conversation
Distinguish a live daemon from a temporarily occupied daemon address. When a spawn timeout is provided and the daemon is no longer serving, wait for the address to become reusable and retry the bind once. Add focused tests for live-daemon, shutdown-tail, and non-blocking behavior. Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com>
|
Tick the box to add this pull request to the merge queue (same as
|
fujitatomoya
left a comment
fujitatomoya
left a comment
There was a problem hiding this comment.
@sylvesterkaczmarek did you actually try this PR with windows that reproduce the issue?
| # On Windows a daemon can stop serving before its process releases the | ||
| # listening socket. Give that shutdown tail time to finish, then retry the | ||
| # bind. Another process may win the race, in which case it owns the socket. | ||
| if not wait_for(_is_daemon_address_free, timeout): |
There was a problem hiding this comment.
full-timeout stall (and possible infinite wait) when a foreign process holds the daemon port?
the daemon address is a fixed loopback port derived from ROS_DOMAIN_ID; any unrelated process can hold it.
if that happens, is_daemon_running() is false, and wait_for(_is_daemon_address_free, timeout) polls a predicate that can never become true.
the CLI blocks for the entire spawn timeout, then returns the same false the pre-PR code returned immediately though. this could be worse for UX, the module's documented timeout convention
Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com>
Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com>
Thanks. I addressed the port-owner concern by making the retry Windows-only and bounding it to a 1-second socket-release grace period rather than the caller's full or indefinite spawn timeout. I also added regression coverage for a foreign Windows port owner. I have not reproduced the original failure on a Windows host myself, so I am not claiming that verification. I would still want Windows CI/build-farm validation before merge. |
2 participants
Summary
Addresses the remaining failure mode in #1229.
shutdown_daemon()now waits for the XML-RPC address to become reusable, but a daemon that exits through its inactivity timeout can still stop serving before the process releases the listening socket. During that window,spawn_daemon()currently interpretsEADDRINUSEas an already-running daemon and returnsFalse.When a spawn timeout is provided, distinguish that shutdown-tail case from a live daemon, wait for the address to become reusable, and retry the bind once.
Existing behavior is preserved when:
Testing
Added focused unit tests for:
Did you use Generative AI?
Yes. AI was used to assist with tests.