Skip to content

Check socket is actually freed and reusable during shutdown daemon. - #1230

Merged
ahcorde merged 1 commit into
rollingfrom
issues/1229
May 14, 2026
Merged

Check socket is actually freed and reusable during shutdown daemon.#1230
ahcorde merged 1 commit into
rollingfrom
issues/1229

Conversation

@fujitatomoya

@fujitatomoya fujitatomoya commented May 14, 2026

Copy link
Copy Markdown
Collaborator

Description

Closes #1229

Is this user-facing behavior change?

Not really, only when the daemon is shutdown, it guarantees that the socket is reusable on the address/port. (only for windows.)

Did you use Generative AI?

Yes, partially Claude Opus 4.7

Additional Information

@fujitatomoya
Check socket is actually freed and reusable during shutdown daemon.
4bdc428 
Signed-off-by: Tomoya Fujita <tomoya.fujita825@gmail.com>
fujitatomoya
fujitatomoya commented May 14, 2026
View reviewed changes
Comment thread ros2cli/ros2cli/node/daemon.py
# non-Windows so TIME_WAIT doesn't make us falsely report busy.
try:
with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s:
if os.name != 'nt':

@fujitatomoya fujitatomoya May 14, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is required because of

ros2cli/ros2cli/ros2cli/xmlrpc/local_server.py

Line 40 in bf77534

allow_reuse_address = False if os.name == 'nt' else True

@fujitatomoya
fujitatomoya requested review from ahcorde and cottsay May 14, 2026 00:22
@fujitatomoya fujitatomoya self-assigned this May 14, 2026
@fujitatomoya fujitatomoya mentioned this pull request May 14, 2026
Open
@ahcorde

ahcorde commented May 14, 2026

Copy link
Copy Markdown
Contributor

Pulls: #1230
Gist: https://gist.githubusercontent.com/ahcorde/7b74e4dc51d59fdf483050ce2a490ee2/raw/e6f0c17107e943084b8573310a2da7fd5c96ad14/ros2.repos
BUILD args: --packages-above-and-dependencies ros2cli
TEST args: --packages-above ros2cli
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/19263

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@ahcorde
ahcorde merged commit 9c05387 into rolling May 14, 2026
3 checks passed
@ahcorde
ahcorde deleted the issues/1229 branch May 14, 2026 20:56
@ahcorde

ahcorde commented May 14, 2026

Copy link
Copy Markdown
Contributor

@fujitatomoya do we need to backport this ?

@fujitatomoya

fujitatomoya commented May 14, 2026

Copy link
Copy Markdown
Collaborator Author

i say yes. although these test failures are exposed with current rolling, i believe this is good for test stability.

@fujitatomoya

fujitatomoya commented May 14, 2026

Copy link
Copy Markdown
Collaborator Author

@Mergifyio backport lyrical

@mergify

mergify Bot commented May 14, 2026 •
edited
Loading

Copy link
Copy Markdown

backport lyrical

✅ Backports have been created

Details

@mergify mergify Bot mentioned this pull request May 14, 2026
Merged
ahcorde pushed a commit that referenced this pull request May 15, 2026
@mergify Tomoya Fujita
Check socket is actually freed and reusable during shutdown daemon. (#…
fb2d293 
…1230) (#1231)

(cherry picked from commit 9c05387)

Signed-off-by: Tomoya Fujita <tomoya.fujita825@gmail.com>
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ahcorde ahcorde ahcorde approved these changes
@cottsay cottsay Awaiting requested review from cottsay

Assignees

@fujitatomoya fujitatomoya

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

🧑‍🌾 ros2cli_daemon tests failing in win rel because it can't spawn daemons

2 participants

@fujitatomoya @ahcorde