libbinder: RpcSession exposes sp<RpcServer>

This object guarantees a strong pointer IFF it is associated with a
server. The wp<> return type here previously was for convenience (idk?)
but users of it shouldn't be concerned with the underlying memory
situation.

Bug: 167966510
Test: binderRpcTest
Change-Id: I6578c3a4246e1bd07f7697c11d4b56899b50245b

3 files changed, 13 insertions, 4 deletions

diff --git a/libs/binder/RpcSession.cpp b/libs/binder/RpcSession.cpp
index 3dbd11fd37..f953a05c08 100644
--- a/libs/binder/RpcSession.cpp
+++ b/libs/binder/RpcSession.cpp
@@ -312,8 +312,13 @@ void RpcSession::join(sp<RpcSession>&& session, PreJoinSetupResult&& setupResult
     }
 }
 
-wp<RpcServer> RpcSession::server() {
-    return mForServer;
+sp<RpcServer> RpcSession::server() {
+    RpcServer* unsafeServer = mForServer.unsafe_get();
+    sp<RpcServer> server = mForServer.promote();
+
+    LOG_ALWAYS_FATAL_IF((unsafeServer == nullptr) != (server == nullptr),
+                        "wp<> is to avoid strong cycle only");
+    return server;
 }
 
 bool RpcSession::setupSocketClient(const RpcSocketAddress& addr) {
diff --git a/libs/binder/RpcState.cpp b/libs/binder/RpcState.cpp
index 8dd6dafd60..967610977d 100644
--- a/libs/binder/RpcState.cpp
+++ b/libs/binder/RpcState.cpp
@@ -741,7 +741,7 @@ processTransactInternalTailCall:
                     break;
                 }
                 default: {
-                    sp<RpcServer> server = session->server().promote();
+                    sp<RpcServer> server = session->server();
                     if (server) {
                         switch (transaction->code) {
                             case RPC_SPECIAL_TRANSACT_GET_ROOT: {
diff --git a/libs/binder/include/binder/RpcSession.h b/libs/binder/include/binder/RpcSession.h
index eaa86dda3e..218de205f5 100644
--- a/libs/binder/include/binder/RpcSession.h
+++ b/libs/binder/include/binder/RpcSession.h
@@ -118,7 +118,11 @@ public:
 
     ~RpcSession();
 
-    wp<RpcServer> server();
+    /**
+     * Server if this session is created as part of a server (symmetrical to
+     * client servers). Otherwise, nullptr.
+     */
+    sp<RpcServer> server();
 
     // internal only
     const std::unique_ptr<RpcState>& state() { return mState; }