Skip to content

fix SetHostSocket when add stream failed#3386

Merged
chenBright merged 1 commit into
apache:masterfrom
lo-tk:fix_SetHostSocket
Jul 16, 2026
Merged

fix SetHostSocket when add stream failed#3386
chenBright merged 1 commit into
apache:masterfrom
lo-tk:fix_SetHostSocket

Conversation

@lo-tk

@lo-tk lo-tk commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: introduced by PR #2938 .

Problem Summary:

Stream::SetHostSocket performs host-socket binding inside std::call_once, which makes the first execution result effectively global for all later callers. If AddStream(id()) fails during that first attempt, the lambda returns without setting _host_socket, but the once_flag is still consumed. After that, subsequent callers cannot retry the binding logic and may only observe the post-state through the function return value. This creates a broken state where the stream is not actually attached to any host socket, while later code paths still assume the binding has succeeded, eventually triggering _host_socket == NULL checks or null-dereference style crashes.

Changed:

return _host_socket != NULL ? 0 : -1;

Side effects:

  • Performance effects:

  • Breaking backward compatibility:


Check List:

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adjusts brpc::Stream::SetHostSocket() (in src/brpc/stream.cpp) so it no longer reports success unconditionally after the std::call_once block, and instead returns failure when _host_socket was not actually bound—addressing a broken state that can occur when AddStream(id()) fails during the first (and only) call_once attempt.

Changes:

  • Change Stream::SetHostSocket() to return 0 only when _host_socket is non-null, otherwise -1.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/brpc/stream.cpp
Comment thread src/brpc/stream.cpp

@chenBright chenBright left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@chenBright chenBright merged commit 2e20ccb into apache:master Jul 16, 2026
15 checks passed
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.

4 participants