Skip to content

fix: set socket buffer options before connect#3368

Open
howzi wants to merge 1 commit into
apache:masterfrom
howzi:fix/socket-buffer-options-before-connect
Open

fix: set socket buffer options before connect#3368
howzi wants to merge 1 commit into
apache:masterfrom
howzi:fix/socket-buffer-options-before-connect

Conversation

@howzi

@howzi howzi commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

现有的tcp SNDBUF和RCVBUF设置时机有问题,不仅会导致设置失败,测试中发现还会导致buf固定位非预期的值。

参考:
https://man7.org/linux/man-pages/man7/tcp.7.html
image

@howzi howzi force-pushed the fix/socket-buffer-options-before-connect branch from 51bfd0e to 308fadf Compare July 3, 2026 09:56
@wwbmmm wwbmmm requested a review from Copilot July 5, 2026 02:07

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 fixes the timing of applying TCP socket buffer size options (SO_SNDBUF/SO_RCVBUF) so they take effect reliably by setting them before connect() on client sockets and before bind()/listen() on server listening sockets (so accepted sockets inherit the sizes).

Changes:

  • Refactors socket buffer sizing into SetSocketBufferOptions(int fd) and applies it before connect() in Socket::Connect.
  • Extends butil::tcp_listen with an optional callback invoked before bind()/listen() and uses it from Server::StartInternal to set listener buffer sizes early.
  • Adds unit tests verifying buffer sizes are set before connect and inherited on accept.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/brpc_socket_unittest.cpp Adds helper routines and tests to validate buffer option timing/inheritance.
src/butil/endpoint.h Adds BeforeListenCallback type + tcp_listen overload taking a callback.
src/butil/endpoint.cpp Implements callback-aware tcp_listen and keeps the original wrapper overload.
src/brpc/socket.h Declares SetSocketBufferOptions(int fd) helper.
src/brpc/socket.cpp Moves buffer sizing into SetSocketBufferOptions and calls it before connect().
src/brpc/server.cpp Passes SetSocketBufferOptions into tcp_listen so listener fds get sized early.

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

Comment on lines +1158 to +1161
GetSocketBufferValues(reference_fd, expected);

ASSERT_NE(default_values.recv_buffer, expected->recv_buffer);
ASSERT_NE(default_values.send_buffer, expected->send_buffer);
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.

2 participants