commit a08546707ce4dd72997c9b30b118d8ce86612f9b
author Steven Moreland <smoreland@google.com> Thu May 20 01:18:54 2021 +0000
committer Steven Moreland <smoreland@google.com> Thu May 20 01:20:33 2021 +0000
tree 85a725cc8568fcbce99b4b21aced52f821a07309
parent 4ec3c43c8a9f16f90ab5830c4aa1e238a0c248cf

libbinder: RpcServer privatize acceptOne

This doesn't configure RpcServer w/ a way to be shutdown, and we don't
really need it, so simplifying things.

Bug: 185167543
Test: N/A
Change-Id: I7648ff04f6096f215a90caf3b9f21060eca882ac

libs/binder/RpcServer.cpp

diff --git a/libs/binder/RpcServer.cpp b/libs/binder/RpcServer.cpp
index 249ce6c..1128057 100644
--- a/libs/binder/RpcServer.cpp
+++ b/libs/binder/RpcServer.cpp
@@ -139,7 +139,7 @@
     }
 
     while (mShutdownTrigger->triggerablePollRead(mServer)) {
-        (void)acceptOneNoCheck();
+        (void)acceptOne();
     }
 
     {
@@ -150,12 +150,6 @@
 }
 
 bool RpcServer::acceptOne() {
-    LOG_ALWAYS_FATAL_IF(!mAgreedExperimental, "no!");
-    LOG_ALWAYS_FATAL_IF(!hasServer(), "RpcServer must be setup to acceptOne.");
-    return acceptOneNoCheck();
-}
-
-bool RpcServer::acceptOneNoCheck() {
     unique_fd clientFd(
             TEMP_FAILURE_RETRY(accept4(mServer.get(), nullptr, nullptr /*length*/, SOCK_CLOEXEC)));
 

libs/binder/include/binder/RpcServer.h

diff --git a/libs/binder/include/binder/RpcServer.h b/libs/binder/include/binder/RpcServer.h
index d1a8627..365bed6 100644
--- a/libs/binder/include/binder/RpcServer.h
+++ b/libs/binder/include/binder/RpcServer.h
@@ -135,12 +135,6 @@
     [[nodiscard]] bool shutdown();
 
     /**
-     * Accept one connection on this server. You must have at least one client
-     * session before calling this.
-     */
-    [[nodiscard]] bool acceptOne();
-
-    /**
      * For debugging!
      */
     std::vector<sp<RpcSession>> listSessions();
@@ -158,7 +152,7 @@
 
     void establishConnection(sp<RpcServer>&& session, base::unique_fd clientFd);
     bool setupSocketServer(const RpcSocketAddress& address);
-    [[nodiscard]] bool acceptOneNoCheck();
+    [[nodiscard]] bool acceptOne();
 
     bool mAgreedExperimental = false;
     size_t mMaxThreads = 1;

libs/binder/tests/rpc_fuzzer/main.cpp

diff --git a/libs/binder/tests/rpc_fuzzer/main.cpp b/libs/binder/tests/rpc_fuzzer/main.cpp
index 8a12aea..84f5974 100644
--- a/libs/binder/tests/rpc_fuzzer/main.cpp
+++ b/libs/binder/tests/rpc_fuzzer/main.cpp
@@ -61,7 +61,7 @@
     server->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction();
     CHECK(server->setupUnixDomainServer(kSock.c_str()));
 
-    std::thread serverThread([=] { (void)server->acceptOne(); });
+    std::thread serverThread([=] { (void)server->join(); });
 
     sockaddr_un addr{
             .sun_family = AF_UNIX,
@@ -76,8 +76,6 @@
                      connect(clientFd.get(), reinterpret_cast<sockaddr*>(&addr), sizeof(addr))))
             << strerror(errno);
 
-    serverThread.join();
-
     // TODO(b/182938024): fuzz multiple sessions, instead of just one
 
 #if 0
@@ -90,6 +88,12 @@
 
     clientFd.reset();
 
+    // TODO(185167543): currently this is okay because we only shutdown the one
+    // thread, but once we can shutdown other sessions, we'll need to change
+    // this behavior in order to make sure all of the input is actually read.
+    while (!server->shutdown()) usleep(100);
+    serverThread.join();
+
     // TODO(b/185167543): better way to force a server to shutdown
     while (!server->listSessions().empty() && server->numUninitializedSessions()) {
         usleep(1);