Add connect timeout to NearbyClientSocket.

PiperOrigin-RevId: 828703670

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",
     ],
 )

internal/platform/implementation/windows/nearby_client_socket.cc

@@ -22,6 +22,7 @@
 #include <string>
 #include <utility>
 
+#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<char*>(&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;
 }
 

internal/platform/implementation/windows/nearby_client_socket.h

@@ -21,6 +21,7 @@
 #include <cstdint>
 
 #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<ByteArray> Read(std::int64_t size);
   ExceptionOr<size_t> Skip(size_t offset);
   Exception Write(const ByteArray& data);

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(),

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:

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<api::WifiHotspotSocket> 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;

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:

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<api::WifiLanSocket> 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_);

internal/platform/implementation/windows/wifi_lan_medium.cc

@@ -15,9 +15,9 @@
 #include "internal/platform/implementation/windows/wifi_lan.h"
 
 // Windows headers
+#include <iphlpapi.h>
 #include <windows.h>
 #include <winsock2.h>
-#include <iphlpapi.h>
 
 // Standard C/C++ headers
 #include <cstdint>
@@ -29,16 +29,11 @@
 #include <utility>
 #include <vector>
 
-// 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<winrt::hstring, IInspectable> 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: " <<ipv4_address.ToString();
+    VLOG(1) << "Found ipv4 address: " << ipv4_address.ToString();
     return true;
   }
   // Should not reach here.
@@ -136,13 +131,11 @@ bool GetMdnsIpv6Address(const std::string& address_str,
       }
       GUID network_adapter_id = InspectableReader::ReadGuid(inspectable);
       NET_LUID luid;
-      if (ConvertInterfaceGuidToLuid(&network_adapter_id, &luid) !=
-          NO_ERROR) {
+      if (ConvertInterfaceGuidToLuid(&network_adapter_id, &luid) != NO_ERROR) {
         VLOG(1) << "Failed to get interface luid";
         return false;
       }
-      if (ConvertInterfaceLuidToIndex(&luid, &interface_index) !=
-          NO_ERROR) {
+      if (ConvertInterfaceLuidToIndex(&luid, &interface_index) != NO_ERROR) {
         VLOG(1) << "Failed to get interface index";
         return false;
       }
@@ -151,7 +144,7 @@ bool GetMdnsIpv6Address(const std::string& address_str,
       ipv6_address.SetScopeId(interface_index);
     }
     nsd_service_info.SetIPv6Address(ipv6_address.ToString());
-    VLOG(1) << "Found ipv6 address: " <<ipv6_address.ToString();
+    VLOG(1) << "Found ipv6 address: " << ipv6_address.ToString();
     return true;
   }
   // Should not reach here.
@@ -160,55 +153,13 @@ bool GetMdnsIpv6Address(const std::string& address_str,
 
 // Returns true if a connection can be established to the given address within
 // the given timeout.
-bool TestConnection(
-    const SocketAddress& address, absl::Duration timeout) {
-  bool result = false;
-  int error = -1;
-  int size = sizeof(int);
-  timeval tm;
-  fd_set set;
-  unsigned long non_blocking = 1;  // NOLINT
+bool TestConnection(const SocketAddress& address, absl::Duration timeout) {
   VLOG(1) << "Checking connection to: " << address.ToString();
-  SOCKET sock = socket(address.dual_stack() ? AF_INET6 : AF_INET, SOCK_STREAM,
-                       IPPROTO_TCP);
-  if (address.dual_stack()) {
-    // On Windows dual stack is not the default.
-    // https://learn.microsoft.com/en-us/windows/win32/winsock/dual-stack-sockets#creating-a-dual-stack-socket
-    DWORD v6_only = 0;
-    if (setsockopt(sock, IPPROTO_IPV6, IPV6_V6ONLY,
-                   reinterpret_cast<const char*>(&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<api::WifiLanSocket> WifiLanMedium::ConnectToService(
     return nullptr;
   }
   std::unique_ptr<api::WifiLanSocket> 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<api::WifiLanSocket> 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<api::WifiLanSocket> 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<api::WifiLanSocket> 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();