diff options
author | 2023-03-01 01:25:33 +0000 | |
---|---|---|
committer | 2023-03-01 21:25:17 +0000 | |
commit | feb13e8bfe5d9e23563761f042fcf19b7b008d4d (patch) | |
tree | 509ab35d45a55dea510e433a8e663ad37129b204 | |
parent | 11a9b4824010d4ef39f9f04eb10d98adcd91d242 (diff) |
RPC Binder: setMaxOutgoing{Threads,Connections}
This API is updated to emphasize that it is placing a limit
on the number of connections that will be started corresponding
to threads in a remote pool. These connections won't actually
correspond to threads in the process that this API is called.
Documentation has also been updated to clarify a few questions
from an internal discussion forum.
Fixes: 270374393
Test: binderRpcTest
Change-Id: Ia0d9f0d0f42f58a2c63bf506476b33985f091a34
-rw-r--r-- | cmds/service/service.cpp | 2 | ||||
-rw-r--r-- | libs/binder/RpcSession.cpp | 5 | ||||
-rw-r--r-- | libs/binder/ServiceManagerHost.cpp | 4 | ||||
-rw-r--r-- | libs/binder/include/binder/IServiceManager.h | 10 | ||||
-rw-r--r-- | libs/binder/include/binder/RpcServer.h | 5 | ||||
-rw-r--r-- | libs/binder/include/binder/RpcSession.h | 18 | ||||
-rw-r--r-- | libs/binder/include_rpc_unstable/binder_rpc_unstable.hpp | 6 | ||||
-rw-r--r-- | libs/binder/libbinder_rpc_unstable.cpp | 4 | ||||
-rw-r--r-- | libs/binder/rust/rpcbinder/src/session.rs | 9 | ||||
-rw-r--r-- | libs/binder/tests/binderHostDeviceTest.cpp | 2 | ||||
-rw-r--r-- | libs/binder/tests/binderRpcTest.cpp | 2 | ||||
-rw-r--r-- | libs/binder/tests/binderRpcTestTrusty.cpp | 2 |
12 files changed, 40 insertions, 29 deletions
diff --git a/cmds/service/service.cpp b/cmds/service/service.cpp index d5ca725eb9..5e8ef5d7d8 100644 --- a/cmds/service/service.cpp +++ b/cmds/service/service.cpp @@ -75,7 +75,7 @@ int main(int argc, char* const argv[]) ProcessState::initWithDriver("/dev/vndbinder"); #endif #ifndef __ANDROID__ - setDefaultServiceManager(createRpcDelegateServiceManager({.maxOutgoingThreads = 1})); + setDefaultServiceManager(createRpcDelegateServiceManager({.maxOutgoingConnections = 1})); #endif sp<IServiceManager> sm = defaultServiceManager(); fflush(stdout); diff --git a/libs/binder/RpcSession.cpp b/libs/binder/RpcSession.cpp index ce6ef2becf..1a821f14c9 100644 --- a/libs/binder/RpcSession.cpp +++ b/libs/binder/RpcSession.cpp @@ -90,7 +90,7 @@ size_t RpcSession::getMaxIncomingThreads() { return mMaxIncomingThreads; } -void RpcSession::setMaxOutgoingThreads(size_t threads) { +void RpcSession::setMaxOutgoingConnections(size_t threads) { RpcMutexLockGuard _l(mMutex); LOG_ALWAYS_FATAL_IF(mStartedSetup, "Must set max outgoing threads before setting up connections"); @@ -932,7 +932,8 @@ status_t RpcSession::ExclusiveConnection::find(const sp<RpcSession>& session, Co (session->server() ? "This is a server session, so see RpcSession::setMaxIncomingThreads " "for the corresponding client" - : "This is a client session, so see RpcSession::setMaxOutgoingThreads " + : "This is a client session, so see " + "RpcSession::setMaxOutgoingConnections " "for this client or RpcServer::setMaxThreads for the corresponding " "server")); return WOULD_BLOCK; diff --git a/libs/binder/ServiceManagerHost.cpp b/libs/binder/ServiceManagerHost.cpp index 194254ac69..2b67f030e0 100644 --- a/libs/binder/ServiceManagerHost.cpp +++ b/libs/binder/ServiceManagerHost.cpp @@ -159,8 +159,8 @@ sp<IBinder> getDeviceService(std::vector<std::string>&& serviceDispatcherArgs, LOG_ALWAYS_FATAL_IF(!forwardResult->hostPort().has_value()); auto rpcSession = RpcSession::make(); - if (options.maxOutgoingThreads.has_value()) { - rpcSession->setMaxOutgoingThreads(*options.maxOutgoingThreads); + if (options.maxOutgoingConnections.has_value()) { + rpcSession->setMaxOutgoingConnections(*options.maxOutgoingConnections); } if (status_t status = rpcSession->setupInetClient("127.0.0.1", *forwardResult->hostPort()); diff --git a/libs/binder/include/binder/IServiceManager.h b/libs/binder/include/binder/IServiceManager.h index c78f870153..55167a7db0 100644 --- a/libs/binder/include/binder/IServiceManager.h +++ b/libs/binder/include/binder/IServiceManager.h @@ -224,12 +224,12 @@ bool checkPermission(const String16& permission, pid_t pid, uid_t uid, // } // Resources are cleaned up when the object is destroyed. // -// For each returned binder object, at most |maxOutgoingThreads| outgoing threads are instantiated. -// Hence, only |maxOutgoingThreads| calls can be made simultaneously. Additional calls are blocked -// if there are |maxOutgoingThreads| ongoing calls. See RpcSession::setMaxOutgoingThreads. -// If |maxOutgoingThreads| is not set, default is |RpcSession::kDefaultMaxOutgoingThreads|. +// For each returned binder object, at most |maxOutgoingConnections| outgoing connections are +// instantiated, depending on how many the service on the device is configured with. +// Hence, only |maxOutgoingConnections| calls can be made simultaneously. +// See also RpcSession::setMaxOutgoingConnections. struct RpcDelegateServiceManagerOptions { - std::optional<size_t> maxOutgoingThreads; + std::optional<size_t> maxOutgoingConnections; }; sp<IServiceManager> createRpcDelegateServiceManager( const RpcDelegateServiceManagerOptions& options); diff --git a/libs/binder/include/binder/RpcServer.h b/libs/binder/include/binder/RpcServer.h index 25193a3428..1001b64ede 100644 --- a/libs/binder/include/binder/RpcServer.h +++ b/libs/binder/include/binder/RpcServer.h @@ -119,7 +119,10 @@ public: [[nodiscard]] status_t setupExternalServer(base::unique_fd serverFd); /** - * This must be called before adding a client session. + * This must be called before adding a client session. This corresponds + * to the number of incoming connections to RpcSession objects in the + * server, which will correspond to the number of outgoing connections + * in client RpcSession objects. * * If this is not specified, this will be a single-threaded server. * diff --git a/libs/binder/include/binder/RpcSession.h b/libs/binder/include/binder/RpcSession.h index 40faf2c3b1..e301569acc 100644 --- a/libs/binder/include/binder/RpcSession.h +++ b/libs/binder/include/binder/RpcSession.h @@ -67,26 +67,30 @@ public: /** * Set the maximum number of incoming threads allowed to be made (for things like callbacks). * By default, this is 0. This must be called before setting up this connection as a client. - * Server sessions will inherits this value from RpcServer. + * Server sessions will inherits this value from RpcServer. Each thread will serve a + * connection to the remote RpcSession. * * If this is called, 'shutdown' on this session must also be called. * Otherwise, a threadpool will leak. * - * TODO(b/189955605): start these dynamically + * TODO(b/189955605): start these lazily - currently all are started */ void setMaxIncomingThreads(size_t threads); size_t getMaxIncomingThreads(); /** - * Set the maximum number of outgoing threads allowed to be made. + * Set the maximum number of outgoing connections allowed to be made. * By default, this is |kDefaultMaxOutgoingThreads|. This must be called before setting up this * connection as a client. * - * This limits the number of outgoing threads on top of the remote peer setting. This RpcSession - * will only instantiate |min(maxOutgoingThreads, remoteMaxThreads)| outgoing threads, where - * |remoteMaxThreads| can be retrieved from the remote peer via |getRemoteMaxThreads()|. + * For an RpcSession client, if you are connecting to a server which starts N threads, + * then this must be set to >= N. If you set the maximum number of outgoing connections + * to 1, but the server requests 10, then it would be considered an error. If you set a + * maximum number of connections to 10, and the server requests 1, then only 1 will be + * created. This API is used to limit the amount of resources a server can request you + * create. */ - void setMaxOutgoingThreads(size_t threads); + void setMaxOutgoingConnections(size_t threads); size_t getMaxOutgoingThreads(); /** diff --git a/libs/binder/include_rpc_unstable/binder_rpc_unstable.hpp b/libs/binder/include_rpc_unstable/binder_rpc_unstable.hpp index 42d226b805..e273dff5f6 100644 --- a/libs/binder/include_rpc_unstable/binder_rpc_unstable.hpp +++ b/libs/binder/include_rpc_unstable/binder_rpc_unstable.hpp @@ -126,11 +126,11 @@ AIBinder* ARpcSession_setupPreconnectedClient(ARpcSession* session, void ARpcSession_setFileDescriptorTransportMode(ARpcSession* session, ARpcSession_FileDescriptorTransportMode mode); -// Sets the maximum number of incoming threads. +// Sets the maximum number of incoming threads, to service connections. void ARpcSession_setMaxIncomingThreads(ARpcSession* session, size_t threads); -// Sets the maximum number of outgoing threads. -void ARpcSession_setMaxOutgoingThreads(ARpcSession* session, size_t threads); +// Sets the maximum number of outgoing connections. +void ARpcSession_setMaxOutgoingConnections(ARpcSession* session, size_t threads); // Decrements the refcount of the underlying RpcSession object. void ARpcSession_free(ARpcSession* session); diff --git a/libs/binder/libbinder_rpc_unstable.cpp b/libs/binder/libbinder_rpc_unstable.cpp index daff8c1e16..971b3a056a 100644 --- a/libs/binder/libbinder_rpc_unstable.cpp +++ b/libs/binder/libbinder_rpc_unstable.cpp @@ -265,8 +265,8 @@ void ARpcSession_setMaxIncomingThreads(ARpcSession* handle, size_t threads) { session->setMaxIncomingThreads(threads); } -void ARpcSession_setMaxOutgoingThreads(ARpcSession* handle, size_t threads) { +void ARpcSession_setMaxOutgoingConnections(ARpcSession* handle, size_t threads) { auto session = handleToStrongPointer<RpcSession>(handle); - session->setMaxOutgoingThreads(threads); + session->setMaxOutgoingConnections(threads); } } diff --git a/libs/binder/rust/rpcbinder/src/session.rs b/libs/binder/rust/rpcbinder/src/session.rs index 0b517cf613..28c5390665 100644 --- a/libs/binder/rust/rpcbinder/src/session.rs +++ b/libs/binder/rust/rpcbinder/src/session.rs @@ -75,11 +75,14 @@ impl RpcSessionRef { }; } - /// Sets the maximum number of outgoing threads. - pub fn set_max_outgoing_threads(&self, threads: usize) { + /// Sets the maximum number of outgoing connections. + pub fn set_max_outgoing_connections(&self, connections: usize) { // SAFETY - Only passes the 'self' pointer as an opaque handle. unsafe { - binder_rpc_unstable_bindgen::ARpcSession_setMaxOutgoingThreads(self.as_ptr(), threads) + binder_rpc_unstable_bindgen::ARpcSession_setMaxOutgoingConnections( + self.as_ptr(), + connections, + ) }; } diff --git a/libs/binder/tests/binderHostDeviceTest.cpp b/libs/binder/tests/binderHostDeviceTest.cpp index 464da60dde..77a5fa8d65 100644 --- a/libs/binder/tests/binderHostDeviceTest.cpp +++ b/libs/binder/tests/binderHostDeviceTest.cpp @@ -66,7 +66,7 @@ MATCHER_P(StatusEq, expected, (negation ? "not " : "") + statusToString(expected void initHostRpcServiceManagerOnce() { static std::once_flag gSmOnce; std::call_once(gSmOnce, [] { - setDefaultServiceManager(createRpcDelegateServiceManager({.maxOutgoingThreads = 1})); + setDefaultServiceManager(createRpcDelegateServiceManager({.maxOutgoingConnections = 1})); }); } diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp index 84c93ddc30..846d945704 100644 --- a/libs/binder/tests/binderRpcTest.cpp +++ b/libs/binder/tests/binderRpcTest.cpp @@ -350,7 +350,7 @@ std::unique_ptr<ProcessSession> BinderRpc::createRpcTestSocketServerProcessEtc( for (const auto& session : sessions) { CHECK(session->setProtocolVersion(clientVersion)); session->setMaxIncomingThreads(options.numIncomingConnections); - session->setMaxOutgoingThreads(options.numOutgoingConnections); + session->setMaxOutgoingConnections(options.numOutgoingConnections); session->setFileDescriptorTransportMode(options.clientFileDescriptorTransportMode); switch (socketType) { diff --git a/libs/binder/tests/binderRpcTestTrusty.cpp b/libs/binder/tests/binderRpcTestTrusty.cpp index b3bb5ebda7..84abbac6c6 100644 --- a/libs/binder/tests/binderRpcTestTrusty.cpp +++ b/libs/binder/tests/binderRpcTestTrusty.cpp @@ -71,7 +71,7 @@ std::unique_ptr<ProcessSession> BinderRpc::createRpcTestSocketServerProcessEtc( auto session = android::RpcSession::make(std::move(factory)); EXPECT_TRUE(session->setProtocolVersion(clientVersion)); - session->setMaxOutgoingThreads(options.numOutgoingConnections); + session->setMaxOutgoingConnections(options.numOutgoingConnections); session->setFileDescriptorTransportMode(options.clientFileDescriptorTransportMode); status = session->setupPreconnectedClient({}, [&]() { |