Merge "Simplify initialization and add setValue to support parceling"
diff --git a/include/android/input.h b/include/android/input.h
index f03facb..6d2c1b3 100644
--- a/include/android/input.h
+++ b/include/android/input.h
@@ -950,9 +950,10 @@
* and {@link AMotionEvent_fromJava()}.
* After returning, the specified AInputEvent* object becomes invalid and should no longer be used.
* The underlying Java object remains valid and does not change its state.
+ *
+ * Available since API level 31.
*/
-
-void AInputEvent_release(const AInputEvent* event);
+void AInputEvent_release(const AInputEvent* event) __INTRODUCED_IN(31);
/*** Accessors for key events only. ***/
@@ -1004,8 +1005,10 @@
* Creates a native AInputEvent* object that is a copy of the specified Java android.view.KeyEvent.
* The result may be used with generic and KeyEvent-specific AInputEvent_* functions. The object
* returned by this function must be disposed using {@link AInputEvent_release()}.
+ *
+ * Available since API level 31.
*/
-const AInputEvent* AKeyEvent_fromJava(JNIEnv* env, jobject keyEvent);
+const AInputEvent* AKeyEvent_fromJava(JNIEnv* env, jobject keyEvent) __INTRODUCED_IN(31);
/*** Accessors for motion events only. ***/
@@ -1327,8 +1330,10 @@
* android.view.MotionEvent. The result may be used with generic and MotionEvent-specific
* AInputEvent_* functions. The object returned by this function must be disposed using
* {@link AInputEvent_release()}.
+ *
+ * Available since API level 31.
*/
-const AInputEvent* AMotionEvent_fromJava(JNIEnv* env, jobject motionEvent);
+const AInputEvent* AMotionEvent_fromJava(JNIEnv* env, jobject motionEvent) __INTRODUCED_IN(31);
struct AInputQueue;
/**
diff --git a/libs/binder/BpBinder.cpp b/libs/binder/BpBinder.cpp
index 687ee25..92df874 100644
--- a/libs/binder/BpBinder.cpp
+++ b/libs/binder/BpBinder.cpp
@@ -510,7 +510,7 @@
{
ALOGV("onLastStrongRef BpBinder %p handle %d\n", this, binderHandle());
if (CC_UNLIKELY(isRpcBinder())) {
- (void)rpcSession()->sendDecStrong(rpcAddress());
+ (void)rpcSession()->sendDecStrong(this);
return;
}
IF_ALOGV() {
diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp
index dce6f3b..7575252 100644
--- a/libs/binder/Parcel.cpp
+++ b/libs/binder/Parcel.cpp
@@ -237,7 +237,7 @@
return INVALID_OPERATION;
}
}
- const int32_t handle = proxy ? proxy->getPrivateAccessorForId().binderHandle() : 0;
+ const int32_t handle = proxy ? proxy->getPrivateAccessor().binderHandle() : 0;
obj.hdr.type = BINDER_TYPE_HANDLE;
obj.binder = 0; /* Don't pass uninitialized stack data to a remote process */
obj.handle = handle;
@@ -572,7 +572,7 @@
LOG_ALWAYS_FATAL_IF(mData != nullptr, "format must be set before data is written");
if (binder && binder->remoteBinder() && binder->remoteBinder()->isRpcBinder()) {
- markForRpc(binder->remoteBinder()->getPrivateAccessorForId().rpcSession());
+ markForRpc(binder->remoteBinder()->getPrivateAccessor().rpcSession());
}
}
diff --git a/libs/binder/ProcessState.cpp b/libs/binder/ProcessState.cpp
index 8ab0e88..94b2806 100644
--- a/libs/binder/ProcessState.cpp
+++ b/libs/binder/ProcessState.cpp
@@ -212,7 +212,7 @@
binder_node_info_for_ref info;
memset(&info, 0, sizeof(binder_node_info_for_ref));
- info.handle = binder->getPrivateAccessorForId().binderHandle();
+ info.handle = binder->getPrivateAccessor().binderHandle();
status_t result = ioctl(mDriverFD, BINDER_GET_NODE_INFO_FOR_REF, &info);
@@ -301,7 +301,7 @@
return nullptr;
}
- sp<BpBinder> b = BpBinder::create(handle);
+ sp<BpBinder> b = BpBinder::PrivateAccessor::create(handle);
e->binder = b.get();
if (b) e->refs = b->getWeakRefs();
result = b;
diff --git a/libs/binder/RpcSession.cpp b/libs/binder/RpcSession.cpp
index 9395b50..38958c9 100644
--- a/libs/binder/RpcSession.cpp
+++ b/libs/binder/RpcSession.cpp
@@ -29,6 +29,7 @@
#include <android-base/hex.h>
#include <android-base/macros.h>
#include <android_runtime/vm.h>
+#include <binder/BpBinder.h>
#include <binder/Parcel.h>
#include <binder/RpcServer.h>
#include <binder/RpcTransportRaw.h>
@@ -191,7 +192,7 @@
if (wait) {
LOG_ALWAYS_FATAL_IF(mShutdownListener == nullptr, "Shutdown listener not installed");
- mShutdownListener->waitForShutdown(_l);
+ mShutdownListener->waitForShutdown(_l, sp<RpcSession>::fromExisting(this));
LOG_ALWAYS_FATAL_IF(!mThreads.empty(), "Shutdown failed");
}
@@ -215,6 +216,10 @@
sp<RpcSession>::fromExisting(this), reply, flags);
}
+status_t RpcSession::sendDecStrong(const BpBinder* binder) {
+ return sendDecStrong(binder->getPrivateAccessor().rpcAddress());
+}
+
status_t RpcSession::sendDecStrong(uint64_t address) {
ExclusiveConnection connection;
status_t status = ExclusiveConnection::find(sp<RpcSession>::fromExisting(this),
@@ -245,17 +250,19 @@
void RpcSession::WaitForShutdownListener::onSessionAllIncomingThreadsEnded(
const sp<RpcSession>& session) {
(void)session;
- mShutdown = true;
}
void RpcSession::WaitForShutdownListener::onSessionIncomingThreadEnded() {
mCv.notify_all();
}
-void RpcSession::WaitForShutdownListener::waitForShutdown(std::unique_lock<std::mutex>& lock) {
- while (!mShutdown) {
+void RpcSession::WaitForShutdownListener::waitForShutdown(std::unique_lock<std::mutex>& lock,
+ const sp<RpcSession>& session) {
+ while (session->mIncomingConnections.size() > 0) {
if (std::cv_status::timeout == mCv.wait_for(lock, std::chrono::seconds(1))) {
- ALOGE("Waiting for RpcSession to shut down (1s w/o progress).");
+ ALOGE("Waiting for RpcSession to shut down (1s w/o progress): %zu incoming connections "
+ "still.",
+ session->mIncomingConnections.size());
}
}
}
diff --git a/libs/binder/RpcState.cpp b/libs/binder/RpcState.cpp
index 59643ba..11a083a 100644
--- a/libs/binder/RpcState.cpp
+++ b/libs/binder/RpcState.cpp
@@ -56,7 +56,7 @@
bool isRemote = binder->remoteBinder();
bool isRpc = isRemote && binder->remoteBinder()->isRpcBinder();
- if (isRpc && binder->remoteBinder()->getPrivateAccessorForId().rpcSession() != session) {
+ if (isRpc && binder->remoteBinder()->getPrivateAccessor().rpcSession() != session) {
// We need to be able to send instructions over the socket for how to
// connect to a different server, and we also need to let the host
// process know that this is happening.
@@ -85,8 +85,7 @@
if (binder == node.binder) {
if (isRpc) {
// check integrity of data structure
- uint64_t actualAddr =
- binder->remoteBinder()->getPrivateAccessorForId().rpcAddress();
+ uint64_t actualAddr = binder->remoteBinder()->getPrivateAccessor().rpcAddress();
LOG_ALWAYS_FATAL_IF(addr != actualAddr, "Address mismatch %" PRIu64 " vs %" PRIu64,
addr, actualAddr);
}
@@ -185,7 +184,7 @@
// Currently, all binders are assumed to be part of the same session (no
// device global binders in the RPC world).
- it->second.binder = *out = BpBinder::create(session, it->first);
+ it->second.binder = *out = BpBinder::PrivateAccessor::create(session, it->first);
it->second.timesRecd = 1;
return OK;
}
diff --git a/libs/binder/RpcTransportTls.cpp b/libs/binder/RpcTransportTls.cpp
index d40cfc8..63f9339 100644
--- a/libs/binder/RpcTransportTls.cpp
+++ b/libs/binder/RpcTransportTls.cpp
@@ -347,7 +347,7 @@
ALOGE("%s: %s", __PRETTY_FUNCTION__, ret.error().message().c_str());
return ret.error().code() == 0 ? UNKNOWN_ERROR : -ret.error().code();
}
- return OK;
+ return *ret ? -ECANCELED : OK;
}
status_t RpcTransportTls::interruptableWriteFully(FdTrigger* fdTrigger, const void* data,
diff --git a/libs/binder/ServiceManagerHost.cpp b/libs/binder/ServiceManagerHost.cpp
index 59334b7..27cc563 100644
--- a/libs/binder/ServiceManagerHost.cpp
+++ b/libs/binder/ServiceManagerHost.cpp
@@ -74,12 +74,12 @@
result->toString().c_str());
return std::nullopt;
}
- if (!result->stderr.empty()) {
+ if (!result->stderrStr.empty()) {
LOG_HOST("`adb forward tcp:0 tcp:%d` writes to stderr: %s", devicePort,
- result->stderr.c_str());
+ result->stderrStr.c_str());
}
- unsigned int hostPort = parsePortNumber(result->stdout, "host port");
+ unsigned int hostPort = parsePortNumber(result->stdoutStr, "host port");
if (hostPort == 0) return std::nullopt;
return AdbForwarder(hostPort);
@@ -105,9 +105,9 @@
result->toString().c_str());
return;
}
- if (!result->stderr.empty()) {
+ if (!result->stderrStr.empty()) {
LOG_HOST("`adb forward --remove tcp:%d` writes to stderr: %s", *mPort,
- result->stderr.c_str());
+ result->stderrStr.c_str());
}
LOG_HOST("Successfully run `adb forward --remove tcp:%d`", *mPort);
@@ -139,8 +139,8 @@
ALOGE("Command exits with: %s", result->toString().c_str());
return nullptr;
}
- if (!result->stderr.empty()) {
- LOG_HOST("servicedispatcher writes to stderr: %s", result->stderr.c_str());
+ if (!result->stderrStr.empty()) {
+ LOG_HOST("servicedispatcher writes to stderr: %s", result->stderrStr.c_str());
}
if (!result->stdoutEndsWithNewLine()) {
@@ -148,7 +148,7 @@
return nullptr;
}
- unsigned int devicePort = parsePortNumber(result->stdout, "device port");
+ unsigned int devicePort = parsePortNumber(result->stdoutStr, "device port");
if (devicePort == 0) return nullptr;
auto forwardResult = AdbForwarder::forward(devicePort);
diff --git a/libs/binder/UtilsHost.cpp b/libs/binder/UtilsHost.cpp
index d121ce2..52b8f69 100644
--- a/libs/binder/UtilsHost.cpp
+++ b/libs/binder/UtilsHost.cpp
@@ -63,7 +63,7 @@
if (res.exitCode) os << "code=" << *res.exitCode;
if (res.signal) os << "signal=" << *res.signal;
if (res.pid) os << ", pid=" << *res.pid;
- return os << ", stdout=" << res.stdout << ", stderr=" << res.stderr;
+ return os << ", stdout=" << res.stdoutStr << ", stderr=" << res.stderrStr;
}
std::string CommandResult::toString() const {
@@ -142,9 +142,9 @@
int pollRet = poll(fds, nfds, 1000 /* ms timeout */);
if (pollRet == -1) return android::base::ErrnoError() << "poll()";
- if (!handlePoll(&ret.outPipe, outPollFd, &ret.stdout))
+ if (!handlePoll(&ret.outPipe, outPollFd, &ret.stdoutStr))
return android::base::ErrnoError() << "read(stdout)";
- if (!handlePoll(&ret.errPipe, errPollFd, &ret.stderr))
+ if (!handlePoll(&ret.errPipe, errPollFd, &ret.stderrStr))
return android::base::ErrnoError() << "read(stderr)";
if (end && end(ret)) return ret;
diff --git a/libs/binder/UtilsHost.h b/libs/binder/UtilsHost.h
index 0f29f60..98ac4e0 100644
--- a/libs/binder/UtilsHost.h
+++ b/libs/binder/UtilsHost.h
@@ -43,8 +43,8 @@
std::optional<int32_t> exitCode;
std::optional<int32_t> signal;
std::optional<pid_t> pid;
- std::string stdout;
- std::string stderr;
+ std::string stdoutStr;
+ std::string stderrStr;
android::base::unique_fd outPipe;
android::base::unique_fd errPipe;
@@ -55,15 +55,15 @@
std::swap(exitCode, other.exitCode);
std::swap(signal, other.signal);
std::swap(pid, other.pid);
- std::swap(stdout, other.stdout);
- std::swap(stderr, other.stderr);
+ std::swap(stdoutStr, other.stdoutStr);
+ std::swap(stderrStr, other.stderrStr);
return *this;
}
~CommandResult();
[[nodiscard]] std::string toString() const;
[[nodiscard]] bool stdoutEndsWithNewLine() const {
- return !stdout.empty() && stdout.back() == '\n';
+ return !stdoutStr.empty() && stdoutStr.back() == '\n';
}
private:
diff --git a/libs/binder/include/binder/BpBinder.h b/libs/binder/include/binder/BpBinder.h
index 9f2ce1e..c0454b6 100644
--- a/libs/binder/include/binder/BpBinder.h
+++ b/libs/binder/include/binder/BpBinder.h
@@ -39,9 +39,6 @@
class BpBinder : public IBinder
{
public:
- static sp<BpBinder> create(int32_t handle);
- static sp<BpBinder> create(const sp<RpcSession>& session, uint64_t address);
-
/**
* Return value:
* true - this is associated with a socket RpcSession
@@ -116,13 +113,19 @@
KeyedVector<const void*, entry_t> mObjects;
};
- class PrivateAccessorForId {
+ class PrivateAccessor {
private:
friend class BpBinder;
friend class ::android::Parcel;
friend class ::android::ProcessState;
+ friend class ::android::RpcSession;
friend class ::android::RpcState;
- explicit PrivateAccessorForId(const BpBinder* binder) : mBinder(binder) {}
+ explicit PrivateAccessor(const BpBinder* binder) : mBinder(binder) {}
+
+ static sp<BpBinder> create(int32_t handle) { return BpBinder::create(handle); }
+ static sp<BpBinder> create(const sp<RpcSession>& session, uint64_t address) {
+ return BpBinder::create(session, address);
+ }
// valid if !isRpcBinder
int32_t binderHandle() const { return mBinder->binderHandle(); }
@@ -133,14 +136,15 @@
const BpBinder* mBinder;
};
- const PrivateAccessorForId getPrivateAccessorForId() const {
- return PrivateAccessorForId(this);
- }
+ const PrivateAccessor getPrivateAccessor() const { return PrivateAccessor(this); }
private:
- friend PrivateAccessorForId;
+ friend PrivateAccessor;
friend class sp<BpBinder>;
+ static sp<BpBinder> create(int32_t handle);
+ static sp<BpBinder> create(const sp<RpcSession>& session, uint64_t address);
+
struct BinderHandle {
int32_t handle;
};
diff --git a/libs/binder/include/binder/RpcSession.h b/libs/binder/include/binder/RpcSession.h
index 71eb223..6a29c05 100644
--- a/libs/binder/include/binder/RpcSession.h
+++ b/libs/binder/include/binder/RpcSession.h
@@ -151,7 +151,13 @@
[[nodiscard]] status_t transact(const sp<IBinder>& binder, uint32_t code, const Parcel& data,
Parcel* reply, uint32_t flags);
- [[nodiscard]] status_t sendDecStrong(uint64_t address);
+
+ /**
+ * Generally, you should not call this, unless you are testing error
+ * conditions, as this is called automatically by BpBinders when they are
+ * deleted (this is also why a raw pointer is used here)
+ */
+ [[nodiscard]] status_t sendDecStrong(const BpBinder* binder);
~RpcSession();
@@ -170,6 +176,8 @@
friend RpcState;
explicit RpcSession(std::unique_ptr<RpcTransportCtx> ctx);
+ [[nodiscard]] status_t sendDecStrong(uint64_t address);
+
class EventListener : public virtual RefBase {
public:
virtual void onSessionAllIncomingThreadsEnded(const sp<RpcSession>& session) = 0;
@@ -180,12 +188,12 @@
public:
void onSessionAllIncomingThreadsEnded(const sp<RpcSession>& session) override;
void onSessionIncomingThreadEnded() override;
- void waitForShutdown(std::unique_lock<std::mutex>& lock);
+ void waitForShutdown(std::unique_lock<std::mutex>& lock, const sp<RpcSession>& session);
private:
std::condition_variable mCv;
- volatile bool mShutdown = false;
};
+ friend WaitForShutdownListener;
struct RpcConnection : public RefBase {
std::unique_ptr<RpcTransport> rpcTransport;
diff --git a/libs/binder/tests/binderHostDeviceTest.cpp b/libs/binder/tests/binderHostDeviceTest.cpp
index 3f72b8f..eec3b44 100644
--- a/libs/binder/tests/binderHostDeviceTest.cpp
+++ b/libs/binder/tests/binderHostDeviceTest.cpp
@@ -75,10 +75,10 @@
auto debuggableResult = execute(Split("adb shell getprop ro.debuggable", " "), nullptr);
ASSERT_THAT(debuggableResult, Ok());
ASSERT_EQ(0, debuggableResult->exitCode) << *debuggableResult;
- auto debuggableBool = ParseBool(Trim(debuggableResult->stdout));
- ASSERT_NE(ParseBoolResult::kError, debuggableBool) << Trim(debuggableResult->stdout);
+ auto debuggableBool = ParseBool(Trim(debuggableResult->stdoutStr));
+ ASSERT_NE(ParseBoolResult::kError, debuggableBool) << Trim(debuggableResult->stdoutStr);
if (debuggableBool == ParseBoolResult::kFalse) {
- GTEST_SKIP() << "ro.debuggable=" << Trim(debuggableResult->stdout);
+ GTEST_SKIP() << "ro.debuggable=" << Trim(debuggableResult->stdoutStr);
}
auto lsResult = execute(Split("adb shell which servicedispatcher", " "), nullptr);
diff --git a/libs/binder/tests/binderRpcBenchmark.cpp b/libs/binder/tests/binderRpcBenchmark.cpp
index e430c28..55aa57b 100644
--- a/libs/binder/tests/binderRpcBenchmark.cpp
+++ b/libs/binder/tests/binderRpcBenchmark.cpp
@@ -211,10 +211,9 @@
for (size_t tries = 0; tries < 5; tries++) {
usleep(10000);
status = gSession->setupUnixDomainClient(addr.c_str());
- if (status == OK) goto success;
+ if (status == OK) break;
}
- LOG(FATAL) << "Could not connect: " << statusToString(status).c_str();
-success:
+ CHECK_EQ(status, OK) << "Could not connect: " << statusToString(status).c_str();
::benchmark::RunSpecifiedBenchmarks();
return 0;
diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp
index a4e37ad..6bcf102 100644
--- a/libs/binder/tests/binderRpcTest.cpp
+++ b/libs/binder/tests/binderRpcTest.cpp
@@ -53,6 +53,7 @@
#include "RpcCertificateVerifierSimple.h"
using namespace std::chrono_literals;
+using namespace std::placeholders;
using testing::AssertionFailure;
using testing::AssertionResult;
using testing::AssertionSuccess;
@@ -614,25 +615,20 @@
status = session->setupPreconnectedClient({}, [=]() {
return connectTo(UnixSocketAddress(addr.c_str()));
});
- if (status == OK) goto success;
break;
case SocketType::UNIX:
status = session->setupUnixDomainClient(addr.c_str());
- if (status == OK) goto success;
break;
case SocketType::VSOCK:
status = session->setupVsockClient(VMADDR_CID_LOCAL, vsockPort);
- if (status == OK) goto success;
break;
case SocketType::INET:
status = session->setupInetClient("127.0.0.1", serverInfo.port);
- if (status == OK) goto success;
break;
default:
LOG_ALWAYS_FATAL("Unknown socket type");
}
- LOG_ALWAYS_FATAL("Could not connect %s", statusToString(status).c_str());
- success:
+ CHECK_EQ(status, OK) << "Could not connect: " << statusToString(status);
ret.sessions.push_back({session, session->getRootObject()});
}
return ret;
@@ -1435,16 +1431,35 @@
BinderRpcSimple::PrintTestParam);
class RpcTransportTest
- : public ::testing::TestWithParam<std::tuple<SocketType, RpcSecurity, RpcCertificateFormat>> {
+ : public ::testing::TestWithParam<
+ std::tuple<SocketType, RpcSecurity, std::optional<RpcCertificateFormat>>> {
public:
using ConnectToServer = std::function<base::unique_fd()>;
static inline std::string PrintParamInfo(const testing::TestParamInfo<ParamType>& info) {
auto [socketType, rpcSecurity, certificateFormat] = info.param;
- return PrintToString(socketType) + "_" + newFactory(rpcSecurity)->toCString() + "_" +
- PrintToString(certificateFormat);
+ auto ret = PrintToString(socketType) + "_" + newFactory(rpcSecurity)->toCString();
+ if (certificateFormat.has_value()) ret += "_" + PrintToString(*certificateFormat);
+ return ret;
+ }
+ static std::vector<ParamType> getRpcTranportTestParams() {
+ std::vector<RpcTransportTest::ParamType> ret;
+ for (auto socketType : testSocketTypes(false /* hasPreconnected */)) {
+ for (auto rpcSecurity : RpcSecurityValues()) {
+ switch (rpcSecurity) {
+ case RpcSecurity::RAW: {
+ ret.emplace_back(socketType, rpcSecurity, std::nullopt);
+ } break;
+ case RpcSecurity::TLS: {
+ ret.emplace_back(socketType, rpcSecurity, RpcCertificateFormat::PEM);
+ ret.emplace_back(socketType, rpcSecurity, RpcCertificateFormat::DER);
+ } break;
+ }
+ }
+ }
+ return ret;
}
void TearDown() override {
- for (auto& server : mServers) server->shutdown();
+ for (auto& server : mServers) server->shutdownAndWait();
}
// A server that handles client socket connections.
@@ -1452,7 +1467,7 @@
public:
explicit Server() {}
Server(Server&&) = default;
- ~Server() { shutdown(); }
+ ~Server() { shutdownAndWait(); }
[[nodiscard]] AssertionResult setUp() {
auto [socketType, rpcSecurity, certificateFormat] = GetParam();
auto rpcServer = RpcServer::make(newFactory(rpcSecurity));
@@ -1536,17 +1551,17 @@
ASSERT_TRUE(acceptedFd.ok());
auto serverTransport = mCtx->newTransport(std::move(acceptedFd), mFdTrigger.get());
if (serverTransport == nullptr) return; // handshake failed
- std::string message(kMessage);
- ASSERT_EQ(OK,
- serverTransport->interruptableWriteFully(mFdTrigger.get(), message.data(),
- message.size()));
+ ASSERT_TRUE(mPostConnect(serverTransport.get(), mFdTrigger.get()));
}
- void shutdown() {
- mFdTrigger->trigger();
- if (mThread != nullptr) {
- mThread->join();
- mThread = nullptr;
- }
+ void shutdownAndWait() {
+ shutdown();
+ join();
+ }
+ void shutdown() { mFdTrigger->trigger(); }
+
+ void setPostConnect(
+ std::function<AssertionResult(RpcTransport*, FdTrigger* fdTrigger)> fn) {
+ mPostConnect = std::move(fn);
}
private:
@@ -1558,6 +1573,26 @@
std::shared_ptr<RpcCertificateVerifierSimple> mCertVerifier =
std::make_shared<RpcCertificateVerifierSimple>();
bool mSetup = false;
+ // The function invoked after connection and handshake. By default, it is
+ // |defaultPostConnect| that sends |kMessage| to the client.
+ std::function<AssertionResult(RpcTransport*, FdTrigger* fdTrigger)> mPostConnect =
+ Server::defaultPostConnect;
+
+ void join() {
+ if (mThread != nullptr) {
+ mThread->join();
+ mThread = nullptr;
+ }
+ }
+
+ static AssertionResult defaultPostConnect(RpcTransport* serverTransport,
+ FdTrigger* fdTrigger) {
+ std::string message(kMessage);
+ auto status = serverTransport->interruptableWriteFully(fdTrigger, message.data(),
+ message.size());
+ if (status != OK) return AssertionFailure() << statusToString(status);
+ return AssertionSuccess();
+ }
};
class Client {
@@ -1566,8 +1601,6 @@
Client(Client&&) = default;
[[nodiscard]] AssertionResult setUp() {
auto [socketType, rpcSecurity, certificateFormat] = GetParam();
- mFd = mConnectToServer();
- if (!mFd.ok()) return AssertionFailure() << "Cannot connect to server";
mFdTrigger = FdTrigger::make();
mCtx = newFactory(rpcSecurity, mCertVerifier)->newClientCtx();
if (mCtx == nullptr) return AssertionFailure() << "newClientCtx";
@@ -1577,24 +1610,35 @@
std::shared_ptr<RpcCertificateVerifierSimple> getCertVerifier() const {
return mCertVerifier;
}
+ // connect() and do handshake
+ bool setUpTransport() {
+ mFd = mConnectToServer();
+ if (!mFd.ok()) return AssertionFailure() << "Cannot connect to server";
+ mClientTransport = mCtx->newTransport(std::move(mFd), mFdTrigger.get());
+ return mClientTransport != nullptr;
+ }
+ AssertionResult readMessage(const std::string& expectedMessage = kMessage) {
+ LOG_ALWAYS_FATAL_IF(mClientTransport == nullptr, "setUpTransport not called or failed");
+ std::string readMessage(expectedMessage.size(), '\0');
+ status_t readStatus =
+ mClientTransport->interruptableReadFully(mFdTrigger.get(), readMessage.data(),
+ readMessage.size());
+ if (readStatus != OK) {
+ return AssertionFailure() << statusToString(readStatus);
+ }
+ if (readMessage != expectedMessage) {
+ return AssertionFailure()
+ << "Expected " << expectedMessage << ", actual " << readMessage;
+ }
+ return AssertionSuccess();
+ }
void run(bool handshakeOk = true, bool readOk = true) {
- auto clientTransport = mCtx->newTransport(std::move(mFd), mFdTrigger.get());
- if (clientTransport == nullptr) {
+ if (!setUpTransport()) {
ASSERT_FALSE(handshakeOk) << "newTransport returns nullptr, but it shouldn't";
return;
}
ASSERT_TRUE(handshakeOk) << "newTransport does not return nullptr, but it should";
- std::string expectedMessage(kMessage);
- std::string readMessage(expectedMessage.size(), '\0');
- status_t readStatus =
- clientTransport->interruptableReadFully(mFdTrigger.get(), readMessage.data(),
- readMessage.size());
- if (readOk) {
- ASSERT_EQ(OK, readStatus);
- ASSERT_EQ(readMessage, expectedMessage);
- } else {
- ASSERT_NE(OK, readStatus);
- }
+ ASSERT_EQ(readOk, readMessage());
}
private:
@@ -1604,6 +1648,7 @@
std::unique_ptr<RpcTransportCtx> mCtx;
std::shared_ptr<RpcCertificateVerifierSimple> mCertVerifier =
std::make_shared<RpcCertificateVerifierSimple>();
+ std::unique_ptr<RpcTransport> mClientTransport;
};
// Make A trust B.
@@ -1611,8 +1656,9 @@
status_t trust(A* a, B* b) {
auto [socketType, rpcSecurity, certificateFormat] = GetParam();
if (rpcSecurity != RpcSecurity::TLS) return OK;
- auto bCert = b->getCtx()->getCertificate(certificateFormat);
- return a->getCertVerifier()->addTrustedPeerCertificate(certificateFormat, bCert);
+ LOG_ALWAYS_FATAL_IF(!certificateFormat.has_value());
+ auto bCert = b->getCtx()->getCertificate(*certificateFormat);
+ return a->getCertVerifier()->addTrustedPeerCertificate(*certificateFormat, bCert);
}
static constexpr const char* kMessage = "hello";
@@ -1729,17 +1775,68 @@
maliciousClient.run(true, readOk);
}
-std::vector<RpcCertificateFormat> testRpcCertificateFormats() {
- return {
- RpcCertificateFormat::PEM,
- RpcCertificateFormat::DER,
+TEST_P(RpcTransportTest, Trigger) {
+ std::string msg2 = ", world!";
+ std::mutex writeMutex;
+ std::condition_variable writeCv;
+ bool shouldContinueWriting = false;
+ auto serverPostConnect = [&](RpcTransport* serverTransport, FdTrigger* fdTrigger) {
+ std::string message(kMessage);
+ auto status =
+ serverTransport->interruptableWriteFully(fdTrigger, message.data(), message.size());
+ if (status != OK) return AssertionFailure() << statusToString(status);
+
+ {
+ std::unique_lock<std::mutex> lock(writeMutex);
+ if (!writeCv.wait_for(lock, 3s, [&] { return shouldContinueWriting; })) {
+ return AssertionFailure() << "write barrier not cleared in time!";
+ }
+ }
+
+ status = serverTransport->interruptableWriteFully(fdTrigger, msg2.data(), msg2.size());
+ if (status != -ECANCELED)
+ return AssertionFailure() << "When FdTrigger is shut down, interruptableWriteFully "
+ "should return -ECANCELLED, but it is "
+ << statusToString(status);
+ return AssertionSuccess();
};
+
+ auto server = mServers.emplace_back(std::make_unique<Server>()).get();
+ ASSERT_TRUE(server->setUp());
+
+ // Set up client
+ Client client(server->getConnectToServerFn());
+ ASSERT_TRUE(client.setUp());
+
+ // Exchange keys
+ ASSERT_EQ(OK, trust(&client, server));
+ ASSERT_EQ(OK, trust(server, &client));
+
+ server->setPostConnect(serverPostConnect);
+
+ server->start();
+ // connect() to server and do handshake
+ ASSERT_TRUE(client.setUpTransport());
+ // read the first message. This ensures that server has finished handshake and start handling
+ // client fd. Server thread should pause at writeCv.wait_for().
+ ASSERT_TRUE(client.readMessage(kMessage));
+ // Trigger server shutdown after server starts handling client FD. This ensures that the second
+ // write is on an FdTrigger that has been shut down.
+ server->shutdown();
+ // Continues server thread to write the second message.
+ {
+ std::lock_guard<std::mutex> lock(writeMutex);
+ shouldContinueWriting = true;
+ }
+ writeCv.notify_all();
+ // After this line, server thread unblocks and attempts to write the second message, but
+ // shutdown is triggered, so write should failed with -ECANCELLED. See |serverPostConnect|.
+ // On the client side, second read fails with DEAD_OBJECT
+ ASSERT_FALSE(client.readMessage(msg2));
}
INSTANTIATE_TEST_CASE_P(BinderRpc, RpcTransportTest,
- ::testing::Combine(::testing::ValuesIn(testSocketTypes(false)),
- ::testing::ValuesIn(RpcSecurityValues()),
- ::testing::ValuesIn(testRpcCertificateFormats())),
+ ::testing::ValuesIn(RpcTransportTest::getRpcTranportTestParams()),
RpcTransportTest::PrintParamInfo);
} // namespace android
diff --git a/libs/binder/tests/binderUtilsHostTest.cpp b/libs/binder/tests/binderUtilsHostTest.cpp
index fb24836..4330e3e 100644
--- a/libs/binder/tests/binderUtilsHostTest.cpp
+++ b/libs/binder/tests/binderUtilsHostTest.cpp
@@ -34,7 +34,7 @@
auto result = execute({"echo", "foo"}, nullptr);
ASSERT_THAT(result, Ok());
EXPECT_THAT(result->exitCode, Optional(EX_OK));
- EXPECT_EQ(result->stdout, "foo\n");
+ EXPECT_EQ(result->stdoutStr, "foo\n");
}
TEST(UtilsHost, ExecuteLongRunning) {
@@ -44,7 +44,7 @@
std::vector<std::string> args{"sh", "-c",
"sleep 0.5 && echo -n f && sleep 0.5 && echo oo && sleep 1"};
auto result = execute(std::move(args), [](const CommandResult& commandResult) {
- return android::base::EndsWith(commandResult.stdout, "\n");
+ return android::base::EndsWith(commandResult.stdoutStr, "\n");
});
auto elapsed = std::chrono::system_clock::now() - now;
auto elapsedMs = std::chrono::duration_cast<std::chrono::milliseconds>(elapsed).count();
@@ -53,7 +53,7 @@
ASSERT_THAT(result, Ok());
EXPECT_EQ(std::nullopt, result->exitCode);
- EXPECT_EQ(result->stdout, "foo\n");
+ EXPECT_EQ(result->stdoutStr, "foo\n");
}
// ~CommandResult() called, child process is killed.
@@ -70,7 +70,7 @@
std::vector<std::string> args{"sh", "-c",
"sleep 2 && echo -n f && sleep 2 && echo oo && sleep 2"};
auto result = execute(std::move(args), [](const CommandResult& commandResult) {
- return android::base::EndsWith(commandResult.stdout, "\n");
+ return android::base::EndsWith(commandResult.stdoutStr, "\n");
});
auto elapsed = std::chrono::system_clock::now() - now;
auto elapsedMs = std::chrono::duration_cast<std::chrono::milliseconds>(elapsed).count();
@@ -79,7 +79,7 @@
ASSERT_THAT(result, Ok());
EXPECT_EQ(std::nullopt, result->exitCode);
- EXPECT_EQ(result->stdout, "foo\n");
+ EXPECT_EQ(result->stdoutStr, "foo\n");
}
// ~CommandResult() called, child process is killed.
diff --git a/libs/binder/tests/unit_fuzzers/Android.bp b/libs/binder/tests/unit_fuzzers/Android.bp
index b1263e8..6f054d2 100644
--- a/libs/binder/tests/unit_fuzzers/Android.bp
+++ b/libs/binder/tests/unit_fuzzers/Android.bp
@@ -51,7 +51,6 @@
cc_fuzz {
name: "binder_bpBinderFuzz",
defaults: ["binder_fuzz_defaults"],
- host_supported: false,
srcs: ["BpBinderFuzz.cpp"],
}
diff --git a/libs/binder/tests/unit_fuzzers/BpBinderFuzz.cpp b/libs/binder/tests/unit_fuzzers/BpBinderFuzz.cpp
index c50279b..20c5569 100644
--- a/libs/binder/tests/unit_fuzzers/BpBinderFuzz.cpp
+++ b/libs/binder/tests/unit_fuzzers/BpBinderFuzz.cpp
@@ -19,8 +19,15 @@
#include <commonFuzzHelpers.h>
#include <fuzzer/FuzzedDataProvider.h>
+#include <android-base/logging.h>
#include <binder/BpBinder.h>
#include <binder/IServiceManager.h>
+#include <binder/RpcServer.h>
+#include <binder/RpcSession.h>
+
+#include <signal.h>
+#include <sys/prctl.h>
+#include <thread>
namespace android {
@@ -28,13 +35,30 @@
extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
FuzzedDataProvider fdp(data, size);
- // TODO: In the future it would be more effective to fork a new process and then pass a BBinder
- // to your process. Right now this is not implemented because it would involved fuzzing IPC on a
- // forked process, and libfuzzer will not be able to handle code coverage. This would lead to
- // crashes that are not easy to diagnose.
- int32_t handle = fdp.ConsumeIntegralInRange<int32_t>(0, 1024);
- sp<BpBinder> bpbinder = BpBinder::create(handle);
- if (bpbinder == nullptr) return 0;
+ std::string addr = std::string(getenv("TMPDIR") ?: "/tmp") + "/binderRpcBenchmark";
+ (void)unlink(addr.c_str());
+
+ sp<RpcServer> server = RpcServer::make();
+
+ // use RPC binder because fuzzer can't get coverage from another process.
+ auto thread = std::thread([&]() {
+ prctl(PR_SET_PDEATHSIG, SIGHUP); // racey, okay
+ server->setRootObject(sp<BBinder>::make());
+ server->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction();
+ CHECK_EQ(OK, server->setupUnixDomainServer(addr.c_str()));
+ server->join();
+ });
+
+ sp<RpcSession> session = RpcSession::make();
+ status_t status;
+ for (size_t tries = 0; tries < 5; tries++) {
+ usleep(10000);
+ status = session->setupUnixDomainClient(addr.c_str());
+ if (status == OK) break;
+ }
+ CHECK_EQ(status, OK) << "Unable to connect";
+
+ sp<BpBinder> bpBinder = session->getRootObject()->remoteBinder();
// To prevent memory from running out from calling too many add item operations.
const uint32_t MAX_RUNS = 2048;
@@ -43,12 +67,16 @@
while (fdp.remaining_bytes() > 0 && count++ < MAX_RUNS) {
if (fdp.ConsumeBool()) {
- callArbitraryFunction(&fdp, gBPBinderOperations, bpbinder, s_recipient);
+ callArbitraryFunction(&fdp, gBPBinderOperations, bpBinder, s_recipient);
} else {
- callArbitraryFunction(&fdp, gIBinderOperations, bpbinder.get());
+ callArbitraryFunction(&fdp, gIBinderOperations, bpBinder.get());
}
}
+ CHECK(session->shutdownAndWait(true)) << "couldn't shutdown session";
+ CHECK(server->shutdown()) << "couldn't shutdown server";
+ thread.join();
+
return 0;
}
} // namespace android
diff --git a/libs/binder/tests/unit_fuzzers/BpBinderFuzzFunctions.h b/libs/binder/tests/unit_fuzzers/BpBinderFuzzFunctions.h
index 6ca0e2f..741987f 100644
--- a/libs/binder/tests/unit_fuzzers/BpBinderFuzzFunctions.h
+++ b/libs/binder/tests/unit_fuzzers/BpBinderFuzzFunctions.h
@@ -52,7 +52,7 @@
const sp<IBinder::DeathRecipient>& s_recipient) -> void {
// Clean up possible leftover memory.
wp<IBinder::DeathRecipient> outRecipient(nullptr);
- bpbinder->sendObituary();
+ if (!bpbinder->isRpcBinder()) bpbinder->sendObituary();
bpbinder->unlinkToDeath(nullptr, reinterpret_cast<void*>(&kBpBinderCookie), 0,
&outRecipient);
@@ -72,7 +72,9 @@
[](FuzzedDataProvider*, const sp<BpBinder>& bpbinder,
const sp<IBinder::DeathRecipient>&) -> void { bpbinder->remoteBinder(); },
[](FuzzedDataProvider*, const sp<BpBinder>& bpbinder,
- const sp<IBinder::DeathRecipient>&) -> void { bpbinder->sendObituary(); },
+ const sp<IBinder::DeathRecipient>&) -> void {
+ if (!bpbinder->isRpcBinder()) bpbinder->sendObituary();
+ },
[](FuzzedDataProvider* fdp, const sp<BpBinder>& bpbinder,
const sp<IBinder::DeathRecipient>&) -> void {
uint32_t uid = fdp->ConsumeIntegral<uint32_t>();
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index d70e2e9..34b5cdc 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -619,12 +619,18 @@
std::vector<PhysicalDisplayId> SurfaceFlinger::getPhysicalDisplayIdsLocked() const {
std::vector<PhysicalDisplayId> displayIds;
displayIds.reserve(mPhysicalDisplayTokens.size());
+ const auto defaultDisplayId = [this]() REQUIRES(mStateLock) {
+ if (const auto display = getDefaultDisplayDeviceLocked()) {
+ return display->getPhysicalId();
+ }
- const auto internalDisplayId = getInternalDisplayIdLocked();
- displayIds.push_back(internalDisplayId);
+ // fallback to the internal display id if the active display is unknown
+ return getInternalDisplayIdLocked();
+ }();
+ displayIds.push_back(defaultDisplayId);
for (const auto& [id, token] : mPhysicalDisplayTokens) {
- if (id != internalDisplayId) {
+ if (id != defaultDisplayId) {
displayIds.push_back(id);
}
}