Merge "Enable -Wconversion in BufferStateLayer."
diff --git a/cmds/dumpsys/dumpsys.cpp b/cmds/dumpsys/dumpsys.cpp
index f8fdaa0..3d2bdf1 100644
--- a/cmds/dumpsys/dumpsys.cpp
+++ b/cmds/dumpsys/dumpsys.cpp
@@ -21,7 +21,9 @@
#include <android-base/file.h>
#include <android-base/stringprintf.h>
+#include <android-base/strings.h>
#include <android-base/unique_fd.h>
+#include <binder/BpBinder.h>
#include <binder/Parcel.h>
#include <binder/ProcessState.h>
#include <binder/Stability.h>
@@ -58,27 +60,30 @@
}
static void usage() {
- fprintf(stderr,
- "usage: dumpsys\n"
- " To dump all services.\n"
- "or:\n"
- " dumpsys [-t TIMEOUT] [--priority LEVEL] [--dump] [--pid] [--thread] [--help | "
- "-l | --skip SERVICES "
- "| SERVICE [ARGS]]\n"
- " --help: shows this help\n"
- " -l: only list services, do not dump them\n"
- " -t TIMEOUT_SEC: TIMEOUT to use in seconds instead of default 10 seconds\n"
- " -T TIMEOUT_MS: TIMEOUT to use in milliseconds instead of default 10 seconds\n"
- " --dump: ask the service to dump itself (this is the default)\n"
- " --pid: dump PID instead of usual dump\n"
- " --proto: filter services that support dumping data in proto format. Dumps\n"
- " will be in proto format.\n"
- " --priority LEVEL: filter services based on specified priority\n"
- " LEVEL must be one of CRITICAL | HIGH | NORMAL\n"
- " --skip SERVICES: dumps all services but SERVICES (comma-separated list)\n"
- " --stability: dump binder stability information instead of usual dump\n"
- " --thread: dump thread usage instead of usual dump\n"
- " SERVICE [ARGS]: dumps only service SERVICE, optionally passing ARGS to it\n");
+ fprintf(
+ stderr,
+ "usage: dumpsys\n"
+ " To dump all services.\n"
+ "or:\n"
+ " dumpsys [-t TIMEOUT] [--priority LEVEL] [--clients] [--dump] [--pid] [--thread] "
+ "[--help | "
+ "-l | --skip SERVICES "
+ "| SERVICE [ARGS]]\n"
+ " --help: shows this help\n"
+ " -l: only list services, do not dump them\n"
+ " -t TIMEOUT_SEC: TIMEOUT to use in seconds instead of default 10 seconds\n"
+ " -T TIMEOUT_MS: TIMEOUT to use in milliseconds instead of default 10 seconds\n"
+ " --clients: dump client PIDs instead of usual dump\n"
+ " --dump: ask the service to dump itself (this is the default)\n"
+ " --pid: dump PID instead of usual dump\n"
+ " --proto: filter services that support dumping data in proto format. Dumps\n"
+ " will be in proto format.\n"
+ " --priority LEVEL: filter services based on specified priority\n"
+ " LEVEL must be one of CRITICAL | HIGH | NORMAL\n"
+ " --skip SERVICES: dumps all services but SERVICES (comma-separated list)\n"
+ " --stability: dump binder stability information instead of usual dump\n"
+ " --thread: dump thread usage instead of usual dump\n"
+ " SERVICE [ARGS]: dumps only service SERVICE, optionally passing ARGS to it\n");
}
static bool IsSkipped(const Vector<String16>& skipped, const String16& service) {
@@ -131,15 +136,12 @@
int dumpTypeFlags = 0;
int timeoutArgMs = 10000;
int priorityFlags = IServiceManager::DUMP_FLAG_PRIORITY_ALL;
- static struct option longOptions[] = {{"help", no_argument, 0, 0},
- {"dump", no_argument, 0, 0},
- {"pid", no_argument, 0, 0},
- {"priority", required_argument, 0, 0},
- {"proto", no_argument, 0, 0},
- {"skip", no_argument, 0, 0},
- {"stability", no_argument, 0, 0},
- {"thread", no_argument, 0, 0},
- {0, 0, 0, 0}};
+ static struct option longOptions[] = {
+ {"help", no_argument, 0, 0}, {"clients", no_argument, 0, 0},
+ {"dump", no_argument, 0, 0}, {"pid", no_argument, 0, 0},
+ {"priority", required_argument, 0, 0}, {"proto", no_argument, 0, 0},
+ {"skip", no_argument, 0, 0}, {"stability", no_argument, 0, 0},
+ {"thread", no_argument, 0, 0}, {0, 0, 0, 0}};
// Must reset optind, otherwise subsequent calls will fail (wouldn't happen on main.cpp, but
// happens on test cases).
@@ -178,6 +180,8 @@
dumpTypeFlags |= TYPE_STABILITY;
} else if (!strcmp(longOptions[optionIndex].name, "thread")) {
dumpTypeFlags |= TYPE_THREAD;
+ } else if (!strcmp(longOptions[optionIndex].name, "clients")) {
+ dumpTypeFlags |= TYPE_CLIENTS;
}
break;
@@ -373,6 +377,35 @@
return OK;
}
+static status_t dumpClientsToFd(const sp<IBinder>& service, const unique_fd& fd) {
+ std::string clientPids;
+ const auto remoteBinder = service->remoteBinder();
+ if (remoteBinder == nullptr) {
+ WriteStringToFd("Client PIDs are not available for local binders.\n", fd.get());
+ return OK;
+ }
+ const auto handle = remoteBinder->getDebugBinderHandle();
+ if (handle == std::nullopt) {
+ return OK;
+ }
+ std::vector<pid_t> pids;
+ pid_t myPid = getpid();
+ pid_t servicePid;
+ status_t status = service->getDebugPid(&servicePid);
+ if (status != OK) {
+ return status;
+ }
+ status =
+ getBinderClientPids(BinderDebugContext::BINDER, myPid, servicePid, handle.value(), &pids);
+ if (status != OK) {
+ return status;
+ }
+ pids.erase(std::remove_if(pids.begin(), pids.end(), [&](pid_t pid) { return pid == myPid; }),
+ pids.end());
+ WriteStringToFd("Client PIDs: " + ::android::base::Join(pids, ", ") + "\n", fd.get());
+ return OK;
+}
+
static void reportDumpError(const String16& serviceName, status_t error, const char* context) {
if (error == OK) return;
@@ -413,6 +446,10 @@
status_t err = dumpThreadsToFd(service, remote_end);
reportDumpError(serviceName, err, "dumping thread info");
}
+ if (dumpTypeFlags & TYPE_CLIENTS) {
+ status_t err = dumpClientsToFd(service, remote_end);
+ reportDumpError(serviceName, err, "dumping clients info");
+ }
// other types always act as a header, this is usually longer
if (dumpTypeFlags & TYPE_DUMP) {
diff --git a/cmds/dumpsys/dumpsys.h b/cmds/dumpsys/dumpsys.h
index 05c5d5e..6ab1a7d 100644
--- a/cmds/dumpsys/dumpsys.h
+++ b/cmds/dumpsys/dumpsys.h
@@ -52,10 +52,11 @@
static void setServiceArgs(Vector<String16>& args, bool asProto, int priorityFlags);
enum Type {
- TYPE_DUMP = 0x1, // dump using `dump` function
- TYPE_PID = 0x2, // dump pid of server only
- TYPE_STABILITY = 0x4, // dump stability information of server
- TYPE_THREAD = 0x8, // dump thread usage of server only
+ TYPE_DUMP = 0x1, // dump using `dump` function
+ TYPE_PID = 0x2, // dump pid of server only
+ TYPE_STABILITY = 0x4, // dump stability information of server
+ TYPE_THREAD = 0x8, // dump thread usage of server only
+ TYPE_CLIENTS = 0x10, // dump pid of clients
};
/**
diff --git a/cmds/dumpsys/tests/dumpsys_test.cpp b/cmds/dumpsys/tests/dumpsys_test.cpp
index 312f4d7..fa63db5 100644
--- a/cmds/dumpsys/tests/dumpsys_test.cpp
+++ b/cmds/dumpsys/tests/dumpsys_test.cpp
@@ -627,6 +627,29 @@
AssertOutputFormat(format);
}
+// Tests 'dumpsys --clients'
+TEST_F(DumpsysTest, ListAllServicesWithClients) {
+ ExpectListServices({"Locksmith", "Valet"});
+ ExpectCheckService("Locksmith");
+ ExpectCheckService("Valet");
+
+ CallMain({"--clients"});
+
+ AssertRunningServices({"Locksmith", "Valet"});
+
+ const std::string format("(.|\n)*((Client PIDs are not available for local binders.)(.|\n)*){2}");
+ AssertOutputFormat(format);
+}
+
+// Tests 'dumpsys --clients service_name'
+TEST_F(DumpsysTest, ListServiceWithClients) {
+ ExpectCheckService("Locksmith");
+
+ CallMain({"--clients", "Locksmith"});
+
+ const std::string format("Client PIDs are not available for local binders.\n");
+ AssertOutputFormat(format);
+}
// Tests 'dumpsys --thread --stability'
TEST_F(DumpsysTest, ListAllServicesWithMultipleOptions) {
ExpectListServices({"Locksmith", "Valet"});
diff --git a/include/ftl/NamedEnum.h b/include/ftl/NamedEnum.h
index f50ff46..6e98fee 100644
--- a/include/ftl/NamedEnum.h
+++ b/include/ftl/NamedEnum.h
@@ -49,6 +49,10 @@
// Example (cont'd): V = android::test::TestEnums::ONE
view = view.substr(valStart);
+ // Check invalid enum values with cast, like V = (android::test::TestEnums)8.
+ if (view.find('(') != std::string::npos) {
+ return std::nullopt;
+ }
size_t nameStart = view.rfind("::");
if (nameStart == std::string::npos) {
return std::nullopt;
diff --git a/libs/binder/Android.bp b/libs/binder/Android.bp
index c18b1ce..2dfe0c9 100644
--- a/libs/binder/Android.bp
+++ b/libs/binder/Android.bp
@@ -101,6 +101,7 @@
"BpBinder.cpp",
"BufferedTextOutput.cpp",
"Debug.cpp",
+ "FdTrigger.cpp",
"IInterface.cpp",
"IMemory.cpp",
"IPCThreadState.cpp",
@@ -356,5 +357,6 @@
"libbinder",
"liblog",
"libutils",
+ "android.debug_aidl-cpp",
],
}
diff --git a/libs/binder/Binder.cpp b/libs/binder/Binder.cpp
index 628381c..d3eef4e 100644
--- a/libs/binder/Binder.cpp
+++ b/libs/binder/Binder.cpp
@@ -555,7 +555,9 @@
return status;
}
rpcServer->setRootObjectWeak(weakThis);
- rpcServer->setupExternalServer(std::move(socketFd));
+ if (auto status = rpcServer->setupExternalServer(std::move(socketFd)); status != OK) {
+ return status;
+ }
rpcServer->setMaxThreads(binderThreadPoolMaxCount);
rpcServer->start();
e->mRpcServerLinks.emplace(link);
diff --git a/libs/binder/BpBinder.cpp b/libs/binder/BpBinder.cpp
index 765e21c..55566e2 100644
--- a/libs/binder/BpBinder.cpp
+++ b/libs/binder/BpBinder.cpp
@@ -188,6 +188,14 @@
return std::get<BinderHandle>(mHandle).handle;
}
+std::optional<int32_t> BpBinder::getDebugBinderHandle() const {
+ if (!isRpcBinder()) {
+ return binderHandle();
+ } else {
+ return std::nullopt;
+ }
+}
+
bool BpBinder::isDescriptorCached() const {
Mutex::Autolock _l(mLock);
return mDescriptorCache.size() ? true : false;
diff --git a/libs/binder/FdTrigger.cpp b/libs/binder/FdTrigger.cpp
new file mode 100644
index 0000000..ecf13dc
--- /dev/null
+++ b/libs/binder/FdTrigger.cpp
@@ -0,0 +1,62 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#define LOG_TAG "FdTrigger"
+#include <log/log.h>
+
+#include <poll.h>
+
+#include <android-base/macros.h>
+
+#include "FdTrigger.h"
+namespace android {
+
+std::unique_ptr<FdTrigger> FdTrigger::make() {
+ auto ret = std::make_unique<FdTrigger>();
+ if (!android::base::Pipe(&ret->mRead, &ret->mWrite)) {
+ ALOGE("Could not create pipe %s", strerror(errno));
+ return nullptr;
+ }
+ return ret;
+}
+
+void FdTrigger::trigger() {
+ mWrite.reset();
+}
+
+bool FdTrigger::isTriggered() {
+ return mWrite == -1;
+}
+
+status_t FdTrigger::triggerablePoll(base::borrowed_fd fd, int16_t event) {
+ while (true) {
+ pollfd pfd[]{{.fd = fd.get(), .events = static_cast<int16_t>(event), .revents = 0},
+ {.fd = mRead.get(), .events = 0, .revents = 0}};
+ int ret = TEMP_FAILURE_RETRY(poll(pfd, arraysize(pfd), -1));
+ if (ret < 0) {
+ return -errno;
+ }
+ if (ret == 0) {
+ continue;
+ }
+ if (pfd[1].revents & POLLHUP) {
+ return -ECANCELED;
+ }
+ return pfd[0].revents & event ? OK : DEAD_OBJECT;
+ }
+}
+
+} // namespace android
diff --git a/libs/binder/FdTrigger.h b/libs/binder/FdTrigger.h
new file mode 100644
index 0000000..984e685
--- /dev/null
+++ b/libs/binder/FdTrigger.h
@@ -0,0 +1,56 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <memory>
+
+#include <android-base/unique_fd.h>
+#include <utils/Errors.h>
+
+namespace android {
+
+/** This is not a pipe. */
+class FdTrigger {
+public:
+ /** Returns nullptr for error case */
+ static std::unique_ptr<FdTrigger> make();
+
+ /**
+ * Close the write end of the pipe so that the read end receives POLLHUP.
+ * Not threadsafe.
+ */
+ void trigger();
+
+ /**
+ * Whether this has been triggered.
+ */
+ bool isTriggered();
+
+ /**
+ * Poll for a read event.
+ *
+ * event - for pollfd
+ *
+ * Return:
+ * true - time to read!
+ * false - trigger happened
+ */
+ status_t triggerablePoll(base::borrowed_fd fd, int16_t event);
+
+private:
+ base::unique_fd mWrite;
+ base::unique_fd mRead;
+};
+} // namespace android
diff --git a/libs/binder/RpcServer.cpp b/libs/binder/RpcServer.cpp
index 879e462..a20445b 100644
--- a/libs/binder/RpcServer.cpp
+++ b/libs/binder/RpcServer.cpp
@@ -29,6 +29,7 @@
#include <binder/RpcTransportRaw.h>
#include <log/log.h>
+#include "FdTrigger.h"
#include "RpcSocketAddress.h"
#include "RpcState.h"
#include "RpcWireFormat.h"
@@ -55,25 +56,25 @@
mAgreedExperimental = true;
}
-bool RpcServer::setupUnixDomainServer(const char* path) {
+status_t RpcServer::setupUnixDomainServer(const char* path) {
return setupSocketServer(UnixSocketAddress(path));
}
-bool RpcServer::setupVsockServer(unsigned int port) {
+status_t RpcServer::setupVsockServer(unsigned int port) {
// realizing value w/ this type at compile time to avoid ubsan abort
constexpr unsigned int kAnyCid = VMADDR_CID_ANY;
return setupSocketServer(VsockSocketAddress(kAnyCid, port));
}
-bool RpcServer::setupInetServer(const char* address, unsigned int port,
- unsigned int* assignedPort) {
+status_t RpcServer::setupInetServer(const char* address, unsigned int port,
+ unsigned int* assignedPort) {
if (assignedPort != nullptr) *assignedPort = 0;
auto aiStart = InetSocketAddress::getAddrInfo(address, port);
- if (aiStart == nullptr) return false;
+ if (aiStart == nullptr) return UNKNOWN_ERROR;
for (auto ai = aiStart.get(); ai != nullptr; ai = ai->ai_next) {
InetSocketAddress socketAddress(ai->ai_addr, ai->ai_addrlen, address, port);
- if (!setupSocketServer(socketAddress)) {
+ if (status_t status = setupSocketServer(socketAddress); status != OK) {
continue;
}
@@ -84,7 +85,7 @@
int savedErrno = errno;
ALOGE("Could not getsockname at %s: %s", socketAddress.toString().c_str(),
strerror(savedErrno));
- return false;
+ return -savedErrno;
}
LOG_ALWAYS_FATAL_IF(len != sizeof(addr), "Wrong socket type: len %zu vs len %zu",
static_cast<size_t>(len), sizeof(addr));
@@ -97,11 +98,11 @@
*assignedPort = realPort;
}
- return true;
+ return OK;
}
ALOGE("None of the socket address resolved for %s:%u can be set up as inet server.", address,
port);
- return false;
+ return UNKNOWN_ERROR;
}
void RpcServer::setMaxThreads(size_t threads) {
@@ -156,7 +157,7 @@
LOG_ALWAYS_FATAL_IF(!mServer.ok(), "RpcServer must be setup to join.");
LOG_ALWAYS_FATAL_IF(mShutdownTrigger != nullptr, "Already joined");
mJoinThreadRunning = true;
- mShutdownTrigger = RpcSession::FdTrigger::make();
+ mShutdownTrigger = FdTrigger::make();
LOG_ALWAYS_FATAL_IF(mShutdownTrigger == nullptr, "Cannot create join signaler");
mCtx = mRpcTransportCtxFactory->newServerCtx();
@@ -167,7 +168,7 @@
status_t status;
while ((status = mShutdownTrigger->triggerablePoll(mServer, POLLIN)) == OK) {
unique_fd clientFd(TEMP_FAILURE_RETRY(
- accept4(mServer.get(), nullptr, nullptr /*length*/, SOCK_CLOEXEC)));
+ accept4(mServer.get(), nullptr, nullptr /*length*/, SOCK_CLOEXEC | SOCK_NONBLOCK)));
if (clientFd < 0) {
ALOGE("Could not accept4 socket: %s", strerror(errno));
@@ -259,7 +260,7 @@
status_t status = OK;
int clientFdForLog = clientFd.get();
- auto client = server->mCtx->newTransport(std::move(clientFd));
+ auto client = server->mCtx->newTransport(std::move(clientFd), server->mShutdownTrigger.get());
if (client == nullptr) {
ALOGE("Dropping accept4()-ed socket because sslAccept fails");
status = DEAD_OBJECT;
@@ -270,8 +271,8 @@
RpcConnectionHeader header;
if (status == OK) {
- status = server->mShutdownTrigger->interruptableReadFully(client.get(), &header,
- sizeof(header));
+ status = client->interruptableReadFully(server->mShutdownTrigger.get(), &header,
+ sizeof(header));
if (status != OK) {
ALOGE("Failed to read ID for client connecting to RPC server: %s",
statusToString(status).c_str());
@@ -296,8 +297,8 @@
.version = protocolVersion,
};
- status = server->mShutdownTrigger->interruptableWriteFully(client.get(), &response,
- sizeof(response));
+ status = client->interruptableWriteFully(server->mShutdownTrigger.get(), &response,
+ sizeof(response));
if (status != OK) {
ALOGE("Failed to send new session response: %s", statusToString(status).c_str());
// still need to cleanup before we can return
@@ -366,7 +367,7 @@
}
if (incoming) {
- LOG_ALWAYS_FATAL_IF(!session->addOutgoingConnection(std::move(client), true),
+ LOG_ALWAYS_FATAL_IF(OK != session->addOutgoingConnection(std::move(client), true),
"server state must already be initialized");
return;
}
@@ -383,21 +384,22 @@
RpcSession::join(std::move(session), std::move(setupResult));
}
-bool RpcServer::setupSocketServer(const RpcSocketAddress& addr) {
+status_t RpcServer::setupSocketServer(const RpcSocketAddress& addr) {
LOG_RPC_DETAIL("Setting up socket server %s", addr.toString().c_str());
LOG_ALWAYS_FATAL_IF(hasServer(), "Each RpcServer can only have one server.");
- unique_fd serverFd(
- TEMP_FAILURE_RETRY(socket(addr.addr()->sa_family, SOCK_STREAM | SOCK_CLOEXEC, 0)));
+ unique_fd serverFd(TEMP_FAILURE_RETRY(
+ socket(addr.addr()->sa_family, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0)));
if (serverFd == -1) {
- ALOGE("Could not create socket: %s", strerror(errno));
- return false;
+ int savedErrno = errno;
+ ALOGE("Could not create socket: %s", strerror(savedErrno));
+ return -savedErrno;
}
if (0 != TEMP_FAILURE_RETRY(bind(serverFd.get(), addr.addr(), addr.addrSize()))) {
int savedErrno = errno;
ALOGE("Could not bind socket at %s: %s", addr.toString().c_str(), strerror(savedErrno));
- return false;
+ return -savedErrno;
}
// Right now, we create all threads at once, making accept4 slow. To avoid hanging the client,
@@ -407,16 +409,16 @@
if (0 != TEMP_FAILURE_RETRY(listen(serverFd.get(), 50 /*backlog*/))) {
int savedErrno = errno;
ALOGE("Could not listen socket at %s: %s", addr.toString().c_str(), strerror(savedErrno));
- return false;
+ return -savedErrno;
}
LOG_RPC_DETAIL("Successfully setup socket server %s", addr.toString().c_str());
- if (!setupExternalServer(std::move(serverFd))) {
+ if (status_t status = setupExternalServer(std::move(serverFd)); status != OK) {
ALOGE("Another thread has set up server while calling setupSocketServer. Race?");
- return false;
+ return status;
}
- return true;
+ return OK;
}
void RpcServer::onSessionAllIncomingThreadsEnded(const sp<RpcSession>& session) {
@@ -449,15 +451,15 @@
return std::move(mServer);
}
-bool RpcServer::setupExternalServer(base::unique_fd serverFd) {
+status_t RpcServer::setupExternalServer(base::unique_fd serverFd) {
LOG_ALWAYS_FATAL_IF(!mAgreedExperimental, "no!");
std::lock_guard<std::mutex> _l(mLock);
if (mServer.ok()) {
ALOGE("Each RpcServer can only have one server.");
- return false;
+ return INVALID_OPERATION;
}
mServer = std::move(serverFd);
- return true;
+ return OK;
}
} // namespace android
diff --git a/libs/binder/RpcSession.cpp b/libs/binder/RpcSession.cpp
index 7565f1b..4c47005 100644
--- a/libs/binder/RpcSession.cpp
+++ b/libs/binder/RpcSession.cpp
@@ -35,9 +35,11 @@
#include <jni.h>
#include <utils/String8.h>
+#include "FdTrigger.h"
#include "RpcSocketAddress.h"
#include "RpcState.h"
#include "RpcWireFormat.h"
+#include "Utils.h"
#ifdef __GLIBC__
extern "C" pid_t gettid();
@@ -107,54 +109,61 @@
return mProtocolVersion;
}
-bool RpcSession::setupUnixDomainClient(const char* path) {
+status_t RpcSession::setupUnixDomainClient(const char* path) {
return setupSocketClient(UnixSocketAddress(path));
}
-bool RpcSession::setupVsockClient(unsigned int cid, unsigned int port) {
+status_t RpcSession::setupVsockClient(unsigned int cid, unsigned int port) {
return setupSocketClient(VsockSocketAddress(cid, port));
}
-bool RpcSession::setupInetClient(const char* addr, unsigned int port) {
+status_t RpcSession::setupInetClient(const char* addr, unsigned int port) {
auto aiStart = InetSocketAddress::getAddrInfo(addr, port);
- if (aiStart == nullptr) return false;
+ if (aiStart == nullptr) return UNKNOWN_ERROR;
for (auto ai = aiStart.get(); ai != nullptr; ai = ai->ai_next) {
InetSocketAddress socketAddress(ai->ai_addr, ai->ai_addrlen, addr, port);
- if (setupSocketClient(socketAddress)) return true;
+ if (status_t status = setupSocketClient(socketAddress); status == OK) return OK;
}
ALOGE("None of the socket address resolved for %s:%u can be added as inet client.", addr, port);
- return false;
+ return NAME_NOT_FOUND;
}
-bool RpcSession::setupPreconnectedClient(unique_fd fd, std::function<unique_fd()>&& request) {
- return setupClient([&](const RpcAddress& sessionId, bool incoming) {
+status_t RpcSession::setupPreconnectedClient(unique_fd fd, std::function<unique_fd()>&& request) {
+ return setupClient([&](const RpcAddress& sessionId, bool incoming) -> status_t {
// std::move'd from fd becomes -1 (!ok())
if (!fd.ok()) {
fd = request();
- if (!fd.ok()) return false;
+ if (!fd.ok()) return BAD_VALUE;
+ }
+ if (auto res = setNonBlocking(fd); !res.ok()) {
+ ALOGE("setupPreconnectedClient: %s", res.error().message().c_str());
+ return res.error().code() == 0 ? UNKNOWN_ERROR : -res.error().code();
}
return initAndAddConnection(std::move(fd), sessionId, incoming);
});
}
-bool RpcSession::addNullDebuggingClient() {
+status_t RpcSession::addNullDebuggingClient() {
// Note: only works on raw sockets.
+ if (auto status = initShutdownTrigger(); status != OK) return status;
+
unique_fd serverFd(TEMP_FAILURE_RETRY(open("/dev/null", O_WRONLY | O_CLOEXEC)));
if (serverFd == -1) {
- ALOGE("Could not connect to /dev/null: %s", strerror(errno));
- return false;
+ int savedErrno = errno;
+ ALOGE("Could not connect to /dev/null: %s", strerror(savedErrno));
+ return -savedErrno;
}
auto ctx = mRpcTransportCtxFactory->newClientCtx();
if (ctx == nullptr) {
ALOGE("Unable to create RpcTransportCtx for null debugging client");
- return false;
+ return NO_MEMORY;
}
- auto server = ctx->newTransport(std::move(serverFd));
+ auto server = ctx->newTransport(std::move(serverFd), mShutdownTrigger.get());
if (server == nullptr) {
ALOGE("Unable to set up RpcTransport");
- return false;
+ return UNKNOWN_ERROR;
}
return addOutgoingConnection(std::move(server), false);
}
@@ -215,91 +224,6 @@
return state()->sendDecStrong(connection.get(), sp<RpcSession>::fromExisting(this), address);
}
-std::unique_ptr<RpcSession::FdTrigger> RpcSession::FdTrigger::make() {
- auto ret = std::make_unique<RpcSession::FdTrigger>();
- if (!android::base::Pipe(&ret->mRead, &ret->mWrite)) {
- ALOGE("Could not create pipe %s", strerror(errno));
- return nullptr;
- }
- return ret;
-}
-
-void RpcSession::FdTrigger::trigger() {
- mWrite.reset();
-}
-
-bool RpcSession::FdTrigger::isTriggered() {
- return mWrite == -1;
-}
-
-status_t RpcSession::FdTrigger::triggerablePoll(RpcTransport* rpcTransport, int16_t event) {
- return triggerablePoll(rpcTransport->pollSocket(), event);
-}
-
-status_t RpcSession::FdTrigger::triggerablePoll(base::borrowed_fd fd, int16_t event) {
- while (true) {
- pollfd pfd[]{{.fd = fd.get(), .events = static_cast<int16_t>(event), .revents = 0},
- {.fd = mRead.get(), .events = POLLHUP, .revents = 0}};
- int ret = TEMP_FAILURE_RETRY(poll(pfd, arraysize(pfd), -1));
- if (ret < 0) {
- return -errno;
- }
- if (ret == 0) {
- continue;
- }
- if (pfd[1].revents & POLLHUP) {
- return -ECANCELED;
- }
- return pfd[0].revents & event ? OK : DEAD_OBJECT;
- }
-}
-
-status_t RpcSession::FdTrigger::interruptableWriteFully(RpcTransport* rpcTransport,
- const void* data, size_t size) {
- const uint8_t* buffer = reinterpret_cast<const uint8_t*>(data);
- const uint8_t* end = buffer + size;
-
- MAYBE_WAIT_IN_FLAKE_MODE;
-
- status_t status;
- while ((status = triggerablePoll(rpcTransport, POLLOUT)) == OK) {
- auto writeSize = rpcTransport->send(buffer, end - buffer);
- if (!writeSize.ok()) {
- LOG_RPC_DETAIL("RpcTransport::send(): %s", writeSize.error().message().c_str());
- return writeSize.error().code() == 0 ? UNKNOWN_ERROR : -writeSize.error().code();
- }
-
- if (*writeSize == 0) return DEAD_OBJECT;
-
- buffer += *writeSize;
- if (buffer == end) return OK;
- }
- return status;
-}
-
-status_t RpcSession::FdTrigger::interruptableReadFully(RpcTransport* rpcTransport, void* data,
- size_t size) {
- uint8_t* buffer = reinterpret_cast<uint8_t*>(data);
- uint8_t* end = buffer + size;
-
- MAYBE_WAIT_IN_FLAKE_MODE;
-
- status_t status;
- while ((status = triggerablePoll(rpcTransport, POLLIN)) == OK) {
- auto readSize = rpcTransport->recv(buffer, end - buffer);
- if (!readSize.ok()) {
- LOG_RPC_DETAIL("RpcTransport::recv(): %s", readSize.error().message().c_str());
- return readSize.error().code() == 0 ? UNKNOWN_ERROR : -readSize.error().code();
- }
-
- if (*readSize == 0) return DEAD_OBJECT; // EOF
-
- buffer += *readSize;
- if (buffer == end) return OK;
- }
- return status;
-}
-
status_t RpcSession::readId() {
{
std::lock_guard<std::mutex> _l(mMutex);
@@ -475,43 +399,48 @@
return server;
}
-bool RpcSession::setupClient(
- const std::function<bool(const RpcAddress& sessionId, bool incoming)>& connectAndInit) {
+status_t RpcSession::setupClient(
+ const std::function<status_t(const RpcAddress& sessionId, bool incoming)>& connectAndInit) {
{
std::lock_guard<std::mutex> _l(mMutex);
LOG_ALWAYS_FATAL_IF(mOutgoingConnections.size() != 0,
"Must only setup session once, but already has %zu clients",
mOutgoingConnections.size());
}
+ if (auto status = initShutdownTrigger(); status != OK) return status;
- if (!connectAndInit(RpcAddress::zero(), false /*incoming*/)) return false;
+ if (status_t status = connectAndInit(RpcAddress::zero(), false /*incoming*/); status != OK)
+ return status;
{
ExclusiveConnection connection;
- status_t status = ExclusiveConnection::find(sp<RpcSession>::fromExisting(this),
- ConnectionUse::CLIENT, &connection);
- if (status != OK) return false;
+ if (status_t status = ExclusiveConnection::find(sp<RpcSession>::fromExisting(this),
+ ConnectionUse::CLIENT, &connection);
+ status != OK)
+ return status;
uint32_t version;
- status = state()->readNewSessionResponse(connection.get(),
- sp<RpcSession>::fromExisting(this), &version);
- if (!setProtocolVersion(version)) return false;
+ if (status_t status =
+ state()->readNewSessionResponse(connection.get(),
+ sp<RpcSession>::fromExisting(this), &version);
+ status != OK)
+ return status;
+ if (!setProtocolVersion(version)) return BAD_VALUE;
}
// TODO(b/189955605): we should add additional sessions dynamically
// instead of all at once.
- // TODO(b/186470974): first risk of blocking
size_t numThreadsAvailable;
if (status_t status = getRemoteMaxThreads(&numThreadsAvailable); status != OK) {
ALOGE("Could not get max threads after initial session setup: %s",
statusToString(status).c_str());
- return false;
+ return status;
}
if (status_t status = readId(); status != OK) {
ALOGE("Could not get session id after initial session setup: %s",
statusToString(status).c_str());
- return false;
+ return status;
}
// TODO(b/189955605): we should add additional sessions dynamically
@@ -522,34 +451,36 @@
// we've already setup one client
for (size_t i = 0; i + 1 < numThreadsAvailable; i++) {
- if (!connectAndInit(mId.value(), false /*incoming*/)) return false;
+ if (status_t status = connectAndInit(mId.value(), false /*incoming*/); status != OK)
+ return status;
}
for (size_t i = 0; i < mMaxThreads; i++) {
- if (!connectAndInit(mId.value(), true /*incoming*/)) return false;
+ if (status_t status = connectAndInit(mId.value(), true /*incoming*/); status != OK)
+ return status;
}
- return true;
+ return OK;
}
-bool RpcSession::setupSocketClient(const RpcSocketAddress& addr) {
+status_t RpcSession::setupSocketClient(const RpcSocketAddress& addr) {
return setupClient([&](const RpcAddress& sessionId, bool incoming) {
return setupOneSocketConnection(addr, sessionId, incoming);
});
}
-bool RpcSession::setupOneSocketConnection(const RpcSocketAddress& addr, const RpcAddress& sessionId,
- bool incoming) {
+status_t RpcSession::setupOneSocketConnection(const RpcSocketAddress& addr,
+ const RpcAddress& sessionId, bool incoming) {
for (size_t tries = 0; tries < 5; tries++) {
if (tries > 0) usleep(10000);
- unique_fd serverFd(
- TEMP_FAILURE_RETRY(socket(addr.addr()->sa_family, SOCK_STREAM | SOCK_CLOEXEC, 0)));
+ unique_fd serverFd(TEMP_FAILURE_RETRY(
+ socket(addr.addr()->sa_family, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0)));
if (serverFd == -1) {
int savedErrno = errno;
ALOGE("Could not create socket at %s: %s", addr.toString().c_str(),
strerror(savedErrno));
- return false;
+ return -savedErrno;
}
if (0 != TEMP_FAILURE_RETRY(connect(serverFd.get(), addr.addr(), addr.addrSize()))) {
@@ -557,10 +488,34 @@
ALOGW("Connection reset on %s", addr.toString().c_str());
continue;
}
- int savedErrno = errno;
- ALOGE("Could not connect socket at %s: %s", addr.toString().c_str(),
- strerror(savedErrno));
- return false;
+ if (errno != EAGAIN && errno != EINPROGRESS) {
+ int savedErrno = errno;
+ ALOGE("Could not connect socket at %s: %s", addr.toString().c_str(),
+ strerror(savedErrno));
+ return -savedErrno;
+ }
+ // For non-blocking sockets, connect() may return EAGAIN (for unix domain socket) or
+ // EINPROGRESS (for others). Call poll() and getsockopt() to get the error.
+ status_t pollStatus = mShutdownTrigger->triggerablePoll(serverFd, POLLOUT);
+ if (pollStatus != OK) {
+ ALOGE("Could not POLLOUT after connect() on non-blocking socket: %s",
+ statusToString(pollStatus).c_str());
+ return pollStatus;
+ }
+ int soError;
+ socklen_t soErrorLen = sizeof(soError);
+ int ret = getsockopt(serverFd.get(), SOL_SOCKET, SO_ERROR, &soError, &soErrorLen);
+ if (ret == -1) {
+ int savedErrno = errno;
+ ALOGE("Could not getsockopt() after connect() on non-blocking socket: %s",
+ strerror(savedErrno));
+ return -savedErrno;
+ }
+ if (soError != 0) {
+ ALOGE("After connect(), getsockopt() returns error for socket at %s: %s",
+ addr.toString().c_str(), strerror(soError));
+ return -soError;
+ }
}
LOG_RPC_DETAIL("Socket at %s client with fd %d", addr.toString().c_str(), serverFd.get());
@@ -568,20 +523,22 @@
}
ALOGE("Ran out of retries to connect to %s", addr.toString().c_str());
- return false;
+ return UNKNOWN_ERROR;
}
-bool RpcSession::initAndAddConnection(unique_fd fd, const RpcAddress& sessionId, bool incoming) {
+status_t RpcSession::initAndAddConnection(unique_fd fd, const RpcAddress& sessionId,
+ bool incoming) {
+ LOG_ALWAYS_FATAL_IF(mShutdownTrigger == nullptr);
auto ctx = mRpcTransportCtxFactory->newClientCtx();
if (ctx == nullptr) {
ALOGE("Unable to create client RpcTransportCtx with %s sockets",
mRpcTransportCtxFactory->toCString());
- return false;
+ return NO_MEMORY;
}
- auto server = ctx->newTransport(std::move(fd));
+ auto server = ctx->newTransport(std::move(fd), mShutdownTrigger.get());
if (server == nullptr) {
ALOGE("Unable to set up RpcTransport in %s context", mRpcTransportCtxFactory->toCString());
- return false;
+ return UNKNOWN_ERROR;
}
LOG_RPC_DETAIL("Socket at client with RpcTransport %p", server.get());
@@ -594,16 +551,12 @@
if (incoming) header.options |= RPC_CONNECTION_OPTION_INCOMING;
- auto sentHeader = server->send(&header, sizeof(header));
- if (!sentHeader.ok()) {
+ auto sendHeaderStatus =
+ server->interruptableWriteFully(mShutdownTrigger.get(), &header, sizeof(header));
+ if (sendHeaderStatus != OK) {
ALOGE("Could not write connection header to socket: %s",
- sentHeader.error().message().c_str());
- return false;
- }
- if (*sentHeader != sizeof(header)) {
- ALOGE("Could not write connection header to socket: sent %zd bytes, expected %zd",
- *sentHeader, sizeof(header));
- return false;
+ statusToString(sendHeaderStatus).c_str());
+ return sendHeaderStatus;
}
LOG_RPC_DETAIL("Socket at client: header sent");
@@ -615,7 +568,7 @@
}
}
-bool RpcSession::addIncomingConnection(std::unique_ptr<RpcTransport> rpcTransport) {
+status_t RpcSession::addIncomingConnection(std::unique_ptr<RpcTransport> rpcTransport) {
std::mutex mutex;
std::condition_variable joinCv;
std::unique_lock<std::mutex> lock(mutex);
@@ -641,22 +594,24 @@
});
joinCv.wait(lock, [&] { return ownershipTransferred; });
LOG_ALWAYS_FATAL_IF(!ownershipTransferred);
- return true;
+ return OK;
}
-bool RpcSession::addOutgoingConnection(std::unique_ptr<RpcTransport> rpcTransport, bool init) {
+status_t RpcSession::initShutdownTrigger() {
+ // first client connection added, but setForServer not called, so
+ // initializaing for a client.
+ if (mShutdownTrigger == nullptr) {
+ mShutdownTrigger = FdTrigger::make();
+ mEventListener = mShutdownListener = sp<WaitForShutdownListener>::make();
+ if (mShutdownTrigger == nullptr) return INVALID_OPERATION;
+ }
+ return OK;
+}
+
+status_t RpcSession::addOutgoingConnection(std::unique_ptr<RpcTransport> rpcTransport, bool init) {
sp<RpcConnection> connection = sp<RpcConnection>::make();
{
std::lock_guard<std::mutex> _l(mMutex);
-
- // first client connection added, but setForServer not called, so
- // initializaing for a client.
- if (mShutdownTrigger == nullptr) {
- mShutdownTrigger = FdTrigger::make();
- mEventListener = mShutdownListener = sp<WaitForShutdownListener>::make();
- if (mShutdownTrigger == nullptr) return false;
- }
-
connection->rpcTransport = std::move(rpcTransport);
connection->exclusiveTid = gettid();
mOutgoingConnections.push_back(connection);
@@ -672,7 +627,7 @@
connection->exclusiveTid = std::nullopt;
}
- return status == OK;
+ return status;
}
bool RpcSession::setForServer(const wp<RpcServer>& server, const wp<EventListener>& eventListener,
diff --git a/libs/binder/RpcState.cpp b/libs/binder/RpcState.cpp
index 23382c3..b58f1b3 100644
--- a/libs/binder/RpcState.cpp
+++ b/libs/binder/RpcState.cpp
@@ -283,8 +283,8 @@
}
if (status_t status =
- session->mShutdownTrigger->interruptableWriteFully(connection->rpcTransport.get(),
- data, size);
+ connection->rpcTransport->interruptableWriteFully(session->mShutdownTrigger.get(),
+ data, size);
status != OK) {
LOG_RPC_DETAIL("Failed to write %s (%zu bytes) on RpcTransport %p, error: %s", what, size,
connection->rpcTransport.get(), statusToString(status).c_str());
@@ -305,8 +305,8 @@
}
if (status_t status =
- session->mShutdownTrigger->interruptableReadFully(connection->rpcTransport.get(),
- data, size);
+ connection->rpcTransport->interruptableReadFully(session->mShutdownTrigger.get(),
+ data, size);
status != OK) {
LOG_RPC_DETAIL("Failed to read %s (%zu bytes) on RpcTransport %p, error: %s", what, size,
connection->rpcTransport.get(), statusToString(status).c_str());
diff --git a/libs/binder/RpcTransportRaw.cpp b/libs/binder/RpcTransportRaw.cpp
index 2fc1945..d77fc52 100644
--- a/libs/binder/RpcTransportRaw.cpp
+++ b/libs/binder/RpcTransportRaw.cpp
@@ -17,8 +17,11 @@
#define LOG_TAG "RpcRawTransport"
#include <log/log.h>
+#include <poll.h>
+
#include <binder/RpcTransportRaw.h>
+#include "FdTrigger.h"
#include "RpcState.h"
using android::base::ErrnoError;
@@ -32,14 +35,14 @@
class RpcTransportRaw : public RpcTransport {
public:
explicit RpcTransportRaw(android::base::unique_fd socket) : mSocket(std::move(socket)) {}
- Result<size_t> send(const void *buf, size_t size) override {
+ Result<size_t> send(const void* buf, size_t size) {
ssize_t ret = TEMP_FAILURE_RETRY(::send(mSocket.get(), buf, size, MSG_NOSIGNAL));
if (ret < 0) {
return ErrnoError() << "send()";
}
return ret;
}
- Result<size_t> recv(void *buf, size_t size) override {
+ Result<size_t> recv(void* buf, size_t size) {
ssize_t ret = TEMP_FAILURE_RETRY(::recv(mSocket.get(), buf, size, MSG_NOSIGNAL));
if (ret < 0) {
return ErrnoError() << "recv()";
@@ -47,14 +50,56 @@
return ret;
}
Result<size_t> peek(void *buf, size_t size) override {
- ssize_t ret = TEMP_FAILURE_RETRY(::recv(mSocket.get(), buf, size, MSG_PEEK | MSG_DONTWAIT));
+ ssize_t ret = TEMP_FAILURE_RETRY(::recv(mSocket.get(), buf, size, MSG_PEEK));
if (ret < 0) {
return ErrnoError() << "recv(MSG_PEEK)";
}
return ret;
}
- bool pending() override { return false; }
- android::base::borrowed_fd pollSocket() const override { return mSocket; }
+
+ status_t interruptableWriteFully(FdTrigger* fdTrigger, const void* data, size_t size) override {
+ const uint8_t* buffer = reinterpret_cast<const uint8_t*>(data);
+ const uint8_t* end = buffer + size;
+
+ MAYBE_WAIT_IN_FLAKE_MODE;
+
+ status_t status;
+ while ((status = fdTrigger->triggerablePoll(mSocket.get(), POLLOUT)) == OK) {
+ auto writeSize = this->send(buffer, end - buffer);
+ if (!writeSize.ok()) {
+ LOG_RPC_DETAIL("RpcTransport::send(): %s", writeSize.error().message().c_str());
+ return writeSize.error().code() == 0 ? UNKNOWN_ERROR : -writeSize.error().code();
+ }
+
+ if (*writeSize == 0) return DEAD_OBJECT;
+
+ buffer += *writeSize;
+ if (buffer == end) return OK;
+ }
+ return status;
+ }
+
+ status_t interruptableReadFully(FdTrigger* fdTrigger, void* data, size_t size) override {
+ uint8_t* buffer = reinterpret_cast<uint8_t*>(data);
+ uint8_t* end = buffer + size;
+
+ MAYBE_WAIT_IN_FLAKE_MODE;
+
+ status_t status;
+ while ((status = fdTrigger->triggerablePoll(mSocket.get(), POLLIN)) == OK) {
+ auto readSize = this->recv(buffer, end - buffer);
+ if (!readSize.ok()) {
+ LOG_RPC_DETAIL("RpcTransport::recv(): %s", readSize.error().message().c_str());
+ return readSize.error().code() == 0 ? UNKNOWN_ERROR : -readSize.error().code();
+ }
+
+ if (*readSize == 0) return DEAD_OBJECT; // EOF
+
+ buffer += *readSize;
+ if (buffer == end) return OK;
+ }
+ return status;
+ }
private:
android::base::unique_fd mSocket;
@@ -63,7 +108,7 @@
// RpcTransportCtx with TLS disabled.
class RpcTransportCtxRaw : public RpcTransportCtx {
public:
- std::unique_ptr<RpcTransport> newTransport(android::base::unique_fd fd) const {
+ std::unique_ptr<RpcTransport> newTransport(android::base::unique_fd fd, FdTrigger*) const {
return std::make_unique<RpcTransportRaw>(std::move(fd));
}
};
diff --git a/libs/binder/ServiceManagerHost.cpp b/libs/binder/ServiceManagerHost.cpp
index 1c2f9b4..59334b7 100644
--- a/libs/binder/ServiceManagerHost.cpp
+++ b/libs/binder/ServiceManagerHost.cpp
@@ -158,8 +158,10 @@
LOG_ALWAYS_FATAL_IF(!forwardResult->hostPort().has_value());
auto rpcSession = RpcSession::make();
- if (!rpcSession->setupInetClient("127.0.0.1", *forwardResult->hostPort())) {
- ALOGE("Unable to set up inet client on host port %u", *forwardResult->hostPort());
+ if (status_t status = rpcSession->setupInetClient("127.0.0.1", *forwardResult->hostPort());
+ status != OK) {
+ ALOGE("Unable to set up inet client on host port %u: %s", *forwardResult->hostPort(),
+ statusToString(status).c_str());
return nullptr;
}
auto binder = rpcSession->getRootObject();
diff --git a/libs/binder/Utils.cpp b/libs/binder/Utils.cpp
index 90a4502..d2a5be1 100644
--- a/libs/binder/Utils.cpp
+++ b/libs/binder/Utils.cpp
@@ -18,10 +18,24 @@
#include <string.h>
+using android::base::ErrnoError;
+using android::base::Result;
+
namespace android {
void zeroMemory(uint8_t* data, size_t size) {
memset(data, 0, size);
}
-} // namespace android
+Result<void> setNonBlocking(android::base::borrowed_fd fd) {
+ int flags = TEMP_FAILURE_RETRY(fcntl(fd.get(), F_GETFL));
+ if (flags == -1) {
+ return ErrnoError() << "Could not get flags for fd";
+ }
+ if (int ret = TEMP_FAILURE_RETRY(fcntl(fd.get(), F_SETFL, flags | O_NONBLOCK)); ret == -1) {
+ return ErrnoError() << "Could not set non-blocking flag for fd";
+ }
+ return {};
+}
+
+} // namespace android
diff --git a/libs/binder/Utils.h b/libs/binder/Utils.h
index f94b158..1e383da 100644
--- a/libs/binder/Utils.h
+++ b/libs/binder/Utils.h
@@ -17,9 +17,14 @@
#include <cstdint>
#include <stddef.h>
+#include <android-base/result.h>
+#include <android-base/unique_fd.h>
+
namespace android {
// avoid optimizations
void zeroMemory(uint8_t* data, size_t size);
+android::base::Result<void> setNonBlocking(android::base::borrowed_fd fd);
+
} // namespace android
diff --git a/libs/binder/include/binder/BpBinder.h b/libs/binder/include/binder/BpBinder.h
index c69bb9e..a6d35c7 100644
--- a/libs/binder/include/binder/BpBinder.h
+++ b/libs/binder/include/binder/BpBinder.h
@@ -90,6 +90,8 @@
static void setLimitCallback(binder_proxy_limit_callback cb);
static void setBinderProxyCountWatermarks(int high, int low);
+ std::optional<int32_t> getDebugBinderHandle() const;
+
class ObjectManager {
public:
ObjectManager();
diff --git a/libs/binder/include/binder/RpcServer.h b/libs/binder/include/binder/RpcServer.h
index 4abf3b9..bf3e7e0 100644
--- a/libs/binder/include/binder/RpcServer.h
+++ b/libs/binder/include/binder/RpcServer.h
@@ -32,6 +32,7 @@
namespace android {
+class FdTrigger;
class RpcSocketAddress;
/**
@@ -59,12 +60,12 @@
* process B makes binder b and sends it to A
* A uses this 'back session' to send things back to B
*/
- [[nodiscard]] bool setupUnixDomainServer(const char* path);
+ [[nodiscard]] status_t setupUnixDomainServer(const char* path);
/**
* Creates an RPC server at the current port.
*/
- [[nodiscard]] bool setupVsockServer(unsigned int port);
+ [[nodiscard]] status_t setupVsockServer(unsigned int port);
/**
* Creates an RPC server at the current port using IPv4.
@@ -80,8 +81,8 @@
* "0.0.0.0" allows for connections on any IP address that the device may
* have
*/
- [[nodiscard]] bool setupInetServer(const char* address, unsigned int port,
- unsigned int* assignedPort);
+ [[nodiscard]] status_t setupInetServer(const char* address, unsigned int port,
+ unsigned int* assignedPort);
/**
* If setup*Server has been successful, return true. Otherwise return false.
@@ -97,7 +98,7 @@
* Set up server using an external FD previously set up by releaseServer().
* Return false if there's already a server.
*/
- bool setupExternalServer(base::unique_fd serverFd);
+ [[nodiscard]] status_t setupExternalServer(base::unique_fd serverFd);
void iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction();
@@ -175,7 +176,7 @@
void onSessionIncomingThreadEnded() override;
static void establishConnection(sp<RpcServer>&& server, base::unique_fd clientFd);
- bool setupSocketServer(const RpcSocketAddress& address);
+ status_t setupSocketServer(const RpcSocketAddress& address);
const std::unique_ptr<RpcTransportCtxFactory> mRpcTransportCtxFactory;
bool mAgreedExperimental = false;
@@ -190,7 +191,7 @@
sp<IBinder> mRootObject;
wp<IBinder> mRootObjectWeak;
std::map<RpcAddress, sp<RpcSession>> mSessions;
- std::unique_ptr<RpcSession::FdTrigger> mShutdownTrigger;
+ std::unique_ptr<FdTrigger> mShutdownTrigger;
std::condition_variable mShutdownCv;
std::unique_ptr<RpcTransportCtx> mCtx;
};
diff --git a/libs/binder/include/binder/RpcSession.h b/libs/binder/include/binder/RpcSession.h
index 0b46606..761c50d 100644
--- a/libs/binder/include/binder/RpcSession.h
+++ b/libs/binder/include/binder/RpcSession.h
@@ -39,6 +39,7 @@
class RpcSocketAddress;
class RpcState;
class RpcTransport;
+class FdTrigger;
constexpr uint32_t RPC_WIRE_PROTOCOL_VERSION_NEXT = 0;
constexpr uint32_t RPC_WIRE_PROTOCOL_VERSION_EXPERIMENTAL = 0xF0000000;
@@ -78,17 +79,17 @@
* This should be called once per thread, matching 'join' in the remote
* process.
*/
- [[nodiscard]] bool setupUnixDomainClient(const char* path);
+ [[nodiscard]] status_t setupUnixDomainClient(const char* path);
/**
* Connects to an RPC server at the CVD & port.
*/
- [[nodiscard]] bool setupVsockClient(unsigned int cvd, unsigned int port);
+ [[nodiscard]] status_t setupVsockClient(unsigned int cvd, unsigned int port);
/**
* Connects to an RPC server at the given address and port.
*/
- [[nodiscard]] bool setupInetClient(const char* addr, unsigned int port);
+ [[nodiscard]] status_t setupInetClient(const char* addr, unsigned int port);
/**
* Starts talking to an RPC server which has already been connected to. This
@@ -100,8 +101,8 @@
*
* For future compatibility, 'request' should not reference any stack data.
*/
- [[nodiscard]] bool setupPreconnectedClient(base::unique_fd fd,
- std::function<base::unique_fd()>&& request);
+ [[nodiscard]] status_t setupPreconnectedClient(base::unique_fd fd,
+ std::function<base::unique_fd()>&& request);
/**
* For debugging!
@@ -110,7 +111,7 @@
* response will never be satisfied. All data sent here will be
* unceremoniously cast down the bottomless pit, /dev/null.
*/
- [[nodiscard]] bool addNullDebuggingClient();
+ [[nodiscard]] status_t addNullDebuggingClient();
/**
* Query the other side of the session for the root object hosted by that
@@ -161,50 +162,6 @@
friend RpcState;
explicit RpcSession(std::unique_ptr<RpcTransportCtxFactory> rpcTransportCtxFactory);
- /** This is not a pipe. */
- struct FdTrigger {
- /** Returns nullptr for error case */
- static std::unique_ptr<FdTrigger> make();
-
- /**
- * Close the write end of the pipe so that the read end receives POLLHUP.
- * Not threadsafe.
- */
- void trigger();
-
- /**
- * Whether this has been triggered.
- */
- bool isTriggered();
-
- /**
- * Poll for a read event.
- *
- * event - for pollfd
- *
- * Return:
- * true - time to read!
- * false - trigger happened
- */
- status_t triggerablePoll(base::borrowed_fd fd, int16_t event);
-
- /**
- * Read (or write), but allow to be interrupted by this trigger.
- *
- * Return:
- * true - succeeded in completely processing 'size'
- * false - interrupted (failure or trigger)
- */
- status_t interruptableReadFully(RpcTransport* rpcTransport, void* data, size_t size);
- status_t interruptableWriteFully(RpcTransport* rpcTransport, const void* data, size_t size);
-
- private:
- status_t triggerablePoll(RpcTransport* rpcTransport, int16_t event);
-
- base::unique_fd mWrite;
- base::unique_fd mRead;
- };
-
class EventListener : public virtual RefBase {
public:
virtual void onSessionAllIncomingThreadsEnded(const sp<RpcSession>& session) = 0;
@@ -253,15 +210,17 @@
// join on thread passed to preJoinThreadOwnership
static void join(sp<RpcSession>&& session, PreJoinSetupResult&& result);
- [[nodiscard]] bool setupClient(
- const std::function<bool(const RpcAddress& sessionId, bool incoming)>& connectAndInit);
- [[nodiscard]] bool setupSocketClient(const RpcSocketAddress& address);
- [[nodiscard]] bool setupOneSocketConnection(const RpcSocketAddress& address,
- const RpcAddress& sessionId, bool incoming);
- [[nodiscard]] bool initAndAddConnection(base::unique_fd fd, const RpcAddress& sessionId,
- bool incoming);
- [[nodiscard]] bool addIncomingConnection(std::unique_ptr<RpcTransport> rpcTransport);
- [[nodiscard]] bool addOutgoingConnection(std::unique_ptr<RpcTransport> rpcTransport, bool init);
+ [[nodiscard]] status_t setupClient(
+ const std::function<status_t(const RpcAddress& sessionId, bool incoming)>&
+ connectAndInit);
+ [[nodiscard]] status_t setupSocketClient(const RpcSocketAddress& address);
+ [[nodiscard]] status_t setupOneSocketConnection(const RpcSocketAddress& address,
+ const RpcAddress& sessionId, bool incoming);
+ [[nodiscard]] status_t initAndAddConnection(base::unique_fd fd, const RpcAddress& sessionId,
+ bool incoming);
+ [[nodiscard]] status_t addIncomingConnection(std::unique_ptr<RpcTransport> rpcTransport);
+ [[nodiscard]] status_t addOutgoingConnection(std::unique_ptr<RpcTransport> rpcTransport,
+ bool init);
[[nodiscard]] bool setForServer(const wp<RpcServer>& server,
const wp<RpcSession::EventListener>& eventListener,
const RpcAddress& sessionId);
@@ -269,6 +228,8 @@
std::unique_ptr<RpcTransport> rpcTransport);
[[nodiscard]] bool removeIncomingConnection(const sp<RpcConnection>& connection);
+ status_t initShutdownTrigger();
+
enum class ConnectionUse {
CLIENT,
CLIENT_ASYNC,
diff --git a/libs/binder/include/binder/RpcTransport.h b/libs/binder/include/binder/RpcTransport.h
index 1164600..1b69519 100644
--- a/libs/binder/include/binder/RpcTransport.h
+++ b/libs/binder/include/binder/RpcTransport.h
@@ -23,42 +23,30 @@
#include <android-base/result.h>
#include <android-base/unique_fd.h>
+#include <utils/Errors.h>
namespace android {
+class FdTrigger;
+
// Represents a socket connection.
class RpcTransport {
public:
virtual ~RpcTransport() = default;
- // replacement of ::send(). errno may not be set if TLS is enabled.
- virtual android::base::Result<size_t> send(const void *buf, size_t size) = 0;
-
- // replacement of ::recv(). errno may not be set if TLS is enabled.
- virtual android::base::Result<size_t> recv(void *buf, size_t size) = 0;
-
- // replacement of ::recv(MSG_PEEK). errno may not be set if TLS is enabled.
- //
- // Implementation details:
- // - For TLS, this may invoke syscalls and read data from the transport
- // into an internal buffer in userspace. After that, pending() == true.
- // - For raw sockets, this calls ::recv(MSG_PEEK), which leaves the data in the kernel buffer;
- // pending() is always false.
+ // replacement of ::recv(MSG_PEEK). Error code may not be set if TLS is enabled.
virtual android::base::Result<size_t> peek(void *buf, size_t size) = 0;
- // Returns true if there are data pending in a userspace buffer that RpcTransport holds.
- //
- // Implementation details:
- // - For TLS, this does not invoke any syscalls or read any data from the
- // transport. This only returns whether there are data pending in the internal buffer in
- // userspace.
- // - For raw sockets, this always returns false.
- virtual bool pending() = 0;
-
- // Returns fd for polling.
- //
- // Do not directly read / write on this raw fd!
- [[nodiscard]] virtual android::base::borrowed_fd pollSocket() const = 0;
+ /**
+ * Read (or write), but allow to be interrupted by a trigger.
+ *
+ * Return:
+ * OK - succeeded in completely processing 'size'
+ * error - interrupted (failure or trigger)
+ */
+ virtual status_t interruptableWriteFully(FdTrigger *fdTrigger, const void *buf,
+ size_t size) = 0;
+ virtual status_t interruptableReadFully(FdTrigger *fdTrigger, void *buf, size_t size) = 0;
protected:
RpcTransport() = default;
@@ -68,8 +56,13 @@
class RpcTransportCtx {
public:
virtual ~RpcTransportCtx() = default;
+
+ // Create a new RpcTransport object.
+ //
+ // Implemenion details: for TLS, this function may incur I/O. |fdTrigger| may be used
+ // to interrupt I/O. This function blocks until handshake is finished.
[[nodiscard]] virtual std::unique_ptr<RpcTransport> newTransport(
- android::base::unique_fd fd) const = 0;
+ android::base::unique_fd fd, FdTrigger *fdTrigger) const = 0;
protected:
RpcTransportCtx() = default;
diff --git a/libs/binder/libbinder_rpc_unstable.cpp b/libs/binder/libbinder_rpc_unstable.cpp
index 68ec669..bcb13ae 100644
--- a/libs/binder/libbinder_rpc_unstable.cpp
+++ b/libs/binder/libbinder_rpc_unstable.cpp
@@ -19,16 +19,20 @@
#include <binder/RpcServer.h>
#include <binder/RpcSession.h>
+using android::OK;
using android::RpcServer;
using android::RpcSession;
+using android::status_t;
+using android::statusToString;
extern "C" {
bool RunRpcServer(AIBinder* service, unsigned int port) {
auto server = RpcServer::make();
server->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction();
- if (!server->setupVsockServer(port)) {
- LOG(ERROR) << "Failed to set up vsock server with port " << port;
+ if (status_t status = server->setupVsockServer(port); status != OK) {
+ LOG(ERROR) << "Failed to set up vsock server with port " << port
+ << " error: " << statusToString(status).c_str();
return false;
}
server->setRootObject(AIBinder_toPlatformBinder(service));
@@ -41,8 +45,9 @@
AIBinder* RpcClient(unsigned int cid, unsigned int port) {
auto session = RpcSession::make();
- if (!session->setupVsockClient(cid, port)) {
- LOG(ERROR) << "Failed to set up vsock client with CID " << cid << " and port " << port;
+ if (status_t status = session->setupVsockClient(cid, port); status != OK) {
+ LOG(ERROR) << "Failed to set up vsock client with CID " << cid << " and port " << port
+ << " error: " << statusToString(status).c_str();
return nullptr;
}
return AIBinder_fromPlatformBinder(session->getRootObject());
diff --git a/libs/binder/ndk/include_cpp/android/binder_interface_utils.h b/libs/binder/ndk/include_cpp/android/binder_interface_utils.h
index 70a3906..5de64f8 100644
--- a/libs/binder/ndk/include_cpp/android/binder_interface_utils.h
+++ b/libs/binder/ndk/include_cpp/android/binder_interface_utils.h
@@ -43,23 +43,37 @@
namespace ndk {
/**
+ * analog using std::shared_ptr for internally held refcount
+ *
* ref must be called at least one time during the lifetime of this object. The recommended way to
* construct this object is with SharedRefBase::make.
- *
- * Note - this class used to not inherit from enable_shared_from_this, so
- * std::make_shared works, but it won't be portable against old copies of this
- * class.
*/
-class SharedRefBase : public std::enable_shared_from_this<SharedRefBase> {
+class SharedRefBase {
public:
SharedRefBase() {}
- virtual ~SharedRefBase() {}
+ virtual ~SharedRefBase() {
+ std::call_once(mFlagThis, [&]() {
+ __assert(__FILE__, __LINE__, "SharedRefBase: no ref created during lifetime");
+ });
+
+ if (ref() != nullptr) {
+ __assert(__FILE__, __LINE__,
+ "SharedRefBase: destructed but still able to lock weak_ptr. Is this object "
+ "double-owned?");
+ }
+ }
/**
* A shared_ptr must be held to this object when this is called. This must be called once during
* the lifetime of this object.
*/
- std::shared_ptr<SharedRefBase> ref() { return shared_from_this(); }
+ std::shared_ptr<SharedRefBase> ref() {
+ std::shared_ptr<SharedRefBase> thiz = mThis.lock();
+
+ std::call_once(mFlagThis, [&]() { mThis = thiz = std::shared_ptr<SharedRefBase>(this); });
+
+ return thiz;
+ }
/**
* Convenience method for a ref (see above) which automatically casts to the desired child type.
@@ -78,13 +92,8 @@
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
T* t = new T(std::forward<Args>(args)...);
#pragma clang diagnostic pop
-
- // T may have a private (though necessarily virtual!) destructor, so we
- // can't use refbase. For getting the first ref, we don't use ref()
- // since the internal shared_ptr isn't guaranteed to exist yet.
- SharedRefBase* base = static_cast<SharedRefBase*>(t);
- std::shared_ptr<SharedRefBase> strongBase(base);
- return std::static_pointer_cast<T>(strongBase);
+ // warning: Potential leak of memory pointed to by 't' [clang-analyzer-unix.Malloc]
+ return t->template ref<T>(); // NOLINT(clang-analyzer-unix.Malloc)
}
static void operator delete(void* p) { std::free(p); }
@@ -97,9 +106,13 @@
#if !defined(__ANDROID_API__) || __ANDROID_API__ >= 30 || defined(__ANDROID_APEX__)
private:
#else
- [[deprecated("Prefer SharedRefBase::make<T>(...) or std::make_shared<T>() if possible.")]]
+ [[deprecated("Prefer SharedRefBase::make<T>(...) if possible.")]]
#endif
static void* operator new(size_t s) { return std::malloc(s); }
+
+ private:
+ std::once_flag mFlagThis;
+ std::weak_ptr<SharedRefBase> mThis;
};
/**
diff --git a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp
index 204c6b6..4b36530 100644
--- a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp
+++ b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp
@@ -54,15 +54,6 @@
constexpr uint64_t kContextTestValue = 0xb4e42fb4d9a1d715;
class MyBinderNdkUnitTest : public aidl::BnBinderNdkUnitTest {
- public:
- MyBinderNdkUnitTest() = default;
- MyBinderNdkUnitTest(bool* deleted) : deleted(deleted) {}
- ~MyBinderNdkUnitTest() {
- if (deleted) {
- *deleted = true;
- }
- }
-
ndk::ScopedAStatus repeatInt(int32_t in, int32_t* out) {
*out = in;
return ndk::ScopedAStatus::ok();
@@ -131,7 +122,6 @@
}
uint64_t contextTestValue = kContextTestValue;
- bool* deleted = nullptr;
};
int generatedService() {
@@ -234,25 +224,15 @@
return true;
}
-TEST(NdkBinder, MakeShared) {
- const char* kInstance = "make_shared_test_instance";
- bool deleted = false;
+TEST(NdkBinder, DetectDoubleOwn) {
+ auto badService = ndk::SharedRefBase::make<MyBinderNdkUnitTest>();
+ EXPECT_DEATH(std::shared_ptr<MyBinderNdkUnitTest>(badService.get()),
+ "Is this object double-owned?");
+}
- {
- auto service = std::make_shared<MyBinderNdkUnitTest>(&deleted);
- auto binder = service->asBinder();
- ASSERT_EQ(EX_NONE, AServiceManager_addService(binder.get(), kInstance));
- auto binder2 = ndk::SpAIBinder(AServiceManager_checkService(kInstance));
- ASSERT_EQ(binder.get(), binder2.get());
-
- // overwrite service
- ASSERT_EQ(EX_NONE,
- AServiceManager_addService(
- std::make_shared<MyBinderNdkUnitTest>(&deleted)->asBinder().get(),
- kInstance));
- }
-
- EXPECT_TRUE(deleted);
+TEST(NdkBinder, DetectNoSharedRefBaseCreated) {
+ EXPECT_DEATH(std::make_shared<MyBinderNdkUnitTest>(),
+ "SharedRefBase: no ref created during lifetime");
}
TEST(NdkBinder, GetServiceThatDoesntExist) {
diff --git a/libs/binder/run_rpc_tests.sh b/libs/binder/run_rpc_tests.sh
new file mode 100755
index 0000000..7ba682d
--- /dev/null
+++ b/libs/binder/run_rpc_tests.sh
@@ -0,0 +1,44 @@
+#!/usr/bin/env bash
+
+set -ex
+
+if [ $# -eq 0 ]; then
+ :
+elif [ $# -eq 1 ] && [[ "$1" =~ ^host|device$ ]]; then
+ :
+else
+ echo "usage: $0 [device|host]"
+ false
+fi
+
+# Script maintained for RPC development, while it is active, to quickly run
+# tests. Generally, to match VTS/presubmit behavior, 'atest' should be used.
+
+function dtest () { adb shell /data/nativetest64/$1/$@; }
+function hbench () { $AT/out/host/linux-x86/benchmarktest/$1/$@; }
+function hfuzz () { time $ANDROID_HOST_OUT/fuzz/x86_64/$1/$@; }
+function htest () { time $ANDROID_BUILD_TOP/out/host/linux-x86/nativetest/$1/$@; }
+function pdtest () { adb wait-for-device && adb shell mkdir -p /data/nativetest64/$1 && adb push $OUT/data/nativetest64/$1/$1 /data/nativetest64/$1/$1 && dtest $@; }
+function dbench () { adb shell /data/benchmarktest64/$1/$@; }
+function pdbench () { adb wait-for-device && adb shell mkdir -p /data/benchmarktest64/$1 && adb push $OUT/data/benchmarktest64/$1/$1 /data/benchmarktest64/$1/$1 && dbench $@; }
+
+$ANDROID_BUILD_TOP/build/soong/soong_ui.bash --make-mode \
+ binderRpcTest \
+ binder_rpc_fuzzer \
+ binder_parcel_fuzzer \
+ binderLibTest \
+ binderRpcBenchmark
+
+if ! [ "$1" = "device" ]; then
+ htest binderRpcTest
+ hbench binderRpcBenchmark
+ hfuzz binder_rpc_fuzzer -max_total_time=30
+ hfuzz binder_parcel_fuzzer -max_total_time=30
+fi
+
+if ! [ "$1" = "host" ]; then
+ pdtest binderRpcTest
+ pdtest binderLibTest
+ pdbench binderRpcBenchmark
+fi
+
diff --git a/libs/binder/rust/src/parcel/file_descriptor.rs b/libs/binder/rust/src/parcel/file_descriptor.rs
index 20e9178..179b7c8 100644
--- a/libs/binder/rust/src/parcel/file_descriptor.rs
+++ b/libs/binder/rust/src/parcel/file_descriptor.rs
@@ -23,7 +23,7 @@
use crate::sys;
use std::fs::File;
-use std::os::unix::io::{AsRawFd, FromRawFd};
+use std::os::unix::io::{AsRawFd, FromRawFd, RawFd};
/// Rust version of the Java class android.os.ParcelFileDescriptor
#[derive(Debug)]
@@ -48,6 +48,12 @@
}
}
+impl AsRawFd for ParcelFileDescriptor {
+ fn as_raw_fd(&self) -> RawFd {
+ self.0.as_raw_fd()
+ }
+}
+
impl Serialize for ParcelFileDescriptor {
fn serialize(&self, parcel: &mut Parcel) -> Result<()> {
let fd = self.0.as_raw_fd();
diff --git a/libs/binder/servicedispatcher.cpp b/libs/binder/servicedispatcher.cpp
index a6e3f7d..9811cdf 100644
--- a/libs/binder/servicedispatcher.cpp
+++ b/libs/binder/servicedispatcher.cpp
@@ -23,9 +23,12 @@
#include <android-base/logging.h>
#include <android-base/properties.h>
#include <android-base/stringprintf.h>
+#include <android/debug/BnAdbCallback.h>
+#include <android/debug/IAdbManager.h>
#include <android/os/BnServiceManager.h>
#include <android/os/IServiceManager.h>
#include <binder/IServiceManager.h>
+#include <binder/ProcessState.h>
#include <binder/RpcServer.h>
using android::BBinder;
@@ -33,6 +36,7 @@
using android::OK;
using android::RpcServer;
using android::sp;
+using android::status_t;
using android::statusToString;
using android::String16;
using android::base::Basename;
@@ -49,6 +53,7 @@
const char* kLocalInetAddress = "127.0.0.1";
using ServiceRetriever = decltype(&android::IServiceManager::checkService);
+using android::debug::IAdbManager;
int Usage(const char* program) {
auto basename = Basename(program);
@@ -87,8 +92,8 @@
}
rpcServer->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction();
unsigned int port;
- if (!rpcServer->setupInetServer(kLocalInetAddress, 0, &port)) {
- LOG(ERROR) << "setupInetServer failed";
+ if (status_t status = rpcServer->setupInetServer(kLocalInetAddress, 0, &port); status != OK) {
+ LOG(ERROR) << "setupInetServer failed: " << statusToString(status);
return EX_SOFTWARE;
}
auto socket = rpcServer->releaseServer();
@@ -200,8 +205,8 @@
rpcServer->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction();
rpcServer->setRootObject(service);
unsigned int port;
- if (!rpcServer->setupInetServer(kLocalInetAddress, 0, &port)) {
- LOG(ERROR) << "Unable to set up inet server";
+ if (status_t status = rpcServer->setupInetServer(kLocalInetAddress, 0, &port); status != OK) {
+ LOG(ERROR) << "Unable to set up inet server: " << statusToString(status);
return EX_SOFTWARE;
}
LOG(INFO) << "Finish wrapping servicemanager with RPC on port " << port;
@@ -212,6 +217,25 @@
__builtin_unreachable();
}
+class AdbCallback : public android::debug::BnAdbCallback {
+public:
+ android::binder::Status onDebuggingChanged(bool enabled,
+ android::debug::AdbTransportType) override {
+ if (!enabled) {
+ LOG(ERROR) << "ADB debugging disabled, exiting.";
+ exit(EX_SOFTWARE);
+ }
+ return android::binder::Status::ok();
+ }
+};
+
+void exitOnAdbDebuggingDisabled() {
+ auto adb = android::waitForService<IAdbManager>(String16("adb"));
+ CHECK(adb != nullptr) << "Unable to retrieve service adb";
+ auto status = adb->registerCallback(sp<AdbCallback>::make());
+ CHECK(status.isOk()) << "Unable to call IAdbManager::registerCallback: " << status;
+}
+
// Log to logd. For warning and more severe messages, also log to stderr.
class ServiceDispatcherLogger {
public:
@@ -252,6 +276,10 @@
}
}
+ android::ProcessState::self()->setThreadPoolMaxThreadCount(1);
+ android::ProcessState::self()->startThreadPool();
+ exitOnAdbDebuggingDisabled();
+
if (optind + 1 != argc) return Usage(argv[0]);
auto name = argv[optind];
diff --git a/libs/binder/tests/IBinderRpcBenchmark.aidl b/libs/binder/tests/IBinderRpcBenchmark.aidl
index 1457422..2baf680 100644
--- a/libs/binder/tests/IBinderRpcBenchmark.aidl
+++ b/libs/binder/tests/IBinderRpcBenchmark.aidl
@@ -17,4 +17,5 @@
interface IBinderRpcBenchmark {
@utf8InCpp String repeatString(@utf8InCpp String str);
IBinder repeatBinder(IBinder binder);
+ byte[] repeatBytes(in byte[] bytes);
}
diff --git a/libs/binder/tests/binderHostDeviceTest.cpp b/libs/binder/tests/binderHostDeviceTest.cpp
index 5dd9212..3f72b8f 100644
--- a/libs/binder/tests/binderHostDeviceTest.cpp
+++ b/libs/binder/tests/binderHostDeviceTest.cpp
@@ -101,8 +101,9 @@
[[nodiscard]] static sp<IBinder> get(unsigned int hostPort) {
auto rpcSession = RpcSession::make();
- if (!rpcSession->setupInetClient("127.0.0.1", hostPort)) {
- ADD_FAILURE() << "Failed to setupInetClient on " << hostPort;
+ if (status_t status = rpcSession->setupInetClient("127.0.0.1", hostPort); status != OK) {
+ ADD_FAILURE() << "Failed to setupInetClient on " << hostPort << ": "
+ << statusToString(status);
return nullptr;
}
return rpcSession->getRootObject();
diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp
index 65db7f6..eea7d8c 100644
--- a/libs/binder/tests/binderLibTest.cpp
+++ b/libs/binder/tests/binderLibTest.cpp
@@ -1192,8 +1192,8 @@
if (rpcServer == nullptr) return {};
rpcServer->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction();
unsigned int port;
- if (!rpcServer->setupInetServer("127.0.0.1", 0, &port)) {
- ADD_FAILURE() << "setupInetServer failed";
+ if (status_t status = rpcServer->setupInetServer("127.0.0.1", 0, &port); status != OK) {
+ ADD_FAILURE() << "setupInetServer failed" << statusToString(status);
return {};
}
return {rpcServer->releaseServer(), port};
diff --git a/libs/binder/tests/binderRpcBenchmark.cpp b/libs/binder/tests/binderRpcBenchmark.cpp
index 5f4a7b5..e430c28 100644
--- a/libs/binder/tests/binderRpcBenchmark.cpp
+++ b/libs/binder/tests/binderRpcBenchmark.cpp
@@ -42,6 +42,8 @@
using android::RpcServer;
using android::RpcSession;
using android::sp;
+using android::status_t;
+using android::statusToString;
using android::String16;
using android::binder::Status;
@@ -50,8 +52,12 @@
*out = str;
return Status::ok();
}
- Status repeatBinder(const sp<IBinder>& str, sp<IBinder>* out) override {
- *out = str;
+ Status repeatBinder(const sp<IBinder>& binder, sp<IBinder>* out) override {
+ *out = binder;
+ return Status::ok();
+ }
+ Status repeatBytes(const std::vector<uint8_t>& bytes, std::vector<uint8_t>* out) override {
+ *out = bytes;
return Status::ok();
}
};
@@ -61,12 +67,11 @@
RPC,
};
-static void EachTransport(benchmark::internal::Benchmark* b) {
+static const std::initializer_list<int64_t> kTransportList = {
#ifdef __BIONIC__
- b->Args({Transport::KERNEL});
+ Transport::KERNEL,
#endif
- b->Args({Transport::RPC});
-}
+ Transport::RPC};
static sp<RpcSession> gSession = RpcSession::make();
#ifdef __BIONIC__
@@ -96,9 +101,9 @@
CHECK_EQ(OK, binder->pingBinder());
}
}
-BENCHMARK(BM_pingTransaction)->Apply(EachTransport);
+BENCHMARK(BM_pingTransaction)->ArgsProduct({kTransportList});
-void BM_repeatString(benchmark::State& state) {
+void BM_repeatTwoPageString(benchmark::State& state) {
sp<IBinder> binder = getBinderForOptions(state);
sp<IBinderRpcBenchmark> iface = interface_cast<IBinderRpcBenchmark>(binder);
@@ -125,10 +130,30 @@
CHECK(ret.isOk()) << ret;
}
}
-BENCHMARK(BM_repeatString)->Apply(EachTransport);
+BENCHMARK(BM_repeatTwoPageString)->ArgsProduct({kTransportList});
+
+void BM_throughputForTransportAndBytes(benchmark::State& state) {
+ sp<IBinder> binder = getBinderForOptions(state);
+ sp<IBinderRpcBenchmark> iface = interface_cast<IBinderRpcBenchmark>(binder);
+ CHECK(iface != nullptr);
+
+ std::vector<uint8_t> bytes = std::vector<uint8_t>(state.range(1));
+ for (size_t i = 0; i < bytes.size(); i++) {
+ bytes[i] = i % 256;
+ }
+
+ while (state.KeepRunning()) {
+ std::vector<uint8_t> out;
+ Status ret = iface->repeatBytes(bytes, &out);
+ CHECK(ret.isOk()) << ret;
+ }
+}
+BENCHMARK(BM_throughputForTransportAndBytes)
+ ->ArgsProduct({kTransportList,
+ {64, 1024, 2048, 4096, 8182, 16364, 32728, 65535, 65536, 65537}});
void BM_repeatBinder(benchmark::State& state) {
- sp<IBinder> binder = gSession->getRootObject();
+ sp<IBinder> binder = getBinderForOptions(state);
CHECK(binder != nullptr);
sp<IBinderRpcBenchmark> iface = interface_cast<IBinderRpcBenchmark>(binder);
CHECK(iface != nullptr);
@@ -142,7 +167,7 @@
CHECK(ret.isOk()) << ret;
}
}
-BENCHMARK(BM_repeatBinder)->Apply(EachTransport);
+BENCHMARK(BM_repeatBinder)->ArgsProduct({kTransportList});
int main(int argc, char** argv) {
::benchmark::Initialize(&argc, argv);
@@ -152,15 +177,15 @@
(void)unlink(addr.c_str());
std::cerr << "Tests suffixes:" << std::endl;
- std::cerr << "\t\\" << Transport::KERNEL << " is KERNEL" << std::endl;
- std::cerr << "\t\\" << Transport::RPC << " is RPC" << std::endl;
+ std::cerr << "\t.../" << Transport::KERNEL << " is KERNEL" << std::endl;
+ std::cerr << "\t.../" << Transport::RPC << " is RPC" << std::endl;
if (0 == fork()) {
prctl(PR_SET_PDEATHSIG, SIGHUP); // racey, okay
sp<RpcServer> server = RpcServer::make();
server->setRootObject(sp<MyBinderRpcBenchmark>::make());
server->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction();
- CHECK(server->setupUnixDomainServer(addr.c_str()));
+ CHECK_EQ(OK, server->setupUnixDomainServer(addr.c_str()));
server->join();
exit(1);
}
@@ -182,11 +207,13 @@
CHECK_NE(nullptr, gKernelBinder.get());
#endif
+ status_t status;
for (size_t tries = 0; tries < 5; tries++) {
usleep(10000);
- if (gSession->setupUnixDomainClient(addr.c_str())) goto success;
+ status = gSession->setupUnixDomainClient(addr.c_str());
+ if (status == OK) goto success;
}
- LOG(FATAL) << "Could not connect.";
+ LOG(FATAL) << "Could not connect: " << statusToString(status).c_str();
success:
::benchmark::RunSpecifiedBenchmarks();
diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp
index 36a6804..15ccae9 100644
--- a/libs/binder/tests/binderRpcTest.cpp
+++ b/libs/binder/tests/binderRpcTest.cpp
@@ -89,7 +89,7 @@
auto server = RpcServer::make(newFactory(GetParam()));
server->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction();
ASSERT_FALSE(server->hasServer());
- ASSERT_TRUE(server->setupExternalServer(std::move(sink)));
+ ASSERT_EQ(OK, server->setupExternalServer(std::move(sink)));
ASSERT_TRUE(server->hasServer());
base::unique_fd retrieved = server->releaseServer();
ASSERT_FALSE(server->hasServer());
@@ -484,13 +484,13 @@
case SocketType::PRECONNECTED:
[[fallthrough]];
case SocketType::UNIX:
- CHECK(server->setupUnixDomainServer(addr.c_str())) << addr;
+ CHECK_EQ(OK, server->setupUnixDomainServer(addr.c_str())) << addr;
break;
case SocketType::VSOCK:
- CHECK(server->setupVsockServer(vsockPort));
+ CHECK_EQ(OK, server->setupVsockServer(vsockPort));
break;
case SocketType::INET: {
- CHECK(server->setupInetServer(kLocalInetAddress, 0, &outPort));
+ CHECK_EQ(OK, server->setupInetServer(kLocalInetAddress, 0, &outPort));
CHECK_NE(0, outPort);
break;
}
@@ -516,30 +516,35 @@
CHECK_NE(0, outPort);
}
+ status_t status;
+
for (size_t i = 0; i < options.numSessions; i++) {
sp<RpcSession> session = RpcSession::make(newFactory(rpcSecurity));
session->setMaxThreads(options.numIncomingConnections);
switch (socketType) {
case SocketType::PRECONNECTED:
- if (session->setupPreconnectedClient({}, [=]() {
- return connectToUds(addr.c_str());
- }))
- goto success;
+ status = session->setupPreconnectedClient({}, [=]() {
+ return connectToUds(addr.c_str());
+ });
+ if (status == OK) goto success;
break;
case SocketType::UNIX:
- if (session->setupUnixDomainClient(addr.c_str())) goto success;
+ status = session->setupUnixDomainClient(addr.c_str());
+ if (status == OK) goto success;
break;
case SocketType::VSOCK:
- if (session->setupVsockClient(VMADDR_CID_LOCAL, vsockPort)) goto success;
+ status = session->setupVsockClient(VMADDR_CID_LOCAL, vsockPort);
+ if (status == OK) goto success;
break;
case SocketType::INET:
- if (session->setupInetClient("127.0.0.1", outPort)) goto success;
+ status = session->setupInetClient("127.0.0.1", outPort);
+ if (status == OK) goto success;
break;
default:
LOG_ALWAYS_FATAL("Unknown socket type");
}
- LOG_ALWAYS_FATAL("Could not connect");
+ LOG_ALWAYS_FATAL("Could not connect %s", statusToString(status).c_str());
success:
ret.sessions.push_back({session, session->getRootObject()});
}
@@ -1191,14 +1196,19 @@
unsigned int vsockPort = allocateVsockPort();
sp<RpcServer> server = RpcServer::make(RpcTransportCtxFactoryRaw::make());
server->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction();
- CHECK(server->setupVsockServer(vsockPort));
+ if (status_t status = server->setupVsockServer(vsockPort); status != OK) {
+ if (status == -EAFNOSUPPORT) {
+ return false;
+ }
+ LOG_ALWAYS_FATAL("Could not setup vsock server: %s", statusToString(status).c_str());
+ }
server->start();
sp<RpcSession> session = RpcSession::make(RpcTransportCtxFactoryRaw::make());
- bool okay = session->setupVsockClient(VMADDR_CID_LOCAL, vsockPort);
+ status_t status = session->setupVsockClient(VMADDR_CID_LOCAL, vsockPort);
while (!server->shutdown()) usleep(10000);
- ALOGE("Detected vsock loopback supported: %d", okay);
- return okay;
+ ALOGE("Detected vsock loopback supported: %s", statusToString(status).c_str());
+ return status == OK;
}
static std::vector<SocketType> testSocketTypes() {
@@ -1274,7 +1284,7 @@
unlink(addr.c_str());
auto server = RpcServer::make(newFactory(GetParam()));
server->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction();
- ASSERT_TRUE(server->setupUnixDomainServer(addr.c_str()));
+ ASSERT_EQ(OK, server->setupUnixDomainServer(addr.c_str()));
auto joinEnds = std::make_shared<OneOffSignal>();
// If things are broken and the thread never stops, don't block other tests. Because the thread
@@ -1315,14 +1325,14 @@
auto rpcServer = RpcServer::make();
rpcServer->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction();
unsigned int port;
- ASSERT_TRUE(rpcServer->setupInetServer(kLocalInetAddress, 0, &port));
+ ASSERT_EQ(OK, rpcServer->setupInetServer(kLocalInetAddress, 0, &port));
auto socket = rpcServer->releaseServer();
auto keepAlive = sp<BBinder>::make();
ASSERT_EQ(OK, binder->setRpcClientDebug(std::move(socket), keepAlive));
auto rpcSession = RpcSession::make();
- ASSERT_TRUE(rpcSession->setupInetClient("127.0.0.1", port));
+ ASSERT_EQ(OK, rpcSession->setupInetClient("127.0.0.1", port));
auto rpcBinder = rpcSession->getRootObject();
ASSERT_NE(nullptr, rpcBinder);
diff --git a/libs/binder/tests/parcel_fuzzer/random_parcel.cpp b/libs/binder/tests/parcel_fuzzer/random_parcel.cpp
index a2472f8..8bf04cc 100644
--- a/libs/binder/tests/parcel_fuzzer/random_parcel.cpp
+++ b/libs/binder/tests/parcel_fuzzer/random_parcel.cpp
@@ -37,7 +37,7 @@
void fillRandomParcel(Parcel* p, FuzzedDataProvider&& provider) {
if (provider.ConsumeBool()) {
auto session = RpcSession::make(RpcTransportCtxFactoryRaw::make());
- CHECK(session->addNullDebuggingClient());
+ CHECK_EQ(OK, session->addNullDebuggingClient());
p->markForRpc(session);
fillRandomParcelData(p, std::move(provider));
return;
diff --git a/libs/binder/tests/rpc_fuzzer/main.cpp b/libs/binder/tests/rpc_fuzzer/main.cpp
index 9fc496f..230f5c7 100644
--- a/libs/binder/tests/rpc_fuzzer/main.cpp
+++ b/libs/binder/tests/rpc_fuzzer/main.cpp
@@ -60,7 +60,7 @@
sp<RpcServer> server = RpcServer::make();
server->setRootObject(sp<SomeBinder>::make());
server->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction();
- CHECK(server->setupUnixDomainServer(kSock.c_str()));
+ CHECK_EQ(OK, server->setupUnixDomainServer(kSock.c_str()));
std::thread serverThread([=] { (void)server->join(); });
diff --git a/libs/binderdebug/BinderDebug.cpp b/libs/binderdebug/BinderDebug.cpp
index b435dba..d086b49 100644
--- a/libs/binderdebug/BinderDebug.cpp
+++ b/libs/binderdebug/BinderDebug.cpp
@@ -14,6 +14,7 @@
* limitations under the License.
*/
+#include <android-base/logging.h>
#include <android-base/parseint.h>
#include <android-base/strings.h>
#include <binder/Binder.h>
@@ -116,4 +117,60 @@
return ret;
}
+status_t getBinderClientPids(BinderDebugContext context, pid_t pid, pid_t servicePid,
+ int32_t handle, std::vector<pid_t>* pids) {
+ std::smatch match;
+ static const std::regex kNodeNumber("^\\s+ref \\d+:\\s+desc\\s+(\\d+)\\s+node\\s+(\\d+).*");
+ std::string contextStr = contextToString(context);
+ int32_t node;
+ status_t ret = scanBinderContext(pid, contextStr, [&](const std::string& line) {
+ if (std::regex_search(line, match, kNodeNumber)) {
+ const std::string& descString = match.str(1);
+ int32_t desc;
+ if (!::android::base::ParseInt(descString.c_str(), &desc)) {
+ LOG(ERROR) << "Failed to parse desc int: " << descString;
+ return;
+ }
+ if (handle != desc) {
+ return;
+ }
+ const std::string& nodeString = match.str(2);
+ if (!::android::base::ParseInt(nodeString.c_str(), &node)) {
+ LOG(ERROR) << "Failed to parse node int: " << nodeString;
+ return;
+ }
+ return;
+ }
+ return;
+ });
+ if (ret != OK) {
+ return ret;
+ }
+ static const std::regex kClients("^\\s+node\\s+(\\d+).*proc\\s+([\\d+\\s*]*)");
+ ret = scanBinderContext(servicePid, contextStr, [&](const std::string& line) {
+ if (std::regex_search(line, match, kClients)) {
+ const std::string nodeString = match.str(1);
+ int32_t matchedNode;
+ if (!::android::base::ParseInt(nodeString.c_str(), &matchedNode)) {
+ LOG(ERROR) << "Failed to parse node int: " << nodeString;
+ return;
+ }
+ if (node != matchedNode) {
+ return;
+ }
+ const std::string clients = match.str(2);
+ for (const std::string& pidStr : base::Split(clients, " ")) {
+ int32_t pid;
+ if (!::android::base::ParseInt(pidStr, &pid)) {
+ return;
+ }
+ pids->push_back(pid);
+ }
+ return;
+ }
+ return;
+ });
+ return ret;
+}
+
} // namespace android
diff --git a/libs/binderdebug/include/binderdebug/BinderDebug.h b/libs/binderdebug/include/binderdebug/BinderDebug.h
index 14a0ef3..dfd5a7c 100644
--- a/libs/binderdebug/include/binderdebug/BinderDebug.h
+++ b/libs/binderdebug/include/binderdebug/BinderDebug.h
@@ -32,6 +32,14 @@
VNDBINDER,
};
+/**
+ * pid is the pid of the service
+ */
status_t getBinderPidInfo(BinderDebugContext context, pid_t pid, BinderPidInfo* pidInfo);
+/**
+ * pid is typically the pid of this process that is making the query
+ */
+status_t getBinderClientPids(BinderDebugContext context, pid_t pid, pid_t servicePid,
+ int32_t handle, std::vector<pid_t>* pids);
} // namespace android
diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp
index 23895c0..87aa34d 100644
--- a/libs/gui/LayerState.cpp
+++ b/libs/gui/LayerState.cpp
@@ -174,6 +174,7 @@
SAFE_PARCEL(output.write, destinationFrame);
SAFE_PARCEL(output.writeBool, isTrustedOverlay);
+ SAFE_PARCEL(output.writeStrongBinder, releaseBufferEndpoint);
return NO_ERROR;
}
@@ -303,6 +304,7 @@
SAFE_PARCEL(input.read, destinationFrame);
SAFE_PARCEL(input.readBool, &isTrustedOverlay);
+ SAFE_PARCEL(input.readNullableStrongBinder, &releaseBufferEndpoint);
return NO_ERROR;
}
@@ -461,6 +463,7 @@
if (other.what & eBufferChanged) {
what |= eBufferChanged;
buffer = other.buffer;
+ releaseBufferEndpoint = other.releaseBufferEndpoint;
}
if (other.what & eAcquireFenceChanged) {
what |= eAcquireFenceChanged;
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index f620f5a..2e25ae4 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -147,8 +147,16 @@
return mCallbackIdCounter++;
}
+sp<TransactionCompletedListener> TransactionCompletedListener::sInstance = nullptr;
+
+void TransactionCompletedListener::setInstance(const sp<TransactionCompletedListener>& listener) {
+ sInstance = listener;
+}
+
sp<TransactionCompletedListener> TransactionCompletedListener::getInstance() {
- static sp<TransactionCompletedListener> sInstance = new TransactionCompletedListener;
+ if (sInstance == nullptr) {
+ sInstance = new TransactionCompletedListener;
+ }
return sInstance;
}
@@ -1274,6 +1282,7 @@
removeReleaseBufferCallback(s);
s->what |= layer_state_t::eBufferChanged;
s->buffer = buffer;
+ s->releaseBufferEndpoint = IInterface::asBinder(TransactionCompletedListener::getIInstance());
if (mIsAutoTimestamp) {
mDesiredPresentTime = systemTime();
}
diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h
index b27d9cf..8dc8b9a 100644
--- a/libs/gui/include/gui/LayerState.h
+++ b/libs/gui/include/gui/LayerState.h
@@ -242,6 +242,11 @@
// is used to remove the old callback from the client process map if it is
// overwritten by another setBuffer call.
ReleaseCallbackId releaseCallbackId;
+
+ // Stores which endpoint the release information should be sent to. We don't want to send the
+ // releaseCallbackId and release fence to all listeners so we store which listener the setBuffer
+ // was called with.
+ sp<IBinder> releaseBufferEndpoint;
};
struct ComposerState {
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index 869cef6..7592e0c 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -652,8 +652,10 @@
};
class TransactionCompletedListener : public BnTransactionCompletedListener {
+public:
TransactionCompletedListener();
+protected:
int64_t getNextIdLocked() REQUIRES(mMutex);
std::mutex mMutex;
@@ -731,8 +733,12 @@
void onReleaseBuffer(ReleaseCallbackId, sp<Fence> releaseFence, uint32_t transformHint,
uint32_t currentMaxAcquiredBufferCount) override;
+ // For Testing Only
+ static void setInstance(const sp<TransactionCompletedListener>&);
+
private:
ReleaseBufferCallback popReleaseBufferCallbackLocked(const ReleaseCallbackId&);
+ static sp<TransactionCompletedListener> sInstance;
};
} // namespace android
diff --git a/libs/nativedisplay/include/surfacetexture/SurfaceTexture.h b/libs/nativedisplay/include/surfacetexture/SurfaceTexture.h
index b8b5a7d..bac44c9 100644
--- a/libs/nativedisplay/include/surfacetexture/SurfaceTexture.h
+++ b/libs/nativedisplay/include/surfacetexture/SurfaceTexture.h
@@ -272,11 +272,11 @@
status_t attachToContext(uint32_t tex);
sp<GraphicBuffer> dequeueBuffer(int* outSlotid, android_dataspace* outDataspace,
+ float* outTransformMatrix, uint32_t* outTransform,
bool* outQueueEmpty,
SurfaceTexture_createReleaseFence createFence,
SurfaceTexture_fenceWait fenceWait,
- void* fencePassThroughHandle, ARect* cropRect,
- uint32_t* transform);
+ void* fencePassThroughHandle, ARect* currentCrop);
/**
* takeConsumerOwnership attaches a SurfaceTexture that is currently in the
diff --git a/libs/nativedisplay/include/surfacetexture/surface_texture_platform.h b/libs/nativedisplay/include/surfacetexture/surface_texture_platform.h
index 5ae526f..e85009c 100644
--- a/libs/nativedisplay/include/surfacetexture/surface_texture_platform.h
+++ b/libs/nativedisplay/include/surfacetexture/surface_texture_platform.h
@@ -83,11 +83,12 @@
* AHardwareBuffer_release.
*/
AHardwareBuffer* ASurfaceTexture_dequeueBuffer(ASurfaceTexture* st, int* outSlotid,
- android_dataspace* outDataspace, bool* outNewContent,
+ android_dataspace* outDataspace,
+ float* outTransformMatrix, uint32_t* outTransform,
+ bool* outNewContent,
ASurfaceTexture_createReleaseFence createFence,
ASurfaceTexture_fenceWait fenceWait,
- void* fencePassThroughHandle, ARect* cropRect,
- uint32_t* transform);
+ void* fencePassThroughHandle, ARect* currentCrop);
} // namespace android
diff --git a/libs/nativedisplay/surfacetexture/SurfaceTexture.cpp b/libs/nativedisplay/surfacetexture/SurfaceTexture.cpp
index 06c0f74..3535e67 100644
--- a/libs/nativedisplay/surfacetexture/SurfaceTexture.cpp
+++ b/libs/nativedisplay/surfacetexture/SurfaceTexture.cpp
@@ -464,11 +464,11 @@
}
sp<GraphicBuffer> SurfaceTexture::dequeueBuffer(int* outSlotid, android_dataspace* outDataspace,
+ float* outTransformMatrix, uint32_t* outTransform,
bool* outQueueEmpty,
SurfaceTexture_createReleaseFence createFence,
SurfaceTexture_fenceWait fenceWait,
- void* fencePassThroughHandle, ARect* cropRect,
- uint32_t* transform) {
+ void* fencePassThroughHandle, ARect* currentCrop) {
Mutex::Autolock _l(mMutex);
sp<GraphicBuffer> buffer;
@@ -484,8 +484,9 @@
buffer = mImageConsumer.dequeueBuffer(outSlotid, outDataspace, outQueueEmpty, *this,
createFence, fenceWait, fencePassThroughHandle);
- *transform = mCurrentTransform;
- *cropRect = mCurrentCrop;
+ memcpy(outTransformMatrix, mCurrentTransformMatrix, sizeof(mCurrentTransformMatrix));
+ *outTransform = mCurrentTransform;
+ *currentCrop = mCurrentCrop;
return buffer;
}
diff --git a/libs/nativedisplay/surfacetexture/surface_texture.cpp b/libs/nativedisplay/surfacetexture/surface_texture.cpp
index 43e7d03..cc0a12d 100644
--- a/libs/nativedisplay/surfacetexture/surface_texture.cpp
+++ b/libs/nativedisplay/surfacetexture/surface_texture.cpp
@@ -193,16 +193,19 @@
}
AHardwareBuffer* ASurfaceTexture_dequeueBuffer(ASurfaceTexture* st, int* outSlotid,
- android_dataspace* outDataspace, bool* outNewContent,
+ android_dataspace* outDataspace,
+ float* outTransformMatrix, uint32_t* outTransform,
+ bool* outNewContent,
ASurfaceTexture_createReleaseFence createFence,
ASurfaceTexture_fenceWait fenceWait, void* handle,
- ARect* cropRect, uint32_t* transform) {
+ ARect* currentCrop) {
sp<GraphicBuffer> buffer;
*outNewContent = false;
bool queueEmpty;
do {
- buffer = st->consumer->dequeueBuffer(outSlotid, outDataspace, &queueEmpty, createFence,
- fenceWait, handle, cropRect, transform);
+ buffer = st->consumer->dequeueBuffer(outSlotid, outDataspace, outTransformMatrix,
+ outTransform, &queueEmpty, createFence, fenceWait,
+ handle, currentCrop);
if (!queueEmpty) {
*outNewContent = true;
}
diff --git a/services/sensorservice/SensorDevice.cpp b/services/sensorservice/SensorDevice.cpp
index c233bf0..c351676 100644
--- a/services/sensorservice/SensorDevice.cpp
+++ b/services/sensorservice/SensorDevice.cpp
@@ -168,7 +168,7 @@
}
}
- // Sanity check and clamp power if it is 0 (or close)
+ // Check and clamp power if it is 0 (or close)
constexpr float MIN_POWER_MA = 0.001; // 1 microAmp
if (sensor.power < MIN_POWER_MA) {
ALOGI("%s's reported power %f invalid, clamped to %f",
diff --git a/services/sensorservice/SensorEventConnection.cpp b/services/sensorservice/SensorEventConnection.cpp
index 9ce8d9b..c58e992 100644
--- a/services/sensorservice/SensorEventConnection.cpp
+++ b/services/sensorservice/SensorEventConnection.cpp
@@ -867,7 +867,7 @@
} else if (numBytesRead == sizeof(uint32_t)) {
uint32_t numAcks = 0;
memcpy(&numAcks, buf, numBytesRead);
- // Sanity check to ensure there are no read errors in recv, numAcks is always
+ // Check to ensure there are no read errors in recv, numAcks is always
// within the range and not zero. If any of the above don't hold reset
// mWakeLockRefCount to zero.
if (numAcks > 0 && numAcks < mWakeLockRefCount) {
diff --git a/services/sensorservice/SensorService.cpp b/services/sensorservice/SensorService.cpp
index 726fe8e..3081b1a 100644
--- a/services/sensorservice/SensorService.cpp
+++ b/services/sensorservice/SensorService.cpp
@@ -1197,7 +1197,7 @@
// We have a dynamic sensor.
if (!sHmacGlobalKeyIsValid) {
- // Rather than risk exposing UUIDs, we cripple dynamic sensors.
+ // Rather than risk exposing UUIDs, we slow down dynamic sensors.
ALOGW("HMAC key failure; dynamic sensor getId() will be wrong.");
return 0;
}
@@ -1223,7 +1223,7 @@
sHmacGlobalKey, sizeof(sHmacGlobalKey),
uuidAndApp, sizeof(uuidAndApp),
hash, &hashLen) == nullptr) {
- // Rather than risk exposing UUIDs, we cripple dynamic sensors.
+ // Rather than risk exposing UUIDs, we slow down dynamic sensors.
ALOGW("HMAC failure; dynamic sensor getId() will be wrong.");
return 0;
}
diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp
index 9e159fc..4d86598 100644
--- a/services/surfaceflinger/BufferStateLayer.cpp
+++ b/services/surfaceflinger/BufferStateLayer.cpp
@@ -153,7 +153,8 @@
// transaction doesn't need a previous release fence.
sp<CallbackHandle> ch;
for (auto& handle : mDrawingState.callbackHandles) {
- if (handle->releasePreviousBuffer) {
+ if (handle->releasePreviousBuffer &&
+ mDrawingState.releaseBufferEndpoint == handle->listener) {
ch = handle;
break;
}
@@ -194,14 +195,9 @@
mFlinger->getMaxAcquiredBufferCountForCurrentRefreshRate(mOwnerUid);
}
- // If there are multiple transactions in this frame, set the previous id on the earliest
- // transacton. We don't need to pass in the released buffer id to multiple transactions.
- // The buffer id does not have to correspond to any particular transaction as long as the
- // listening end point is the same but the client expects the first transaction callback that
- // replaces the presented buffer to contain the release fence. This follows the same logic.
- // see BufferStateLayer::onLayerDisplayed.
for (auto& handle : mDrawingState.callbackHandles) {
- if (handle->releasePreviousBuffer) {
+ if (handle->releasePreviousBuffer &&
+ mDrawingState.releaseBufferEndpoint == handle->listener) {
handle->previousReleaseCallbackId = mPreviousReleaseCallbackId;
break;
}
@@ -415,12 +411,14 @@
nsecs_t desiredPresentTime, bool isAutoTimestamp,
const client_cache_t& clientCacheId, uint64_t frameNumber,
std::optional<nsecs_t> dequeueTime, const FrameTimelineInfo& info,
- const sp<ITransactionCompletedListener>& releaseBufferListener) {
+ const sp<ITransactionCompletedListener>& releaseBufferListener,
+ const sp<IBinder>& releaseBufferEndpoint) {
ATRACE_CALL();
if (mDrawingState.buffer) {
mReleasePreviousBuffer = true;
- if (mDrawingState.buffer != mBufferInfo.mBuffer) {
+ if (mDrawingState.buffer != mBufferInfo.mBuffer ||
+ mDrawingState.frameNumber != mBufferInfo.mFrameNumber) {
// If mDrawingState has a buffer, and we are about to update again
// before swapping to drawing state, then the first buffer will be
// dropped and we should decrement the pending buffer count and
@@ -479,6 +477,7 @@
mDrawingState.width = mDrawingState.buffer->getBuffer()->getWidth();
mDrawingState.height = mDrawingState.buffer->getBuffer()->getHeight();
+ mDrawingState.releaseBufferEndpoint = releaseBufferEndpoint;
return true;
}
@@ -958,22 +957,6 @@
ATRACE_INT(mBlastTransactionName.c_str(), pendingBuffers);
}
-void BufferStateLayer::bufferMayChange(const sp<GraphicBuffer>& newBuffer) {
- if (mDrawingState.buffer != nullptr &&
- (!mBufferInfo.mBuffer ||
- mDrawingState.buffer->getBuffer() != mBufferInfo.mBuffer->getBuffer()) &&
- newBuffer != mDrawingState.buffer->getBuffer()) {
- // If we are about to update mDrawingState.buffer but it has not yet latched
- // then we will drop a buffer and should decrement the pending buffer count and
- // call any release buffer callbacks if set.
- callReleaseBufferCallback(mDrawingState.releaseBufferListener,
- mDrawingState.buffer->getBuffer(), mDrawingState.frameNumber,
- mDrawingState.acquireFence, mTransformHint,
- mFlinger->getMaxAcquiredBufferCountForCurrentRefreshRate(
- mOwnerUid));
- decrementPendingBufferCount();
- }
-}
/*
* We don't want to send the layer's transform to input, but rather the
diff --git a/services/surfaceflinger/BufferStateLayer.h b/services/surfaceflinger/BufferStateLayer.h
index f4eb78a..124e91a 100644
--- a/services/surfaceflinger/BufferStateLayer.h
+++ b/services/surfaceflinger/BufferStateLayer.h
@@ -59,7 +59,8 @@
const sp<Fence>& acquireFence, nsecs_t postTime, nsecs_t desiredPresentTime,
bool isAutoTimestamp, const client_cache_t& clientCacheId, uint64_t frameNumber,
std::optional<nsecs_t> dequeueTime, const FrameTimelineInfo& info,
- const sp<ITransactionCompletedListener>& transactionListener) override;
+ const sp<ITransactionCompletedListener>& transactionListener,
+ const sp<IBinder>& releaseBufferEndpoint) override;
bool setAcquireFence(const sp<Fence>& fence) override;
bool setDataspace(ui::Dataspace dataspace) override;
bool setHdrMetadata(const HdrMetadata& hdrMetadata) override;
@@ -97,7 +98,6 @@
// See mPendingBufferTransactions
void decrementPendingBufferCount();
- void bufferMayChange(const sp<GraphicBuffer>& newBuffer) override;
std::atomic<int32_t>* getPendingBufferCounter() override { return &mPendingBufferTransactions; }
std::string getPendingBufferCounterName() override { return mBlastTransactionName; }
diff --git a/services/surfaceflinger/Client.cpp b/services/surfaceflinger/Client.cpp
index aac6c91..8da2e24 100644
--- a/services/surfaceflinger/Client.cpp
+++ b/services/surfaceflinger/Client.cpp
@@ -14,10 +14,6 @@
* limitations under the License.
*/
-// TODO(b/129481165): remove the #pragma below and fix conversion issues
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wconversion"
-
#include <stdint.h>
#include <sys/types.h>
@@ -132,6 +128,3 @@
// ---------------------------------------------------------------------------
}; // namespace android
-
-// TODO(b/129481165): remove the #pragma below and fix conversion issues
-#pragma clang diagnostic pop // ignored "-Wconversion"
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
index b0443f7..256bca9 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
@@ -666,7 +666,6 @@
RETURN_IF_INVALID_DISPLAY(displayId);
auto& displayData = mDisplayData[displayId];
const auto hwcDisplayId = displayData.hwcDisplay->getId();
- LOG_ALWAYS_FATAL_IF(hwcDisplayId == mPrimaryHwcDisplayId);
mPhysicalDisplayIdMap.erase(hwcDisplayId);
mDisplayData.erase(displayId);
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index db59007..28c8213 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -1113,46 +1113,59 @@
return StretchEffect{};
}
+bool Layer::propagateFrameRateForLayerTree(FrameRate parentFrameRate, bool* transactionNeeded) {
+ // The frame rate for layer tree is this layer's frame rate if present, or the parent frame rate
+ const auto frameRate = [&] {
+ if (mDrawingState.frameRate.rate.isValid() ||
+ mDrawingState.frameRate.type == FrameRateCompatibility::NoVote) {
+ return mDrawingState.frameRate;
+ }
+
+ return parentFrameRate;
+ }();
+
+ *transactionNeeded |= setFrameRateForLayerTree(frameRate);
+
+ // The frame rate is propagated to the children
+ bool childrenHaveFrameRate = false;
+ for (const sp<Layer>& child : mCurrentChildren) {
+ childrenHaveFrameRate |=
+ child->propagateFrameRateForLayerTree(frameRate, transactionNeeded);
+ }
+
+ // If we don't have a valid frame rate, but the children do, we set this
+ // layer as NoVote to allow the children to control the refresh rate
+ if (!frameRate.rate.isValid() && frameRate.type != FrameRateCompatibility::NoVote &&
+ childrenHaveFrameRate) {
+ *transactionNeeded |=
+ setFrameRateForLayerTree(FrameRate(Fps(0.0f), FrameRateCompatibility::NoVote));
+ }
+
+ // We return whether this layer ot its children has a vote. We ignore ExactOrMultiple votes for
+ // the same reason we are allowing touch boost for those layers. See
+ // RefreshRateConfigs::getBestRefreshRate for more details.
+ const auto layerVotedWithDefaultCompatibility =
+ frameRate.rate.isValid() && frameRate.type == FrameRateCompatibility::Default;
+ const auto layerVotedWithNoVote = frameRate.type == FrameRateCompatibility::NoVote;
+ const auto layerVotedWithExactCompatibility =
+ frameRate.rate.isValid() && frameRate.type == FrameRateCompatibility::Exact;
+ return layerVotedWithDefaultCompatibility || layerVotedWithNoVote ||
+ layerVotedWithExactCompatibility || childrenHaveFrameRate;
+}
+
void Layer::updateTreeHasFrameRateVote() {
- const auto traverseTree = [&](const LayerVector::Visitor& visitor) {
- auto parent = getParent();
- while (parent) {
- visitor(parent.get());
- parent = parent->getParent();
+ const auto root = [&]() -> sp<Layer> {
+ sp<Layer> layer = this;
+ while (auto parent = layer->getParent()) {
+ layer = parent;
}
+ return layer;
+ }();
- traverse(LayerVector::StateSet::Current, visitor);
- };
-
- // update parents and children about the vote
- // First traverse the tree and count how many layers has votes.
- int layersWithVote = 0;
- traverseTree([&layersWithVote](Layer* layer) {
- const auto layerVotedWithDefaultCompatibility =
- layer->mDrawingState.frameRate.rate.isValid() &&
- layer->mDrawingState.frameRate.type == FrameRateCompatibility::Default;
- const auto layerVotedWithNoVote =
- layer->mDrawingState.frameRate.type == FrameRateCompatibility::NoVote;
- const auto layerVotedWithExactCompatibility =
- layer->mDrawingState.frameRate.type == FrameRateCompatibility::Exact;
-
- // We do not count layers that are ExactOrMultiple for the same reason
- // we are allowing touch boost for those layers. See
- // RefreshRateConfigs::getBestRefreshRate for more details.
- if (layerVotedWithDefaultCompatibility || layerVotedWithNoVote ||
- layerVotedWithExactCompatibility) {
- layersWithVote++;
- }
- });
-
- // Now we can update the tree frame rate vote for each layer in the tree
- const bool treeHasFrameRateVote = layersWithVote > 0;
bool transactionNeeded = false;
+ root->propagateFrameRateForLayerTree({}, &transactionNeeded);
- traverseTree([treeHasFrameRateVote, &transactionNeeded](Layer* layer) {
- transactionNeeded = layer->updateFrameRateForLayerTree(treeHasFrameRateVote);
- });
-
+ // TODO(b/195668952): we probably don't need eTraversalNeeded here
if (transactionNeeded) {
mFlinger->setTransactionFlags(eTraversalNeeded);
}
@@ -1279,42 +1292,23 @@
return surfaceFrame;
}
-bool Layer::updateFrameRateForLayerTree(bool treeHasFrameRateVote) {
- const auto updateDrawingState = [&](FrameRate frameRate) {
- if (mDrawingState.frameRateForLayerTree == frameRate) {
- return false;
- }
-
- mDrawingState.frameRateForLayerTree = frameRate;
- mDrawingState.sequence++;
- mDrawingState.modified = true;
- setTransactionFlags(eTransactionNeeded);
-
- mFlinger->mScheduler->recordLayerHistory(this, systemTime(),
- LayerHistory::LayerUpdateType::SetFrameRate);
-
- return true;
- };
-
- const auto frameRate = mDrawingState.frameRate;
- if (frameRate.rate.isValid() || frameRate.type == FrameRateCompatibility::NoVote) {
- return updateDrawingState(frameRate);
+bool Layer::setFrameRateForLayerTree(FrameRate frameRate) {
+ if (mDrawingState.frameRateForLayerTree == frameRate) {
+ return false;
}
- // This layer doesn't have a frame rate. Check if its ancestors have a vote
- for (sp<Layer> parent = getParent(); parent; parent = parent->getParent()) {
- if (parent->mDrawingState.frameRate.rate.isValid()) {
- return updateDrawingState(parent->mDrawingState.frameRate);
- }
- }
+ mDrawingState.frameRateForLayerTree = frameRate;
- // This layer and its ancestors don't have a frame rate. If one of successors
- // has a vote, return a NoVote for successors to set the vote
- if (treeHasFrameRateVote) {
- return updateDrawingState(FrameRate(Fps(0.0f), FrameRateCompatibility::NoVote));
- }
+ // TODO(b/195668952): we probably don't need to dirty visible regions here
+ // or even store frameRateForLayerTree in mDrawingState
+ mDrawingState.sequence++;
+ mDrawingState.modified = true;
+ setTransactionFlags(eTransactionNeeded);
- return updateDrawingState(frameRate);
+ mFlinger->mScheduler->recordLayerHistory(this, systemTime(),
+ LayerHistory::LayerUpdateType::SetFrameRate);
+
+ return true;
}
Layer::FrameRate Layer::getFrameRateForLayerTree() const {
@@ -2286,10 +2280,7 @@
WindowInfo info = mDrawingState.inputInfo;
info.id = sequence;
-
- if (info.displayId == ADISPLAY_ID_NONE) {
- info.displayId = getLayerStack();
- }
+ info.displayId = getLayerStack();
// Transform that goes from "logical(rotated)" display to physical/unrotated display.
// This is for when inputflinger operates in physical display-space.
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 3ed7ee9..1e5429a 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -280,6 +280,8 @@
Rect bufferCrop;
Rect destinationFrame;
+
+ sp<IBinder> releaseBufferEndpoint;
};
/*
@@ -422,7 +424,8 @@
const client_cache_t& /*clientCacheId*/, uint64_t /* frameNumber */,
std::optional<nsecs_t> /* dequeueTime */,
const FrameTimelineInfo& /*info*/,
- const sp<ITransactionCompletedListener>& /* releaseBufferListener */) {
+ const sp<ITransactionCompletedListener>& /* releaseBufferListener */,
+ const sp<IBinder>& /* releaseBufferEndpoint */) {
return false;
};
virtual bool setAcquireFence(const sp<Fence>& /*fence*/) { return false; };
@@ -708,12 +711,6 @@
virtual uint32_t doTransaction(uint32_t transactionFlags);
/*
- * Called before updating the drawing state buffer. Used by BufferStateLayer to release any
- * unlatched buffers in the drawing state.
- */
- virtual void bufferMayChange(const sp<GraphicBuffer>& /* newBuffer */){};
-
- /*
* Remove relative z for the layer if its relative parent is not part of the
* provided layer tree.
*/
@@ -1058,6 +1055,8 @@
const std::vector<Layer*>& layersInTree);
void updateTreeHasFrameRateVote();
+ bool propagateFrameRateForLayerTree(FrameRate parentFrameRate, bool* transactionNeeded);
+ bool setFrameRateForLayerTree(FrameRate);
void setZOrderRelativeOf(const wp<Layer>& relativeOf);
bool isTrustedOverlay() const;
@@ -1076,8 +1075,6 @@
// Fills in the frame and transform info for the gui::WindowInfo
void fillInputFrameInfo(gui::WindowInfo& info, const ui::Transform& toPhysicalDisplay);
- bool updateFrameRateForLayerTree(bool treeHasFrameRateVote);
-
// Cached properties computed from drawing state
// Effective transform taking into account parent transforms and any parent scaling, which is
// a transform from the current layer coordinate space to display(screen) coordinate space.
diff --git a/services/surfaceflinger/RefreshRateOverlay.cpp b/services/surfaceflinger/RefreshRateOverlay.cpp
index 2adfe14..54fa791 100644
--- a/services/surfaceflinger/RefreshRateOverlay.cpp
+++ b/services/surfaceflinger/RefreshRateOverlay.cpp
@@ -292,7 +292,7 @@
mLayer->setBuffer(buffer, Fence::NO_FENCE, 0, 0, true, {},
mLayer->getHeadFrameNumber(-1 /* expectedPresentTime */),
std::nullopt /* dequeueTime */, FrameTimelineInfo{},
- nullptr /* releaseBufferListener */);
+ nullptr /* releaseBufferListener */, nullptr /* releaseBufferEndpoint */);
mFlinger.mTransactionFlags.fetch_or(eTransactionMask);
}
@@ -306,7 +306,7 @@
mLayer->setBuffer(buffer, Fence::NO_FENCE, 0, 0, true, {},
mLayer->getHeadFrameNumber(-1 /* expectedPresentTime */),
std::nullopt /* dequeueTime */, FrameTimelineInfo{},
- nullptr /* releaseBufferListener */);
+ nullptr /* releaseBufferListener */, nullptr /* releaseBufferEndpoint */);
mFlinger.mTransactionFlags.fetch_or(eTransactionMask);
}
diff --git a/services/surfaceflinger/RenderArea.cpp b/services/surfaceflinger/RenderArea.cpp
index 9a6c853..5fea521 100644
--- a/services/surfaceflinger/RenderArea.cpp
+++ b/services/surfaceflinger/RenderArea.cpp
@@ -14,10 +14,6 @@
* limitations under the License.
*/
-// TODO(b/129481165): remove the #pragma below and fix conversion issues
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wconversion"
-
#include "RenderArea.h"
namespace android {
@@ -33,6 +29,3 @@
}
} // namespace android
-
-// TODO(b/129481165): remove the #pragma below and fix conversion issues
-#pragma clang diagnostic pop // ignored "-Wconversion"
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index bc5b834..b66bb19 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1328,7 +1328,9 @@
return NO_ERROR;
});
- return future.get();
+ // TODO(b/195698395): Propagate error.
+ future.wait();
+ return NO_ERROR;
}
void SurfaceFlinger::setAutoLowLatencyMode(const sp<IBinder>& displayToken, bool on) {
@@ -4203,7 +4205,7 @@
if (layer->setBuffer(buffer, s.acquireFence, postTime, desiredPresentTime, isAutoTimestamp,
s.cachedBuffer, frameNumber, dequeueBufferTimestamp, frameTimelineInfo,
- s.releaseBufferListener)) {
+ s.releaseBufferListener, s.releaseBufferEndpoint)) {
flags |= eTraversalNeeded;
}
} else if (frameTimelineInfo.vsyncId != FrameTimelineInfo::INVALID_VSYNC_ID) {
diff --git a/services/surfaceflinger/tests/ReleaseBufferCallback_test.cpp b/services/surfaceflinger/tests/ReleaseBufferCallback_test.cpp
index 579a26e..c4d42fa 100644
--- a/services/surfaceflinger/tests/ReleaseBufferCallback_test.cpp
+++ b/services/surfaceflinger/tests/ReleaseBufferCallback_test.cpp
@@ -99,10 +99,10 @@
}
static void waitForReleaseBufferCallback(ReleaseBufferCallbackHelper& releaseCallback,
- const ReleaseCallbackId& expectedCallbackId) {
+ const ReleaseCallbackId& expectedReleaseBufferId) {
ReleaseCallbackId actualReleaseBufferId;
releaseCallback.getCallbackData(&actualReleaseBufferId);
- EXPECT_EQ(expectedCallbackId, actualReleaseBufferId);
+ EXPECT_EQ(expectedReleaseBufferId, actualReleaseBufferId);
releaseCallback.verifyNoCallbacks();
}
static ReleaseBufferCallbackHelper* getReleaseBufferCallbackHelper() {
@@ -333,4 +333,60 @@
ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, firstBufferCallbackId));
}
+TEST_F(ReleaseBufferCallbackTest, DISABLED_Merge_Different_Processes) {
+ sp<TransactionCompletedListener> firstCompletedListener = new TransactionCompletedListener();
+ sp<TransactionCompletedListener> secondCompletedListener = new TransactionCompletedListener();
+
+ CallbackHelper callback1, callback2;
+
+ TransactionCompletedListener::setInstance(firstCompletedListener);
+
+ sp<SurfaceControl> layer = createBufferStateLayer();
+ ReleaseBufferCallbackHelper* releaseCallback = getReleaseBufferCallbackHelper();
+
+ sp<GraphicBuffer> firstBuffer = getBuffer();
+ ReleaseCallbackId firstBufferCallbackId(firstBuffer->getId(), generateFrameNumber());
+
+ // Send initial buffer for the layer
+ submitBuffer(layer, firstBuffer, Fence::NO_FENCE, callback1, firstBufferCallbackId,
+ *releaseCallback);
+
+ ExpectedResult expected;
+ expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer,
+ ExpectedResult::Buffer::NOT_ACQUIRED);
+ ASSERT_NO_FATAL_FAILURE(waitForCallback(callback1, expected));
+
+ // Sent a second buffer to allow the first buffer to get released.
+ sp<GraphicBuffer> secondBuffer = getBuffer();
+ ReleaseCallbackId secondBufferCallbackId(secondBuffer->getId(), generateFrameNumber());
+
+ Transaction transaction1;
+ transaction1.setFrameNumber(layer, secondBufferCallbackId.framenumber);
+ transaction1.setBuffer(layer, secondBuffer, secondBufferCallbackId,
+ releaseCallback->getCallback());
+ transaction1.setAcquireFence(layer, Fence::NO_FENCE);
+ transaction1.addTransactionCompletedCallback(callback1.function, callback1.getContext());
+
+ // Set a different TransactionCompletedListener to mimic a second process
+ TransactionCompletedListener::setInstance(secondCompletedListener);
+
+ // Make sure the second "process" has a callback set up.
+ Transaction transaction2;
+ transaction2.addTransactionCompletedCallback(callback2.function, callback2.getContext());
+
+ // This merging order, merge transaction1 first then transaction2, seems to ensure the listener
+ // for transaction2 is ordered first. This makes sure the wrong process is added first to the
+ // layer's vector of listeners. With the bug, only the secondCompletedListener will get the
+ // release callback id, since it's ordered first. Then firstCompletedListener would fail to get
+ // the release callback id and not invoke the release callback.
+ Transaction().merge(std::move(transaction1)).merge(std::move(transaction2)).apply();
+
+ expected = ExpectedResult();
+ expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer,
+ ExpectedResult::Buffer::NOT_ACQUIRED,
+ ExpectedResult::PreviousBuffer::RELEASED);
+ ASSERT_NO_FATAL_FAILURE(waitForCallback(callback1, expected));
+ ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, firstBufferCallbackId));
+}
+
} // namespace android
diff --git a/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp b/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp
index 8e7554b..a4e9d20 100644
--- a/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp
@@ -487,5 +487,40 @@
EXPECT_TRUE(FRAME_RATE_VOTE1.rate.equalsWithMargin(layerHistorySummary[1].desiredRefreshRate));
}
+TEST_P(SetFrameRateTest, addChildForParentWithTreeVote) {
+ EXPECT_CALL(*mMessageQueue, invalidate()).Times(1);
+
+ const auto& layerFactory = GetParam();
+
+ const auto parent = mLayers.emplace_back(layerFactory->createLayer(mFlinger));
+ const auto child1 = mLayers.emplace_back(layerFactory->createLayer(mFlinger));
+ const auto child2 = mLayers.emplace_back(layerFactory->createLayer(mFlinger));
+ const auto childOfChild1 = mLayers.emplace_back(layerFactory->createLayer(mFlinger));
+
+ addChild(parent, child1);
+ addChild(child1, childOfChild1);
+
+ childOfChild1->setFrameRate(FRAME_RATE_VOTE1);
+ commitTransaction();
+ EXPECT_EQ(FRAME_RATE_TREE, parent->getFrameRateForLayerTree());
+ EXPECT_EQ(FRAME_RATE_TREE, child1->getFrameRateForLayerTree());
+ EXPECT_EQ(FRAME_RATE_VOTE1, childOfChild1->getFrameRateForLayerTree());
+ EXPECT_EQ(FRAME_RATE_NO_VOTE, child2->getFrameRateForLayerTree());
+
+ addChild(parent, child2);
+ commitTransaction();
+ EXPECT_EQ(FRAME_RATE_TREE, parent->getFrameRateForLayerTree());
+ EXPECT_EQ(FRAME_RATE_TREE, child1->getFrameRateForLayerTree());
+ EXPECT_EQ(FRAME_RATE_VOTE1, childOfChild1->getFrameRateForLayerTree());
+ EXPECT_EQ(FRAME_RATE_NO_VOTE, child2->getFrameRateForLayerTree());
+
+ childOfChild1->setFrameRate(FRAME_RATE_NO_VOTE);
+ commitTransaction();
+ EXPECT_EQ(FRAME_RATE_NO_VOTE, parent->getFrameRateForLayerTree());
+ EXPECT_EQ(FRAME_RATE_NO_VOTE, child1->getFrameRateForLayerTree());
+ EXPECT_EQ(FRAME_RATE_NO_VOTE, childOfChild1->getFrameRateForLayerTree());
+ EXPECT_EQ(FRAME_RATE_NO_VOTE, child2->getFrameRateForLayerTree());
+}
+
} // namespace
} // namespace android
diff --git a/services/surfaceflinger/tests/unittests/TransactionFrameTracerTest.cpp b/services/surfaceflinger/tests/unittests/TransactionFrameTracerTest.cpp
index 2845d0a..a749ece 100644
--- a/services/surfaceflinger/tests/unittests/TransactionFrameTracerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TransactionFrameTracerTest.cpp
@@ -119,7 +119,7 @@
FrameTracer::FrameEvent::QUEUE, /*duration*/ 0));
layer->setBuffer(buffer, fence, postTime, /*desiredPresentTime*/ 30, false, mClientCache,
frameNumber, dequeueTime, FrameTimelineInfo{},
- nullptr /* releaseBufferCallback */);
+ nullptr /* releaseBufferCallback */, nullptr /* releaseBufferEndpoint*/);
commitTransaction(layer.get());
bool computeVisisbleRegions;
diff --git a/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp b/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp
index 7bf224d..2a7921f 100644
--- a/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp
@@ -119,7 +119,8 @@
1, 0),
mRenderEngine, false);
layer->setBuffer(buffer, fence, 10, 20, false, mClientCache, 1, std::nullopt,
- {/*vsyncId*/ 1, /*inputEventId*/ 0}, nullptr /* releaseBufferCallback */);
+ {/*vsyncId*/ 1, /*inputEventId*/ 0}, nullptr /* releaseBufferCallback */,
+ nullptr /* releaseBufferEndpoint */);
acquireFence->signalForTest(12);
commitTransaction(layer.get());
@@ -147,7 +148,8 @@
1, 0),
mRenderEngine, false);
layer->setBuffer(buffer1, fence1, 10, 20, false, mClientCache, 1, std::nullopt,
- {/*vsyncId*/ 1, /*inputEventId*/ 0}, nullptr /* releaseBufferCallback */);
+ {/*vsyncId*/ 1, /*inputEventId*/ 0}, nullptr /* releaseBufferCallback */,
+ nullptr /* releaseBufferEndpoint */);
EXPECT_EQ(0u, layer->mDrawingState.bufferlessSurfaceFramesTX.size());
ASSERT_NE(nullptr, layer->mDrawingState.bufferSurfaceFrameTX);
const auto droppedSurfaceFrame = layer->mDrawingState.bufferSurfaceFrameTX;
@@ -160,7 +162,8 @@
mRenderEngine, false);
nsecs_t start = systemTime();
layer->setBuffer(buffer2, fence2, 10, 20, false, mClientCache, 1, std::nullopt,
- {/*vsyncId*/ 1, /*inputEventId*/ 0}, nullptr /* releaseBufferCallback */);
+ {/*vsyncId*/ 1, /*inputEventId*/ 0}, nullptr /* releaseBufferCallback */,
+ nullptr /* releaseBufferEndpoint */);
nsecs_t end = systemTime();
acquireFence2->signalForTest(12);
@@ -200,7 +203,8 @@
1, 0),
mRenderEngine, false);
layer->setBuffer(buffer, fence, 10, 20, false, mClientCache, 1, std::nullopt,
- {/*vsyncId*/ 1, /*inputEventId*/ 0}, nullptr /* releaseBufferCallback */);
+ {/*vsyncId*/ 1, /*inputEventId*/ 0}, nullptr /* releaseBufferCallback */,
+ nullptr /* releaseBufferEndpoint */);
acquireFence->signalForTest(12);
EXPECT_EQ(0u, layer->mDrawingState.bufferlessSurfaceFramesTX.size());
@@ -228,7 +232,8 @@
1, 0),
mRenderEngine, false);
layer->setBuffer(buffer, fence, 10, 20, false, mClientCache, 1, std::nullopt,
- {/*vsyncId*/ 1, /*inputEventId*/ 0}, nullptr /* releaseBufferCallback */);
+ {/*vsyncId*/ 1, /*inputEventId*/ 0}, nullptr /* releaseBufferCallback */,
+ nullptr /* releaseBufferEndpoint */);
EXPECT_EQ(0u, layer->mDrawingState.bufferlessSurfaceFramesTX.size());
ASSERT_NE(nullptr, layer->mDrawingState.bufferSurfaceFrameTX);
@@ -260,7 +265,8 @@
1, 0),
mRenderEngine, false);
layer->setBuffer(buffer, fence, 10, 20, false, mClientCache, 1, std::nullopt,
- {/*vsyncId*/ 3, /*inputEventId*/ 0}, nullptr /* releaseBufferCallback */);
+ {/*vsyncId*/ 3, /*inputEventId*/ 0}, nullptr /* releaseBufferCallback */,
+ nullptr /* releaseBufferEndpoint */);
EXPECT_EQ(2u, layer->mDrawingState.bufferlessSurfaceFramesTX.size());
ASSERT_NE(nullptr, layer->mDrawingState.bufferSurfaceFrameTX);
const auto bufferSurfaceFrameTX = layer->mDrawingState.bufferSurfaceFrameTX;
@@ -298,7 +304,8 @@
1, 0),
mRenderEngine, false);
layer->setBuffer(buffer1, fence1, 10, 20, false, mClientCache, 1, std::nullopt,
- {/*vsyncId*/ 1, /*inputEventId*/ 0}, nullptr /* releaseBufferCallback */);
+ {/*vsyncId*/ 1, /*inputEventId*/ 0}, nullptr /* releaseBufferCallback */,
+ nullptr /* releaseBufferEndpoint */);
ASSERT_NE(nullptr, layer->mDrawingState.bufferSurfaceFrameTX);
const auto droppedSurfaceFrame = layer->mDrawingState.bufferSurfaceFrameTX;
@@ -309,7 +316,8 @@
1, 0),
mRenderEngine, false);
layer->setBuffer(buffer2, fence2, 10, 20, false, mClientCache, 1, std::nullopt,
- {/*vsyncId*/ 1, /*inputEventId*/ 0}, nullptr /* releaseBufferCallback */);
+ {/*vsyncId*/ 1, /*inputEventId*/ 0}, nullptr /* releaseBufferCallback */,
+ nullptr /* releaseBufferEndpoint */);
acquireFence2->signalForTest(12);
ASSERT_NE(nullptr, layer->mDrawingState.bufferSurfaceFrameTX);
@@ -339,7 +347,8 @@
1, 0),
mRenderEngine, false);
layer->setBuffer(buffer1, fence1, 10, 20, false, mClientCache, 1, std::nullopt,
- {/*vsyncId*/ 1, /*inputEventId*/ 0}, nullptr /* releaseBufferCallback */);
+ {/*vsyncId*/ 1, /*inputEventId*/ 0}, nullptr /* releaseBufferCallback */,
+ nullptr /* releaseBufferEndpoint */);
EXPECT_EQ(0u, layer->mDrawingState.bufferlessSurfaceFramesTX.size());
ASSERT_NE(nullptr, layer->mDrawingState.bufferSurfaceFrameTX);
const auto droppedSurfaceFrame1 = layer->mDrawingState.bufferSurfaceFrameTX;
@@ -353,7 +362,7 @@
auto dropStartTime1 = systemTime();
layer->setBuffer(buffer2, fence2, 10, 20, false, mClientCache, 1, std::nullopt,
{/*vsyncId*/ FrameTimelineInfo::INVALID_VSYNC_ID, /*inputEventId*/ 0},
- nullptr /* releaseBufferCallback */);
+ nullptr /* releaseBufferCallback */, nullptr /* releaseBufferEndpoint */);
auto dropEndTime1 = systemTime();
EXPECT_EQ(0u, layer->mDrawingState.bufferlessSurfaceFramesTX.size());
ASSERT_NE(nullptr, layer->mDrawingState.bufferSurfaceFrameTX);
@@ -367,7 +376,8 @@
mRenderEngine, false);
auto dropStartTime2 = systemTime();
layer->setBuffer(buffer3, fence3, 10, 20, false, mClientCache, 1, std::nullopt,
- {/*vsyncId*/ 2, /*inputEventId*/ 0}, nullptr /* releaseBufferCallback */);
+ {/*vsyncId*/ 2, /*inputEventId*/ 0}, nullptr /* releaseBufferCallback */,
+ nullptr /* releaseBufferEndpoint */);
auto dropEndTime2 = systemTime();
acquireFence3->signalForTest(12);
@@ -411,7 +421,8 @@
mRenderEngine, false);
layer->setBuffer(buffer1, fence1, 10, 20, false, mClientCache, 1, std::nullopt,
{/*vsyncId*/ 1, /*inputEventId*/ 0},
- nullptr /* releaseBufferCallback */);
+ nullptr /* releaseBufferCallback */,
+ nullptr /* releaseBufferEndpoint */);
layer->setFrameTimelineVsyncForBufferlessTransaction({/*vsyncId*/ 2,
/*inputEventId*/ 0},
10);