From 600eb80df0e444b68cf663368593ec2935e69bdc Mon Sep 17 00:00:00 2001 From: Francis Tsui Date: Wed, 5 Nov 2025 17:53:04 -0800 Subject: [PATCH] Add connect timeout to NearbyClientSocket. PiperOrigin-RevId: 828703670 --- .../platform/implementation/windows/BUILD | 1 + .../windows/nearby_client_socket.cc | 50 ++++++-- .../windows/nearby_client_socket.h | 3 +- .../windows/nearby_client_socket_test.cc | 11 +- .../implementation/windows/wifi_direct.h | 2 +- .../windows/wifi_hotspot_medium.cc | 4 +- .../windows/wifi_hotspot_socket.h | 5 +- .../implementation/windows/wifi_lan.h | 7 +- .../implementation/windows/wifi_lan_medium.cc | 119 ++++++------------ 9 files changed, 98 insertions(+), 104 deletions(-) diff --git a/internal/platform/implementation/windows/BUILD b/internal/platform/implementation/windows/BUILD index 9942aa8a..25cb2405 100644 --- a/internal/platform/implementation/windows/BUILD +++ b/internal/platform/implementation/windows/BUILD @@ -545,6 +545,7 @@ cc_test( ":windows", "//internal/platform:base", "@com_github_protobuf_matchers//protobuf-matchers", + "@com_google_absl//absl/time", "@com_google_googletest//:gtest_main", ], ) diff --git a/internal/platform/implementation/windows/nearby_client_socket.cc b/internal/platform/implementation/windows/nearby_client_socket.cc index 7870d62f..4edfa5fe 100644 --- a/internal/platform/implementation/windows/nearby_client_socket.cc +++ b/internal/platform/implementation/windows/nearby_client_socket.cc @@ -22,6 +22,7 @@ #include #include +#include "absl/time/time.h" #include "internal/flags/nearby_flags.h" #include "internal/platform/byte_array.h" #include "internal/platform/exception.h" @@ -51,7 +52,8 @@ NearbyClientSocket::~NearbyClientSocket() { } } -bool NearbyClientSocket ::Connect(const SocketAddress& server_address) { +bool NearbyClientSocket ::Connect(const SocketAddress& server_address, + absl::Duration timeout) { if (!is_socket_initiated_) { LOG(WARNING) << "Windows socket is not initiated."; return false; @@ -102,14 +104,49 @@ bool NearbyClientSocket ::Connect(const SocketAddress& server_address) { setsockopt(socket_, IPPROTO_TCP, TCP_NODELAY, reinterpret_cast(&flag), sizeof(flag)); + bool has_timeout = (timeout != absl::InfiniteDuration()); + if (has_timeout) { + unsigned long non_blocking = 1; // NOLINT + if (ioctlsocket(socket_, FIONBIO, &non_blocking) == SOCKET_ERROR) { + LOG(WARNING) << "Failed to set socket to non-blocking, error: " + << WSAGetLastError(); + // turn off timeout if we can't set the socket to non-blocking. + has_timeout = false; + } + } if (connect(socket_, server_address.address(), sizeof(sockaddr_storage)) == SOCKET_ERROR) { - LOG(ERROR) << "Failed to connect socket with error: " << WSAGetLastError(); - closesocket(socket_); - socket_ = INVALID_SOCKET; - return false; + bool connected = false; + if (has_timeout && WSAGetLastError() == WSAEWOULDBLOCK) { + // Wait until timeout or socket is connected. + timeval tm = absl::ToTimeval(timeout); + fd_set set; + FD_ZERO(&set); + FD_SET(socket_, &set); + if (select(/*nfds=*/0, /*readfds=*/nullptr, &set, /*exceptfds=*/nullptr, + &tm) > 0) { + int error = -1; + int size = sizeof(int); + getsockopt(socket_, SOL_SOCKET, SO_ERROR, (char*)&error, + /*(socklen_t *)*/ &size); + connected = (error == 0); + } + } + if (!connected) { + LOG(ERROR) << "Failed to connect socket with error: " + << WSAGetLastError(); + closesocket(socket_); + socket_ = INVALID_SOCKET; + return false; + } + } + if (has_timeout) { + unsigned long non_blocking = 0; // NOLINT + if (ioctlsocket(socket_, FIONBIO, /*argp=*/&non_blocking) == SOCKET_ERROR) { + LOG(ERROR) << "Failed to set socket to blocking, error: " + << WSAGetLastError(); + } } - LOG(INFO) << "Client socket connected successfully"; if (VLOG_IS_ON(1)) { SocketAddress local_address; @@ -120,7 +157,6 @@ bool NearbyClientSocket ::Connect(const SocketAddress& server_address) { << local_address.ToString(); } } - return true; } diff --git a/internal/platform/implementation/windows/nearby_client_socket.h b/internal/platform/implementation/windows/nearby_client_socket.h index ba68c258..d7479fb0 100644 --- a/internal/platform/implementation/windows/nearby_client_socket.h +++ b/internal/platform/implementation/windows/nearby_client_socket.h @@ -21,6 +21,7 @@ #include #include "absl/base/nullability.h" +#include "absl/time/time.h" #include "internal/platform/byte_array.h" #include "internal/platform/exception.h" #include "internal/platform/implementation/windows/socket_address.h" @@ -41,7 +42,7 @@ class NearbyClientSocket { explicit NearbyClientSocket(SOCKET socket); ~NearbyClientSocket(); - bool Connect(const SocketAddress& server_address); + bool Connect(const SocketAddress& server_address, absl::Duration timeout); ExceptionOr Read(std::int64_t size); ExceptionOr Skip(size_t offset); Exception Write(const ByteArray& data); diff --git a/internal/platform/implementation/windows/nearby_client_socket_test.cc b/internal/platform/implementation/windows/nearby_client_socket_test.cc index 73fe02e2..742cacdf 100644 --- a/internal/platform/implementation/windows/nearby_client_socket_test.cc +++ b/internal/platform/implementation/windows/nearby_client_socket_test.cc @@ -15,6 +15,7 @@ #include "internal/platform/implementation/windows/nearby_client_socket.h" #include "gtest/gtest.h" +#include "absl/time/time.h" #include "internal/platform/byte_array.h" #include "internal/platform/implementation/windows/socket_address.h" @@ -49,7 +50,7 @@ TEST(NearbyClientSocketTest, ConnectWithBoundSocketFails) { SocketAddress server_address(/*dual_stack=*/true); SocketAddress::FromString(server_address, "::1", 8080); - EXPECT_FALSE(client_socket.Connect(server_address)); + EXPECT_FALSE(client_socket.Connect(server_address, absl::InfiniteDuration())); closesocket(socket_handle); } @@ -64,7 +65,7 @@ TEST(NearbyClientSocketTest, Connect) { SocketAddress server_address(/*dual_stack=*/true); SocketAddress::FromString(server_address, "::1", bound_address.port()); - EXPECT_TRUE(client_socket.Connect(server_address)); + EXPECT_TRUE(client_socket.Connect(server_address, absl::InfiniteDuration())); closesocket(socket_handle); } @@ -83,7 +84,7 @@ TEST(NearbyClientSocketTest, Read) { NearbyClientSocket client_socket; SocketAddress server_address(/*dual_stack=*/true); SocketAddress::FromString(server_address, "::1", bound_address.port()); - EXPECT_TRUE(client_socket.Connect(server_address)); + EXPECT_TRUE(client_socket.Connect(server_address, absl::InfiniteDuration())); SocketAddress peer_address; int peer_address_length = sizeof(sockaddr_storage); SOCKET accept_socket = accept(socket_handle, peer_address.address(), @@ -105,7 +106,7 @@ TEST(NearbyClientSocketTest, Skip) { NearbyClientSocket client_socket; SocketAddress server_address(/*dual_stack=*/true); SocketAddress::FromString(server_address, "::1", bound_address.port()); - EXPECT_TRUE(client_socket.Connect(server_address)); + EXPECT_TRUE(client_socket.Connect(server_address, absl::InfiniteDuration())); SocketAddress peer_address; int peer_address_length = sizeof(sockaddr_storage); SOCKET accept_socket = accept(socket_handle, peer_address.address(), @@ -128,7 +129,7 @@ TEST(NearbyClientSocketTest, Write) { NearbyClientSocket client_socket; SocketAddress server_address(/*dual_stack=*/true); SocketAddress::FromString(server_address, "::1", bound_address.port()); - EXPECT_TRUE(client_socket.Connect(server_address)); + EXPECT_TRUE(client_socket.Connect(server_address, absl::InfiniteDuration())); SocketAddress peer_address; int peer_address_length = sizeof(sockaddr_storage); SOCKET accept_socket = accept(socket_handle, peer_address.address(), diff --git a/internal/platform/implementation/windows/wifi_direct.h b/internal/platform/implementation/windows/wifi_direct.h index 068badce..5a147e11 100644 --- a/internal/platform/implementation/windows/wifi_direct.h +++ b/internal/platform/implementation/windows/wifi_direct.h @@ -109,7 +109,7 @@ class WifiDirectSocket : public api::WifiDirectSocket { Exception Close() override { return client_socket_->Close(); } bool Connect(const SocketAddress& server_address) { - return client_socket_->Connect(server_address); + return client_socket_->Connect(server_address, absl::InfiniteDuration()); } private: diff --git a/internal/platform/implementation/windows/wifi_hotspot_medium.cc b/internal/platform/implementation/windows/wifi_hotspot_medium.cc index 215d2c0d..9cfa61f6 100644 --- a/internal/platform/implementation/windows/wifi_hotspot_medium.cc +++ b/internal/platform/implementation/windows/wifi_hotspot_medium.cc @@ -54,6 +54,8 @@ using ::winrt::Windows::Devices::WiFiDirect:: WiFiDirectAdvertisementPublisherStatus; using ::winrt::Windows::Devices::WiFiDirect::WiFiDirectConnectionRequest; using ::winrt::Windows::Security::Credentials::PasswordCredential; + +constexpr absl::Duration kConnectTimeout = absl::Milliseconds(500); } // namespace WifiHotspotMedium::~WifiHotspotMedium() { @@ -114,7 +116,7 @@ std::unique_ptr WifiHotspotMedium::ConnectToService( }); } - bool result = wifi_hotspot_socket->Connect(server_address); + bool result = wifi_hotspot_socket->Connect(server_address, kConnectTimeout); if (!result) { LOG(ERROR) << "Failed to connect to service."; return nullptr; diff --git a/internal/platform/implementation/windows/wifi_hotspot_socket.h b/internal/platform/implementation/windows/wifi_hotspot_socket.h index 135311a4..ebf739f2 100644 --- a/internal/platform/implementation/windows/wifi_hotspot_socket.h +++ b/internal/platform/implementation/windows/wifi_hotspot_socket.h @@ -31,6 +31,7 @@ // Nearby connections headers #include "absl/base/nullability.h" #include "absl/base/thread_annotations.h" +#include "absl/time/time.h" #include "internal/platform/exception.h" #include "internal/platform/implementation/wifi_hotspot.h" #include "internal/platform/implementation/windows/nearby_client_socket.h" @@ -71,8 +72,8 @@ class WifiHotspotSocket : public api::WifiHotspotSocket { // Returns Exception::kIo on error, Exception::kSuccess otherwise. Exception Close() override { return client_socket_->Close(); } - bool Connect(const SocketAddress& server_address) { - return client_socket_->Connect(server_address); + bool Connect(const SocketAddress& server_address, absl::Duration timeout) { + return client_socket_->Connect(server_address, timeout); } private: diff --git a/internal/platform/implementation/windows/wifi_lan.h b/internal/platform/implementation/windows/wifi_lan.h index b2ab68c8..8b866e91 100644 --- a/internal/platform/implementation/windows/wifi_lan.h +++ b/internal/platform/implementation/windows/wifi_lan.h @@ -90,8 +90,8 @@ class WifiLanSocket : public api::WifiLanSocket { // Returns Exception::kIo on error, Exception::kSuccess otherwise. Exception Close() override { return client_socket_->Close(); }; - bool Connect(const SocketAddress& server_address) { - return client_socket_->Connect(server_address); + bool Connect(const SocketAddress& server_address, absl::Duration timeout) { + return client_socket_->Connect(server_address, timeout); }; private: @@ -210,7 +210,8 @@ class WifiLanMedium : public api::WifiLanMedium { absl::Duration timeout); std::unique_ptr ConnectToSocket( - const SocketAddress& address, CancellationFlag* cancellation_flag); + const SocketAddress& address, CancellationFlag* cancellation_flag, + absl::Duration timeout); // Methods to manage discovred services. void ClearDiscoveredServices() ABSL_LOCKS_EXCLUDED(mutex_); diff --git a/internal/platform/implementation/windows/wifi_lan_medium.cc b/internal/platform/implementation/windows/wifi_lan_medium.cc index bc5c3a82..035593ff 100644 --- a/internal/platform/implementation/windows/wifi_lan_medium.cc +++ b/internal/platform/implementation/windows/wifi_lan_medium.cc @@ -15,9 +15,9 @@ #include "internal/platform/implementation/windows/wifi_lan.h" // Windows headers +#include #include #include -#include // Standard C/C++ headers #include @@ -29,16 +29,11 @@ #include #include -// ABSL headers +#include "absl/container/flat_hash_map.h" #include "absl/strings/str_format.h" #include "absl/strings/string_view.h" - -// Nearby connections headers #include "absl/synchronization/mutex.h" #include "absl/time/time.h" - -// Nearby connections headers -#include "absl/container/flat_hash_map.h" #include "internal/flags/nearby_flags.h" #include "internal/platform/cancellation_flag.h" #include "internal/platform/cancellation_flag_listener.h" @@ -49,13 +44,13 @@ #include "internal/platform/implementation/windows/generated/winrt/Windows.Devices.Enumeration.h" #include "internal/platform/implementation/windows/generated/winrt/Windows.Foundation.Collections.h" #include "internal/platform/implementation/windows/generated/winrt/Windows.Networking.Connectivity.h" +#include "internal/platform/implementation/windows/nearby_client_socket.h" #include "internal/platform/implementation/windows/network_info.h" #include "internal/platform/implementation/windows/socket_address.h" #include "internal/platform/implementation/windows/string_utils.h" #include "internal/platform/implementation/windows/utils.h" #include "internal/platform/logging.h" #include "internal/platform/nsd_service_info.h" -#include "internal/platform/runnable.h" namespace nearby::windows { namespace { @@ -78,10 +73,10 @@ constexpr absl::string_view kMdnsDeviceSelectorFormat = "AND System.Devices.Dnssd.ServiceName:=\"%s\" AND " "System.Devices.Dnssd.Domain:=\"local\""; -constexpr absl::Duration kConnectTimeout = absl::Seconds(1); +constexpr absl::Duration kConnectTimeout = absl::Milliseconds(500); bool IsSelfInstance(IMapView properties, - absl::string_view self_instance_name) { + absl::string_view self_instance_name) { IInspectable inspectable = properties.TryLookup(L"System.Devices.Dnssd.InstanceName"); if (inspectable == nullptr) { @@ -110,7 +105,7 @@ bool GetMdnsIpv4Address(const std::string& address_str, const sockaddr_in* ipv4_addr = ipv4_address.ipv4_address(); std::memcpy(ip_address_bytes.data(), &ipv4_addr->sin_addr.s_addr, 4); nsd_service_info.SetIPAddress(ip_address_bytes); - VLOG(1) << "Found ipv4 address: " <(&v6_only), - sizeof(v6_only)) == SOCKET_ERROR) { - LOG(WARNING) << "Failed to set IPV6_V6ONLY with error " - << WSAGetLastError(); - } + NearbyClientSocket client_socket; + if (!client_socket.Connect(address, timeout)) { + return false; } - ioctlsocket(sock, /*cmd=*/FIONBIO, /*argp=*/&non_blocking); - if (connect(sock, address.address(), sizeof(sockaddr_storage)) == - SOCKET_ERROR) { - tm.tv_sec = timeout / absl::Seconds(1); - tm.tv_usec = 0; - FD_ZERO(&set); - FD_SET(sock, &set); - - if (select(sock + 1, nullptr, &set, nullptr, &tm) > 0) { - getsockopt(sock, SOL_SOCKET, SO_ERROR, (char*)&error, - /*(socklen_t *)*/ &size); - result = error == 0; - } else { - result = false; - } - } else { - result = true; - } - - non_blocking = 0; - ioctlsocket(sock, /*cmd=*/FIONBIO, /*argp=*/&non_blocking); - - if (result) { - closesocket(sock); - } - - return result; + return true; } } // namespace @@ -379,12 +330,11 @@ std::unique_ptr WifiLanMedium::ConnectToService( return nullptr; } std::unique_ptr socket = - ConnectToSocket(server_address, cancellation_flag); + ConnectToSocket(server_address, cancellation_flag, kConnectTimeout); if (socket != nullptr) { return socket; } - VLOG(1) << "Failed to connect to service by IPv6 address: " - << ipv6_address; + VLOG(1) << "Failed to connect to service by IPv6 address: " << ipv6_address; return nullptr; } @@ -400,12 +350,12 @@ std::unique_ptr WifiLanMedium::ConnectToService( LOG(ERROR) << "no valid service address and port to connect."; return nullptr; } - return ConnectToSocket(server_address, cancellation_flag); + return ConnectToSocket(server_address, cancellation_flag, kConnectTimeout); } std::unique_ptr WifiLanMedium::ConnectToSocket( - const SocketAddress& address, - CancellationFlag* cancellation_flag) { + const SocketAddress& address, CancellationFlag* cancellation_flag, + absl::Duration timeout) { if (cancellation_flag != nullptr && cancellation_flag->Cancelled()) { LOG(INFO) << "connect to service has been cancelled."; return nullptr; @@ -425,7 +375,7 @@ std::unique_ptr WifiLanMedium::ConnectToSocket( socket->Close(); }); } - bool result = wifi_lan_socket->Connect(address); + bool result = wifi_lan_socket->Connect(address, timeout); if (!result) { LOG(ERROR) << "failed to connect to service."; return nullptr; @@ -605,8 +555,8 @@ fire_and_forget WifiLanMedium::Watcher_DeviceAdded( NsdServiceInfo nsd_service_info = nsd_service_info_except.GetResult(); LOG(INFO) << "device found for service name " - << nsd_service_info.GetServiceName() - << " on port " << nsd_service_info.GetPort(); + << nsd_service_info.GetServiceName() << " on port " + << nsd_service_info.GetPort(); if (!IsConnectableIpAddress(nsd_service_info, kConnectTimeout)) { VLOG(1) << "mDNS service " << nsd_service_info.GetServiceName() @@ -642,8 +592,8 @@ fire_and_forget WifiLanMedium::Watcher_DeviceUpdated( GetDiscoveredService(winrt::to_string(deviceInfoUpdate.Id())); if (!last_nsd_service_info.has_value()) { LOG(INFO) << "device updated for service name " - << nsd_service_info.GetServiceName() - << " on port " << nsd_service_info.GetPort(); + << nsd_service_info.GetServiceName() << " on port " + << nsd_service_info.GetPort(); if (IsConnectableIpAddress(nsd_service_info, kConnectTimeout)) { // If the device is not in the discovered service list, but it is // connectable during update, we add it to the discovered service list. @@ -671,14 +621,15 @@ fire_and_forget WifiLanMedium::Watcher_DeviceUpdated( return fire_and_forget{}; } - LOG(INFO) - << "Device is changed from (service name:" - << last_nsd_service_info->GetServiceName() << ", endpoint info:" - << last_nsd_service_info->GetTxtRecord(std::string(kDeviceEndpointInfo)) - << ", port: " << last_nsd_service_info->GetPort() - << ") to (service name:" << nsd_service_info.GetServiceName() << ", " - << nsd_service_info.GetTxtRecord(std::string(kDeviceEndpointInfo)) - << ", port: " << nsd_service_info.GetPort() << ")."; + LOG(INFO) << "Device is changed from (service name:" + << last_nsd_service_info->GetServiceName() << ", endpoint info:" + << last_nsd_service_info->GetTxtRecord( + std::string(kDeviceEndpointInfo)) + << ", port: " << last_nsd_service_info->GetPort() + << ") to (service name:" << nsd_service_info.GetServiceName() + << ", " + << nsd_service_info.GetTxtRecord(std::string(kDeviceEndpointInfo)) + << ", port: " << nsd_service_info.GetPort() << ")."; // Report device lost first. discovered_service_callback_.service_lost_cb(*last_nsd_service_info); @@ -764,8 +715,8 @@ bool WifiLanMedium::IsConnectableIpAddress(NsdServiceInfo& nsd_service_info, } if (!NearbyFlags::GetInstance().GetBoolFlag( - platform::config_package_nearby::nearby_platform_feature:: - kEnableMdnsIpv6)) { + platform::config_package_nearby::nearby_platform_feature:: + kEnableMdnsIpv6)) { return false; } std::string ipv6_address = nsd_service_info.GetIPv6Address();