rpc_binder: Prevent RpcServer shutdown deadlock

RpcServer::~RpcServer invokes shutdown() to trigger exit from all
join and session threads. The function waits for the number of
connections to drop down to zero, but this depends on RpcSession
promoting a wp<RpcServer> to sp<RpcServer>. Since this is happening
during the destructor, when the refcount is zero, this pointer
promotion fails. As a result, the list of connections may not be fully
cleared and the thread calling shutdown() will deadlock.

Fix this by forcing users to call shutdown() earlier and panicing
otherwise.

Bug: 263168076
Test: cleanly shutdown RpcServer with many connections
Change-Id: Ia67a4a839419aafb1bd47fb93ed2e76d56b107c2

2 files changed, 5 insertions, 1 deletions

diff --git a/libs/binder/RpcServer.cpp b/libs/binder/RpcServer.cpp
index fedc1d9593..d47e4f044a 100644
--- a/libs/binder/RpcServer.cpp
+++ b/libs/binder/RpcServer.cpp
@@ -50,7 +50,8 @@ using base::unique_fd;
 
 RpcServer::RpcServer(std::unique_ptr<RpcTransportCtx> ctx) : mCtx(std::move(ctx)) {}
 RpcServer::~RpcServer() {
-    (void)shutdown();
+    RpcMutexUniqueLock _l(mLock);
+    LOG_ALWAYS_FATAL_IF(mShutdownTrigger != nullptr, "Must call shutdown() before destructor");
 }
 
 sp<RpcServer> RpcServer::make(std::unique_ptr<RpcTransportCtxFactory> rpcTransportCtxFactory) {
diff --git a/libs/binder/libbinder_rpc_unstable.cpp b/libs/binder/libbinder_rpc_unstable.cpp
index 89ef46d5c3..78dae4bd1f 100644
--- a/libs/binder/libbinder_rpc_unstable.cpp
+++ b/libs/binder/libbinder_rpc_unstable.cpp
@@ -162,6 +162,9 @@ bool ARpcServer_shutdown(ARpcServer* handle) {
 }
 
 void ARpcServer_free(ARpcServer* handle) {
+    // Ignore the result of ARpcServer_shutdown - either it had been called
+    // earlier, or the RpcServer destructor will panic.
+    (void)ARpcServer_shutdown(handle);
     freeObjectHandle<RpcServer>(handle);
 }