Skip to content

Fix Windows IOCP TcpClient: Init() returns true even when bind fails#3

Merged
lmshao merged 1 commit into
masterfrom
copilot/fix-actions-failure-issue
May 10, 2026
Merged

Fix Windows IOCP TcpClient: Init() returns true even when bind fails#3
lmshao merged 1 commit into
masterfrom
copilot/fix-actions-failure-issue

Conversation

Copilot AI commented May 10, 2026

Copy link
Copy Markdown
Contributor

TcpTest.InitBindFailureInvalidatesClientSocket was failing on Windows IOCP because Init() always returned true even when the local-address bind() inside ReInit() failed (WSAEADDRINUSE / error 10048).

Root cause

ReInit() logged the bind error and returned early, but neither called closesocket() nor set socket_ = INVALID_SOCKET. Init() then unconditionally set isRunning_ = true and returned true with a live (unbound) socket — violating the expected post-condition.

The same missing cleanup existed on the inet_pton remote-address failure path.

Changes — src/platforms/windows/tcp_client_impl.cpp

  • ReInit() — every early-return error path now calls closesocket(socket_) and sets socket_ = INVALID_SOCKET before returning:

    • user-specified bind() failure
    • auto-bind for ConnectEx failure
    • inet_pton remote-address failure
      (IOCP association failure already did this correctly — now consistent throughout)
  • Init() — checks socket_ == INVALID_SOCKET after ReInit() and returns false with taskQueue_->Stop() cleanup:

ReInit();
if (socket_ == INVALID_SOCKET) {
    LMNET_LOGE("Failed to initialize socket");
    taskQueue_->Stop();
    return false;
}

@lmshao lmshao marked this pull request as ready for review May 10, 2026 01:09
Copilot AI review requested due to automatic review settings May 10, 2026 01:09

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Fixes a Windows IOCP TcpClientImpl initialization bug where Init() could return success even when ReInit() failed (e.g., local bind() failure), leaving the client in an invalid-but-“running” state.

Changes:

  • Ensure ReInit() closes the socket and marks it INVALID_SOCKET on early-return error paths (bind failures, remote inet_pton failure).
  • Make Init() validate socket_ after ReInit() and return false (with taskQueue_->Stop() cleanup) when initialization fails.

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

Comment on lines 123 to +133
localAddr_.sin_family = AF_INET;
localAddr_.sin_port = htons(localPort_);
if (localIp_.empty()) {
localIp_ = "0.0.0.0";
}
inet_pton(AF_INET, localIp_.c_str(), &localAddr_.sin_addr);

if (bind(socket_, (sockaddr *)&localAddr_, sizeof(localAddr_)) != 0) {
LMNET_LOGE("bind failed: %d", WSAGetLastError());
closesocket(socket_);
socket_ = INVALID_SOCKET;
@lmshao lmshao merged commit 424c56f into master May 10, 2026
9 of 12 checks passed
@lmshao lmshao deleted the copilot/fix-actions-failure-issue branch May 10, 2026 01:12
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.

3 participants