libbinder: RPC fixup retry logic
By always reading from the pipe in the test, we can ensure we only try
to connect to a server once it's started. After this, additional retry
logic is unneeded.
Bug: 185167543
Test: binderRpcTest
Test: binderRpcTest (after introducing 100ms sleep before 'accept4' in
RpcServer to simulate a server that is busy doing something else or
busy accepting other calls from other clients)
Change-Id: I9b5b2876aa74e6ef1b6e8edc4c25124c29446b86
diff --git a/libs/binder/RpcSession.cpp b/libs/binder/RpcSession.cpp
index f406c2d..cb6c787 100644
--- a/libs/binder/RpcSession.cpp
+++ b/libs/binder/RpcSession.cpp
@@ -215,42 +215,52 @@
// we've already setup one client
for (size_t i = 0; i + 1 < numThreadsAvailable; i++) {
- // TODO(b/185167543): avoid race w/ accept4 not being called on server
- for (size_t tries = 0; tries < 5; tries++) {
- if (setupOneSocketClient(addr, mId.value())) break;
- usleep(10000);
- }
+ // TODO(b/185167543): shutdown existing connections?
+ if (!setupOneSocketClient(addr, mId.value())) return false;
}
return true;
}
bool RpcSession::setupOneSocketClient(const RpcSocketAddress& addr, int32_t id) {
- unique_fd serverFd(
- TEMP_FAILURE_RETRY(socket(addr.addr()->sa_family, SOCK_STREAM | SOCK_CLOEXEC, 0)));
- if (serverFd == -1) {
- int savedErrno = errno;
- ALOGE("Could not create socket at %s: %s", addr.toString().c_str(), strerror(savedErrno));
- return false;
+ for (size_t tries = 0; tries < 5; tries++) {
+ if (tries > 0) usleep(10000);
+
+ unique_fd serverFd(
+ TEMP_FAILURE_RETRY(socket(addr.addr()->sa_family, SOCK_STREAM | SOCK_CLOEXEC, 0)));
+ if (serverFd == -1) {
+ int savedErrno = errno;
+ ALOGE("Could not create socket at %s: %s", addr.toString().c_str(),
+ strerror(savedErrno));
+ return false;
+ }
+
+ if (0 != TEMP_FAILURE_RETRY(connect(serverFd.get(), addr.addr(), addr.addrSize()))) {
+ if (errno == ECONNRESET) {
+ ALOGW("Connection reset on %s", addr.toString().c_str());
+ continue;
+ }
+ int savedErrno = errno;
+ ALOGE("Could not connect socket at %s: %s", addr.toString().c_str(),
+ strerror(savedErrno));
+ return false;
+ }
+
+ if (sizeof(id) != TEMP_FAILURE_RETRY(write(serverFd.get(), &id, sizeof(id)))) {
+ int savedErrno = errno;
+ ALOGE("Could not write id to socket at %s: %s", addr.toString().c_str(),
+ strerror(savedErrno));
+ return false;
+ }
+
+ LOG_RPC_DETAIL("Socket at %s client with fd %d", addr.toString().c_str(), serverFd.get());
+
+ addClient(std::move(serverFd));
+ return true;
}
- if (0 != TEMP_FAILURE_RETRY(connect(serverFd.get(), addr.addr(), addr.addrSize()))) {
- int savedErrno = errno;
- ALOGE("Could not connect socket at %s: %s", addr.toString().c_str(), strerror(savedErrno));
- return false;
- }
-
- if (sizeof(id) != TEMP_FAILURE_RETRY(write(serverFd.get(), &id, sizeof(id)))) {
- int savedErrno = errno;
- ALOGE("Could not write id to socket at %s: %s", addr.toString().c_str(),
- strerror(savedErrno));
- return false;
- }
-
- LOG_RPC_DETAIL("Socket at %s client with fd %d", addr.toString().c_str(), serverFd.get());
-
- addClient(std::move(serverFd));
- return true;
+ ALOGE("Ran out of retries to connect to %s", addr.toString().c_str());
+ return false;
}
void RpcSession::addClient(unique_fd fd) {
diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp
index db7f67d..b3ce744 100644
--- a/libs/binder/tests/binderRpcTest.cpp
+++ b/libs/binder/tests/binderRpcTest.cpp
@@ -327,6 +327,8 @@
server->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction();
server->setMaxThreads(numThreads);
+ unsigned int outPort = 0;
+
switch (socketType) {
case SocketType::UNIX:
CHECK(server->setupUnixDomainServer(addr.c_str())) << addr;
@@ -335,47 +337,43 @@
CHECK(server->setupVsockServer(vsockPort));
break;
case SocketType::INET: {
- unsigned int outPort = 0;
CHECK(server->setupInetServer(0, &outPort));
CHECK_NE(0, outPort);
- CHECK(android::base::WriteFully(pipe->writeEnd(), &outPort,
- sizeof(outPort)));
break;
}
default:
LOG_ALWAYS_FATAL("Unknown socket type");
}
+ CHECK(android::base::WriteFully(pipe->writeEnd(), &outPort, sizeof(outPort)));
+
configure(server);
server->join();
}),
};
- unsigned int inetPort = 0;
+ // always read socket, so that we have waited for the server to start
+ unsigned int outPort = 0;
+ CHECK(android::base::ReadFully(ret.host.getPipe()->readEnd(), &outPort, sizeof(outPort)));
if (socketType == SocketType::INET) {
- CHECK(android::base::ReadFully(ret.host.getPipe()->readEnd(), &inetPort,
- sizeof(inetPort)));
- CHECK_NE(0, inetPort);
+ CHECK_NE(0, outPort);
}
for (size_t i = 0; i < numSessions; i++) {
sp<RpcSession> session = RpcSession::make();
- for (size_t tries = 0; tries < 10; tries++) {
- usleep(10000);
- switch (socketType) {
- case SocketType::UNIX:
- if (session->setupUnixDomainClient(addr.c_str())) goto success;
- break;
- case SocketType::VSOCK:
- if (session->setupVsockClient(VMADDR_CID_LOCAL, vsockPort)) goto success;
- break;
- case SocketType::INET:
- if (session->setupInetClient("127.0.0.1", inetPort)) goto success;
- break;
- default:
- LOG_ALWAYS_FATAL("Unknown socket type");
- }
+ switch (socketType) {
+ case SocketType::UNIX:
+ if (session->setupUnixDomainClient(addr.c_str())) goto success;
+ break;
+ case SocketType::VSOCK:
+ if (session->setupVsockClient(VMADDR_CID_LOCAL, vsockPort)) goto success;
+ break;
+ case SocketType::INET:
+ if (session->setupInetClient("127.0.0.1", outPort)) goto success;
+ break;
+ default:
+ LOG_ALWAYS_FATAL("Unknown socket type");
}
LOG_ALWAYS_FATAL("Could not connect");
success: