Merge "binder: setRpcClientDebug drops numThreads argument." am: 7f26fabeed am: 68bb822d2b (490b144e) · Commits · e / os / platform_frameworks_native

libs/binder/Binder.cpp

+6 −15

Original line number	Diff line number	Diff line
		@@ -150,7 +150,7 @@ status_t IBinder::getDebugPid(pid_t* out) {
		return OK;
		}

		status_t IBinder::setRpcClientDebug(android::base::unique_fd socketFd, uint32_t maxRpcThreads) {
		status_t IBinder::setRpcClientDebug(android::base::unique_fd socketFd) {
		if constexpr (!kEnableRpcDevServers) {
		ALOGW("setRpcClientDebug disallowed because RPC is not enabled");
		return INVALID_OPERATION;
		@@ -158,7 +158,7 @@ status_t IBinder::setRpcClientDebug(android::base::unique_fd socketFd, uint32_t

		BBinder* local = this->localBinder();
		if (local != nullptr) {
		return local->BBinder::setRpcClientDebug(std::move(socketFd), maxRpcThreads);
		return local->BBinder::setRpcClientDebug(std::move(socketFd));
		}

		BpBinder* proxy = this->remoteBinder();
		@@ -173,7 +173,6 @@ status_t IBinder::setRpcClientDebug(android::base::unique_fd socketFd, uint32_t
		status = data.writeFileDescriptor(socketFd.release(), true /* own */);
		if (status != OK) return status;
		}
		if (status = data.writeUint32(maxRpcThreads); status != OK) return status;
		return transact(SET_RPC_CLIENT_TRANSACTION, data, &reply);
		}

		@@ -449,36 +448,29 @@ status_t BBinder::setRpcClientDebug(const Parcel& data) {
		status_t status;
		bool hasSocketFd;
		android::base::unique_fd clientFd;
		uint32_t maxRpcThreads;

		if (status = data.readBool(&hasSocketFd); status != OK) return status;
		if (hasSocketFd) {
		if (status = data.readUniqueFileDescriptor(&clientFd); status != OK) return status;
		}
		if (status = data.readUint32(&maxRpcThreads); status != OK) return status;

		return setRpcClientDebug(std::move(clientFd), maxRpcThreads);
		return setRpcClientDebug(std::move(clientFd));
		}

		status_t BBinder::setRpcClientDebug(android::base::unique_fd socketFd, uint32_t maxRpcThreads) {
		status_t BBinder::setRpcClientDebug(android::base::unique_fd socketFd) {
		if constexpr (!kEnableRpcDevServers) {
		ALOGW("%s: disallowed because RPC is not enabled", __PRETTY_FUNCTION__);
		return INVALID_OPERATION;
		}

		const int socketFdForPrint = socketFd.get();
		LOG_RPC_DETAIL("%s(%d, %" PRIu32 ")", __PRETTY_FUNCTION__, socketFdForPrint, maxRpcThreads);
		LOG_RPC_DETAIL("%s(fd=%d)", __PRETTY_FUNCTION__, socketFdForPrint);

		if (!socketFd.ok()) {
		ALOGE("%s: No socket FD provided.", __PRETTY_FUNCTION__);
		return BAD_VALUE;
		}
		if (maxRpcThreads <= 0) {
		ALOGE("%s: RPC is useless with %" PRIu32 " threads.", __PRETTY_FUNCTION__, maxRpcThreads);
		return BAD_VALUE;
		}

		// TODO(b/182914638): RPC and binder should share the same thread pool count.
		size_t binderThreadPoolMaxCount = ProcessState::self()->getThreadPoolMaxThreadCount();
		if (binderThreadPoolMaxCount <= 1) {
		ALOGE("%s: ProcessState thread pool max count is %zu. RPC is disabled for this service "
		@@ -500,8 +492,7 @@ status_t BBinder::setRpcClientDebug(android::base::unique_fd socketFd, uint32_t
		e->mRpcServer->setRootObjectWeak(wp<BBinder>::fromExisting(this));
		e->mRpcServer->setupExternalServer(std::move(socketFd));
		e->mRpcServer->start();
		LOG_RPC_DETAIL("%s(%d, %" PRIu32 ") successful", __PRETTY_FUNCTION__, socketFdForPrint,
		maxRpcThreads);
		LOG_RPC_DETAIL("%s(fd=%d) successful", __PRETTY_FUNCTION__, socketFdForPrint);
		return OK;
		}

libs/binder/include/binder/Binder.h

+1 −2

Original line number	Diff line number	Diff line
		@@ -101,8 +101,7 @@ public:
		// to another process.
		void setParceled();

		[[nodiscard]] status_t setRpcClientDebug(android::base::unique_fd clientFd,
		uint32_t maxRpcThreads);
		[[nodiscard]] status_t setRpcClientDebug(android::base::unique_fd clientFd);

		protected:
		virtual ~BBinder();

libs/binder/include/binder/IBinder.h

+2 −5

Original line number	Diff line number	Diff line
		@@ -157,12 +157,10 @@ public:
		* Set the RPC client fd to this binder service, for debugging. This is only available on
		* debuggable builds.
		*
		* \|maxRpcThreads\| must be positive because RPC is useless without threads.
		*
		* When this is called on a binder service, the service:
		* 1. sets up RPC server
		* 2. spawns 1 new thread that calls RpcServer::join()
		* - join() spawns at most \|maxRpcThreads\| threads that accept() connections; see RpcServer
		* - join() spawns some number of threads that accept() connections; see RpcServer
		*
		* setRpcClientDebug() may only be called once.
		* TODO(b/182914638): If allow to shut down the client, addRpcClient can be called repeatedly.
		@@ -171,8 +169,7 @@ public:
		* interface freely. See RpcServer::join(). To avoid such race conditions, implement the service
		* functions with multithreading support.
		*/
		[[nodiscard]] status_t setRpcClientDebug(android::base::unique_fd socketFd,
		uint32_t maxRpcThreads);
		[[nodiscard]] status_t setRpcClientDebug(android::base::unique_fd socketFd);

		// NOLINTNEXTLINE(google-default-arguments)
		virtual status_t transact( uint32_t code,

libs/binder/servicedispatcher.cpp

+6 −17

Original line number	Diff line number	Diff line
		@@ -14,7 +14,6 @@
		* limitations under the License.
		*/

		#include <stdint.h>
		#include <sysexits.h>
		#include <unistd.h>

		@@ -22,7 +21,6 @@

		#include <android-base/file.h>
		#include <android-base/logging.h>
		#include <android-base/parseint.h>
		#include <android-base/properties.h>
		#include <android-base/stringprintf.h>
		#include <binder/IServiceManager.h>
		@@ -39,7 +37,6 @@ using android::base::InitLogging;
		using android::base::LogdLogger;
		using android::base::LogId;
		using android::base::LogSeverity;
		using android::base::ParseUint;
		using android::base::StdioLogger;
		using android::base::StringPrintf;

		@@ -47,15 +44,14 @@ namespace {
		int Usage(const char* program) {
		auto format = R"(dispatch calls to RPC service.
		Usage:
		%s [-n <num_threads>] <service_name>
		-n <num_threads>: number of RPC threads added to the service (default 1).
		%s <service_name>
		<service_name>: the service to connect to.
		)";
		LOG(ERROR) << StringPrintf(format, Basename(program).c_str());
		return EX_USAGE;
		}

		int Dispatch(const char* name, uint32_t numThreads) {
		int Dispatch(const char* name) {
		auto sm = defaultServiceManager();
		if (nullptr == sm) {
		LOG(ERROR) << "No servicemanager";
		@@ -78,13 +74,12 @@ int Dispatch(const char* name, uint32_t numThreads) {
		return EX_SOFTWARE;
		}
		auto socket = rpcServer->releaseServer();
		auto status = binder->setRpcClientDebug(std::move(socket), numThreads);
		auto status = binder->setRpcClientDebug(std::move(socket));
		if (status != OK) {
		LOG(ERROR) << "setRpcClientDebug failed with " << statusToString(status);
		return EX_SOFTWARE;
		}
		LOG(INFO) << "Finish setting up RPC on service " << name << " with " << numThreads
		<< " threads on port" << port;
		LOG(INFO) << "Finish setting up RPC on service " << name << " on port" << port;

		std::cout << port << std::endl;
		return EX_OK;
		@@ -117,15 +112,9 @@ int main(int argc, char* argv[]) {
		}
		LOG(WARNING) << "WARNING: servicedispatcher is debug only. Use with caution.";

		uint32_t numThreads = 1;
		int opt;
		while (-1 != (opt = getopt(argc, argv, "n:"))) {
		while (-1 != (opt = getopt(argc, argv, ""))) {
		switch (opt) {
		case 'n': {
		if (!ParseUint(optarg, &numThreads)) {
		return Usage(argv[0]);
		}
		} break;
		default: {
		return Usage(argv[0]);
		}
		@@ -134,5 +123,5 @@ int main(int argc, char* argv[]) {
		if (optind + 1 != argc) return Usage(argv[0]);
		auto name = argv[optind];

		return Dispatch(name, numThreads);
		return Dispatch(name);
		}

libs/binder/tests/binderLibTest.cpp

+32 −41

Original line number	Diff line number	Diff line
		@@ -33,6 +33,7 @@
		#include <android-base/result.h>
		#include <android-base/unique_fd.h>
		#include <binder/Binder.h>
		#include <binder/BpBinder.h>
		#include <binder/IBinder.h>
		#include <binder/IPCThreadState.h>
		#include <binder/IServiceManager.h>
		@@ -112,7 +113,6 @@ enum BinderLibTestTranscationCode {
		BINDER_LIB_TEST_ECHO_VECTOR,
		BINDER_LIB_TEST_REJECT_BUF,
		BINDER_LIB_TEST_CAN_GET_SID,
		BINDER_LIB_TEST_USLEEP,
		BINDER_LIB_TEST_CREATE_TEST_SERVICE,
		};

		@@ -1210,39 +1210,31 @@ public:
		}
		};

		TEST_P(BinderLibRpcTest, SetRpcMaxThreads) {
		TEST_P(BinderLibRpcTest, SetRpcClientDebug) {
		auto binder = GetService();
		ASSERT_TRUE(binder != nullptr);
		auto [socket, port] = CreateSocket();
		ASSERT_TRUE(socket.ok());
		EXPECT_THAT(binder->setRpcClientDebug(std::move(socket), 1), StatusEq(OK));
		EXPECT_THAT(binder->setRpcClientDebug(std::move(socket)), StatusEq(OK));
		}

		TEST_P(BinderLibRpcTest, SetRpcClientNoFd) {
		TEST_P(BinderLibRpcTest, SetRpcClientDebugNoFd) {
		auto binder = GetService();
		ASSERT_TRUE(binder != nullptr);
		EXPECT_THAT(binder->setRpcClientDebug(android::base::unique_fd(), 1), StatusEq(BAD_VALUE));
		EXPECT_THAT(binder->setRpcClientDebug(android::base::unique_fd()), StatusEq(BAD_VALUE));
		}

		TEST_P(BinderLibRpcTest, SetRpcMaxThreadsZero) {
		auto binder = GetService();
		ASSERT_TRUE(binder != nullptr);
		auto [socket, port] = CreateSocket();
		ASSERT_TRUE(socket.ok());
		EXPECT_THAT(binder->setRpcClientDebug(std::move(socket), 0), StatusEq(BAD_VALUE));
		}

		TEST_P(BinderLibRpcTest, SetRpcMaxThreadsTwice) {
		TEST_P(BinderLibRpcTest, SetRpcClientDebugTwice) {
		auto binder = GetService();
		ASSERT_TRUE(binder != nullptr);

		auto [socket1, port1] = CreateSocket();
		ASSERT_TRUE(socket1.ok());
		EXPECT_THAT(binder->setRpcClientDebug(std::move(socket1), 1), StatusEq(OK));
		EXPECT_THAT(binder->setRpcClientDebug(std::move(socket1)), StatusEq(OK));

		auto [socket2, port2] = CreateSocket();
		ASSERT_TRUE(socket2.ok());
		EXPECT_THAT(binder->setRpcClientDebug(std::move(socket2), 1), StatusEq(ALREADY_EXISTS));
		EXPECT_THAT(binder->setRpcClientDebug(std::move(socket2)), StatusEq(ALREADY_EXISTS));
		}

		INSTANTIATE_TEST_CASE_P(BinderLibTest, BinderLibRpcTest, testing::Bool(),
		@@ -1288,42 +1280,47 @@ TEST_P(BinderLibRpcClientTest, Test) {
		auto [socket, socketPort] = CreateSocket();
		ASSERT_TRUE(socket.ok());
		port = socketPort;
		ASSERT_THAT(server->setRpcClientDebug(std::move(socket), numThreads), StatusEq(OK));
		ASSERT_THAT(server->setRpcClientDebug(std::move(socket)), StatusEq(OK));
		}

		auto callUsleep = [](sp<IBinder> server, uint64_t us) {
		Parcel data, reply;
		data.markForBinder(server);
		const char *name = data.isForRpc() ? "RPC" : "binder";
		EXPECT_THAT(data.writeUint64(us), StatusEq(OK));
		EXPECT_THAT(server->transact(BINDER_LIB_TEST_USLEEP, data, &reply), StatusEq(OK))
		<< "for " << name << " server";
		};
		std::mutex mutex;
		std::condition_variable cv;
		bool start = false;

		auto threadFn = [&](size_t threadNum) {
		usleep(threadNum * 50 * 1000); // threadNum * 50ms. Need this to avoid SYN flooding.
		usleep(threadNum * 10 * 1000); // threadNum * 10ms. Need this to avoid SYN flooding.
		auto rpcSession = RpcSession::make();
		ASSERT_TRUE(rpcSession->setupInetClient("127.0.0.1", port));
		auto rpcServerBinder = rpcSession->getRootObject();
		ASSERT_NE(nullptr, rpcServerBinder);

		EXPECT_EQ(OK, rpcServerBinder->pingBinder());

		// Check that \|rpcServerBinder\| and \|server\| points to the same service.
		EXPECT_THAT(GetId(rpcServerBinder), HasValue(id));
		EXPECT_THAT(GetId(rpcServerBinder), HasValue(id)) << "For thread #" << threadNum;

		// Occupy the server thread. The server should still have enough threads to handle
		// other connections.
		// (numThreads - threadNum) * 100ms
		callUsleep(rpcServerBinder, (numThreads - threadNum) * 100 * 1000);
		{
		std::unique_lock<std::mutex> lock(mutex);
		ASSERT_TRUE(cv.wait_for(lock, 1s, [&] { return start; }));
		}
		// Let all threads almost simultaneously ping the service.
		for (size_t i = 0; i < 100; ++i) {
		EXPECT_THAT(rpcServerBinder->pingBinder(), StatusEq(OK))
		<< "For thread #" << threadNum << ", iteration " << i;
		}
		};

		std::vector<std::thread> threads;
		for (size_t i = 0; i < numThreads; ++i) threads.emplace_back(std::bind(threadFn, i));

		{
		std::lock_guard<std::mutex> lock(mutex);
		start = true;
		}
		cv.notify_all();

		for (auto &t : threads) t.join();
		}

		INSTANTIATE_TEST_CASE_P(BinderLibTest, BinderLibRpcClientTest,
		testing::Combine(testing::Bool(), testing::Range(1u, 10u)),
		testing::Combine(testing::Bool(), testing::Values(1u, 10u)),
		BinderLibRpcClientTest::ParamToString);

		class BinderLibTestService : public BBinder {
		@@ -1640,12 +1637,6 @@ public:
		case BINDER_LIB_TEST_CAN_GET_SID: {
		return IPCThreadState::self()->getCallingSid() == nullptr ? BAD_VALUE : NO_ERROR;
		}
		case BINDER_LIB_TEST_USLEEP: {
		uint64_t us;
		if (status_t status = data.readUint64(&us); status != NO_ERROR) return status;
		usleep(us);
		return NO_ERROR;
		}
		case BINDER_LIB_TEST_CREATE_TEST_SERVICE: {
		int32_t id;
		if (status_t status = data.readInt32(&id); status != NO_ERROR) return status;