commit 022db680633b483ec5fab4f6b0722981482b3a02
author Tomasz Wasilczyk <twasilczyk@google.com> Mon Jun 17 13:55:51 2024 -0700
committer Tomasz Wasilczyk <twasilczyk@google.com> Thu Jun 27 12:04:34 2024 -0700
tree 565f28e290cc010c726599d4a6ecbb7a0f2b13b8
parent 1d46f58320055b7aad551b92827bf3405258f25e

RpcServer: let system allocate vsock ports

This greatly improves stability with concurrent test execution

Bug: 285204695
Test: atest --host binder_sdk_test
Change-Id: Ib7b76df693288033e55fa27786bf6efb21d0ebf0

libs/binder/tests/binderRpcTest.cpp

diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp
index 19882ea..9b1b64a 100644
--- a/libs/binder/tests/binderRpcTest.cpp
+++ b/libs/binder/tests/binderRpcTest.cpp
@@ -141,11 +141,6 @@
     return ret;
 };
 
-static unsigned int allocateVsockPort() {
-    static unsigned int vsockPort = 34567;
-    return vsockPort++;
-}
-
 static unique_fd initUnixSocket(std::string addr) {
     auto socket_addr = UnixSocketAddress(addr.c_str());
     unique_fd fd(TEMP_FAILURE_RETRY(socket(socket_addr.addr()->sa_family, SOCK_STREAM, AF_UNIX)));
@@ -300,7 +295,6 @@
     serverConfig.socketType = static_cast<int32_t>(socketType);
     serverConfig.rpcSecurity = static_cast<int32_t>(rpcSecurity);
     serverConfig.serverVersion = serverVersion;
-    serverConfig.vsockPort = allocateVsockPort();
     serverConfig.addr = addr;
     serverConfig.socketFd = socketFd.get();
     for (auto mode : options.serverSupportedFileDescriptorTransportModes) {
@@ -379,7 +373,7 @@
                         unique_fd(dup(bootstrapClientFd.get())));
                 break;
             case SocketType::VSOCK:
-                status = session->setupVsockClient(VMADDR_CID_LOCAL, serverConfig.vsockPort);
+                status = session->setupVsockClient(VMADDR_CID_LOCAL, serverInfo.port);
                 break;
             case SocketType::INET:
                 status = session->setupInetClient("127.0.0.1", serverInfo.port);
@@ -1152,8 +1146,6 @@
 #else // BINDER_RPC_TO_TRUSTY_TEST
 bool testSupportVsockLoopback() {
     // We don't need to enable TLS to know if vsock is supported.
-    unsigned int vsockPort = allocateVsockPort();
-
     unique_fd serverFd(
             TEMP_FAILURE_RETRY(socket(AF_VSOCK, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0)));
 
@@ -1165,16 +1157,21 @@
 
     sockaddr_vm serverAddr{
             .svm_family = AF_VSOCK,
-            .svm_port = vsockPort,
+            .svm_port = VMADDR_PORT_ANY,
             .svm_cid = VMADDR_CID_ANY,
     };
     int ret = TEMP_FAILURE_RETRY(
             bind(serverFd.get(), reinterpret_cast<sockaddr*>(&serverAddr), sizeof(serverAddr)));
-    LOG_ALWAYS_FATAL_IF(0 != ret, "Could not bind socket to port %u: %s", vsockPort,
+    LOG_ALWAYS_FATAL_IF(0 != ret, "Could not bind socket to port VMADDR_PORT_ANY: %s",
                         strerror(errno));
 
+    socklen_t len = sizeof(serverAddr);
+    ret = getsockname(serverFd.get(), reinterpret_cast<sockaddr*>(&serverAddr), &len);
+    LOG_ALWAYS_FATAL_IF(0 != ret, "Failed to getsockname: %s", strerror(errno));
+    LOG_ALWAYS_FATAL_IF(len < sizeof(serverAddr), "getsockname didn't read the full addr struct");
+
     ret = TEMP_FAILURE_RETRY(listen(serverFd.get(), 1 /*backlog*/));
-    LOG_ALWAYS_FATAL_IF(0 != ret, "Could not listen socket on port %u: %s", vsockPort,
+    LOG_ALWAYS_FATAL_IF(0 != ret, "Could not listen socket on port %u: %s", serverAddr.svm_port,
                         strerror(errno));
 
     // Try to connect to the server using the VMADDR_CID_LOCAL cid
@@ -1183,13 +1180,13 @@
     // and they return ETIMEDOUT after that.
     unique_fd connectFd(
             TEMP_FAILURE_RETRY(socket(AF_VSOCK, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0)));
-    LOG_ALWAYS_FATAL_IF(!connectFd.ok(), "Could not create socket for port %u: %s", vsockPort,
-                        strerror(errno));
+    LOG_ALWAYS_FATAL_IF(!connectFd.ok(), "Could not create socket for port %u: %s",
+                        serverAddr.svm_port, strerror(errno));
 
     bool success = false;
     sockaddr_vm connectAddr{
             .svm_family = AF_VSOCK,
-            .svm_port = vsockPort,
+            .svm_port = serverAddr.svm_port,
             .svm_cid = VMADDR_CID_LOCAL,
     };
     ret = TEMP_FAILURE_RETRY(connect(connectFd.get(), reinterpret_cast<sockaddr*>(&connectAddr),
@@ -1538,8 +1535,9 @@
                     };
                 } break;
                 case SocketType::VSOCK: {
-                    auto port = allocateVsockPort();
-                    auto status = rpcServer->setupVsockServer(VMADDR_CID_LOCAL, port);
+                    unsigned port;
+                    auto status =
+                            rpcServer->setupVsockServer(VMADDR_CID_LOCAL, VMADDR_PORT_ANY, &port);
                     if (status != OK) {
                         return AssertionFailure() << "setupVsockServer: " << statusToString(status);
                     }