Merge "Revert "Remove leak detection options""
diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp
index bb6639e..e84428e 100644
--- a/cmds/installd/InstalldNativeService.cpp
+++ b/cmds/installd/InstalldNativeService.cpp
@@ -416,10 +416,12 @@
*/
static int restorecon_app_data_lazy(const std::string& path, const std::string& seInfo, uid_t uid,
bool existing) {
+ ScopedTrace tracer("restorecon-lazy");
int res = 0;
char* before = nullptr;
char* after = nullptr;
if (!existing) {
+ ScopedTrace tracer("new-path");
if (selinux_android_restorecon_pkgdir(path.c_str(), seInfo.c_str(), uid,
SELINUX_ANDROID_RESTORECON_RECURSE) < 0) {
PLOG(ERROR) << "Failed recursive restorecon for " << path;
@@ -446,6 +448,7 @@
// If the initial top-level restorecon above changed the label, then go
// back and restorecon everything recursively
if (strcmp(before, after)) {
+ ScopedTrace tracer("label-change");
if (existing) {
LOG(DEBUG) << "Detected label change from " << before << " to " << after << " at "
<< path << "; running recursive restorecon";
@@ -480,11 +483,15 @@
static int prepare_app_dir(const std::string& path, mode_t target_mode, uid_t uid, gid_t gid,
long project_id) {
- if (fs_prepare_dir_strict(path.c_str(), target_mode, uid, gid) != 0) {
- PLOG(ERROR) << "Failed to prepare " << path;
- return -1;
+ {
+ ScopedTrace tracer("prepare-dir");
+ if (fs_prepare_dir_strict(path.c_str(), target_mode, uid, gid) != 0) {
+ PLOG(ERROR) << "Failed to prepare " << path;
+ return -1;
+ }
}
if (internal_storage_has_project_id()) {
+ ScopedTrace tracer("set-quota");
return set_quota_project_id(path, project_id, true);
}
return 0;
@@ -493,14 +500,20 @@
static int prepare_app_cache_dir(const std::string& parent, const char* name, mode_t target_mode,
uid_t uid, gid_t gid, long project_id) {
auto path = StringPrintf("%s/%s", parent.c_str(), name);
- int ret = prepare_app_cache_dir(parent, name, target_mode, uid, gid);
+ int ret;
+ {
+ ScopedTrace tracer("prepare-cache-dir");
+ ret = prepare_app_cache_dir(parent, name, target_mode, uid, gid);
+ }
if (ret == 0 && internal_storage_has_project_id()) {
+ ScopedTrace tracer("set-quota-cache-dir");
return set_quota_project_id(path, project_id, true);
}
return ret;
}
static bool prepare_app_profile_dir(const std::string& packageName, int32_t appId, int32_t userId) {
+ ScopedTrace tracer("prepare-app-profile");
int32_t uid = multiuser_get_uid(userId, appId);
int shared_app_gid = multiuser_get_shared_gid(userId, appId);
if (shared_app_gid == -1) {
@@ -633,6 +646,7 @@
int32_t previousUid, int32_t cacheGid,
const std::string& seInfo, mode_t targetMode,
long projectIdApp, long projectIdCache) {
+ ScopedTrace tracer("create-dirs");
struct stat st{};
bool parent_dir_exists = (stat(path.c_str(), &st) == 0);
@@ -709,6 +723,7 @@
long projectIdCache = get_project_id(uid, PROJECT_ID_APP_CACHE_START);
if (flags & FLAG_STORAGE_CE) {
+ ScopedTrace tracer("ce");
auto path = create_data_user_ce_package_path(uuid_, userId, pkgname);
auto status = createAppDataDirs(path, uid, uid, previousUid, cacheGid, seInfo, targetMode,
@@ -735,6 +750,7 @@
}
}
if (flags & FLAG_STORAGE_DE) {
+ ScopedTrace tracer("de");
auto path = create_data_user_de_package_path(uuid_, userId, pkgname);
auto status = createAppDataDirs(path, uid, uid, previousUid, cacheGid, seInfo, targetMode,
@@ -752,13 +768,14 @@
}
if (flags & FLAG_STORAGE_SDK) {
+ ScopedTrace tracer("sdk");
// Safe to ignore status since we can retry creating this by calling reconcileSdkData
auto ignore = createSdkSandboxDataPackageDirectory(uuid, packageName, userId, appId, flags);
if (!ignore.isOk()) {
PLOG(WARNING) << "Failed to create sdk data package directory for " << packageName;
}
-
} else {
+ ScopedTrace tracer("destroy-sdk");
// Package does not need sdk storage. Remove it.
destroySdkSandboxDataPackageDirectory(uuid, packageName, userId, flags);
}
@@ -1850,6 +1867,8 @@
CHECK_ARGUMENT_UUID(uuid);
LOCK_USER();
+ ScopedTrace tracer("create-user-data");
+
const char* uuid_ = uuid ? uuid->c_str() : nullptr;
if (flags & FLAG_STORAGE_DE) {
if (uuid_ == nullptr) {
@@ -3555,22 +3574,22 @@
std::lock_guard<std::recursive_mutex> lock(mMountsLock);
std::string mirrorVolCePath(StringPrintf("%s/%s", kDataMirrorCePath, uuid_));
- if (fs_prepare_dir(mirrorVolCePath.c_str(), 0711, AID_SYSTEM, AID_SYSTEM) != 0) {
+ if (fs_prepare_dir(mirrorVolCePath.c_str(), 0511, AID_SYSTEM, AID_SYSTEM) != 0) {
return error("Failed to create CE data mirror");
}
std::string mirrorVolDePath(StringPrintf("%s/%s", kDataMirrorDePath, uuid_));
- if (fs_prepare_dir(mirrorVolDePath.c_str(), 0711, AID_SYSTEM, AID_SYSTEM) != 0) {
+ if (fs_prepare_dir(mirrorVolDePath.c_str(), 0511, AID_SYSTEM, AID_SYSTEM) != 0) {
return error("Failed to create DE data mirror");
}
std::string mirrorVolMiscCePath(StringPrintf("%s/%s", kMiscMirrorCePath, uuid_));
- if (fs_prepare_dir(mirrorVolMiscCePath.c_str(), 0711, AID_SYSTEM, AID_SYSTEM) != 0) {
+ if (fs_prepare_dir(mirrorVolMiscCePath.c_str(), 0511, AID_SYSTEM, AID_SYSTEM) != 0) {
return error("Failed to create CE misc mirror");
}
std::string mirrorVolMiscDePath(StringPrintf("%s/%s", kMiscMirrorDePath, uuid_));
- if (fs_prepare_dir(mirrorVolMiscDePath.c_str(), 0711, AID_SYSTEM, AID_SYSTEM) != 0) {
+ if (fs_prepare_dir(mirrorVolMiscDePath.c_str(), 0511, AID_SYSTEM, AID_SYSTEM) != 0) {
return error("Failed to create DE misc mirror");
}
diff --git a/cmds/installd/SysTrace.h b/cmds/installd/SysTrace.h
index 18506a9..0deaeb4 100644
--- a/cmds/installd/SysTrace.h
+++ b/cmds/installd/SysTrace.h
@@ -19,4 +19,16 @@
namespace android::installd {
void atrace_pm_begin(const char*);
void atrace_pm_end();
+
+class ScopedTrace {
+public:
+ explicit ScopedTrace(const char* label) { atrace_pm_begin(label); }
+ ~ScopedTrace() { atrace_pm_end(); }
+
+private:
+ ScopedTrace(const ScopedTrace&) = delete;
+ ScopedTrace& operator=(const ScopedTrace&) = delete;
+ ScopedTrace(ScopedTrace&&) = delete;
+ ScopedTrace& operator=(ScopedTrace&&) = delete;
+};
} /* namespace android::installd */
diff --git a/cmds/installd/otapreopt_chroot.cpp b/cmds/installd/otapreopt_chroot.cpp
index 1b7acab..c86993c 100644
--- a/cmds/installd/otapreopt_chroot.cpp
+++ b/cmds/installd/otapreopt_chroot.cpp
@@ -165,7 +165,8 @@
// Bind mount necessary directories.
constexpr const char* kBindMounts[] = {
- "/data", "/dev", "/proc", "/sys"
+ "/data", "/dev", "/proc", "/sys",
+ "/sys/fs/selinux" /* Required for apexd which includes libselinux */
};
for (size_t i = 0; i < arraysize(kBindMounts); ++i) {
std::string trg = StringPrintf("/postinstall%s", kBindMounts[i]);
diff --git a/libs/binder/Android.bp b/libs/binder/Android.bp
index 3433138..deff76b 100644
--- a/libs/binder/Android.bp
+++ b/libs/binder/Android.bp
@@ -525,7 +525,6 @@
"libbase",
"libbinder",
"libbinder_ndk",
- "libcutils_sockets",
"liblog",
"libutils",
],
diff --git a/libs/binder/ProcessState.cpp b/libs/binder/ProcessState.cpp
index 3fa6867..02b0447 100644
--- a/libs/binder/ProcessState.cpp
+++ b/libs/binder/ProcessState.cpp
@@ -192,6 +192,7 @@
AutoMutex _l(mLock);
if (!mThreadPoolStarted) {
if (mMaxThreads == 0) {
+ // see also getThreadPoolMaxTotalThreadCount
ALOGW("Extra binder thread started, but 0 threads requested. Do not use "
"*startThreadPool when zero threads are requested.");
}
@@ -407,6 +408,11 @@
mKernelStartedThreads++;
pthread_mutex_unlock(&mThreadCountLock);
}
+ // TODO: if startThreadPool is called on another thread after the process
+ // starts up, the kernel might think that it already requested those
+ // binder threads, and additional won't be started. This is likely to
+ // cause deadlocks, and it will also cause getThreadPoolMaxTotalThreadCount
+ // to return too high of a value.
}
status_t ProcessState::setThreadPoolMaxThreadCount(size_t maxThreads) {
@@ -426,12 +432,32 @@
pthread_mutex_lock(&mThreadCountLock);
base::ScopeGuard detachGuard = [&]() { pthread_mutex_unlock(&mThreadCountLock); };
- // may actually be one more than this, if join is called
if (mThreadPoolStarted) {
- return mCurrentThreads < mKernelStartedThreads
- ? mMaxThreads
- : mMaxThreads + mCurrentThreads - mKernelStartedThreads;
+ LOG_ALWAYS_FATAL_IF(mKernelStartedThreads > mMaxThreads + 1,
+ "too many kernel-started threads: %zu > %zu + 1", mKernelStartedThreads,
+ mMaxThreads);
+
+ // calling startThreadPool starts a thread
+ size_t threads = 1;
+
+ // the kernel is configured to start up to mMaxThreads more threads
+ threads += mMaxThreads;
+
+ // Users may call IPCThreadState::joinThreadPool directly. We don't
+ // currently have a way to count this directly (it could be added by
+ // adding a separate private joinKernelThread method in IPCThreadState).
+ // So, if we are in a race between the kernel thread variable being
+ // incremented in this file and mCurrentThreads being incremented
+ // in IPCThreadState, temporarily forget about the extra join threads.
+ // This is okay, because most callers of this method only care about
+ // having 0, 1, or more threads.
+ if (mCurrentThreads > mKernelStartedThreads) {
+ threads += mCurrentThreads - mKernelStartedThreads;
+ }
+
+ return threads;
}
+
// must not be initialized or maybe has poll thread setup, we
// currently don't track this in libbinder
LOG_ALWAYS_FATAL_IF(mKernelStartedThreads != 0,
diff --git a/libs/binder/include/binder/IPCThreadState.h b/libs/binder/include/binder/IPCThreadState.h
index d261c21..9347ce4 100644
--- a/libs/binder/include/binder/IPCThreadState.h
+++ b/libs/binder/include/binder/IPCThreadState.h
@@ -147,7 +147,12 @@
void flushCommands();
bool flushIfNeeded();
- // For main functions - dangerous for libraries to use
+ // Adds the current thread into the binder threadpool.
+ //
+ // This is in addition to any threads which are started
+ // with startThreadPool. Libraries should not call this
+ // function, as they may be loaded into processes which
+ // try to configure the threadpool differently.
void joinThreadPool(bool isMain = true);
// Stop the local process.
diff --git a/libs/binder/include/binder/ProcessState.h b/libs/binder/include/binder/ProcessState.h
index 81391e9..9dc370b 100644
--- a/libs/binder/include/binder/ProcessState.h
+++ b/libs/binder/include/binder/ProcessState.h
@@ -52,7 +52,26 @@
sp<IBinder> getContextObject(const sp<IBinder>& caller);
- // For main functions - dangerous for libraries to use
+ // This should be called before startThreadPool at the beginning
+ // of a program, and libraries should never call it because programs
+ // should configure their own threadpools. The threadpool size can
+ // never be decreased.
+ //
+ // The 'maxThreads' value refers to the total number of threads
+ // that will be started by the kernel. This is in addition to any
+ // threads started by 'startThreadPool' or 'joinRpcThreadpool'.
+ status_t setThreadPoolMaxThreadCount(size_t maxThreads);
+
+ // Libraries should not call this, as processes should configure
+ // threadpools themselves. Should be called in the main function
+ // directly before any code executes or joins the threadpool.
+ //
+ // Starts one thread, PLUS those requested in setThreadPoolMaxThreadCount,
+ // PLUS those manually requested in joinThreadPool.
+ //
+ // For instance, if setThreadPoolMaxCount(3) is called and
+ // startThreadpPool (+1 thread) and joinThreadPool (+1 thread)
+ // are all called, then up to 5 threads can be started.
void startThreadPool();
[[nodiscard]] bool becomeContextManager();
@@ -63,8 +82,6 @@
// TODO: deprecate.
void spawnPooledThread(bool isMain);
- // For main functions - dangerous for libraries to use
- status_t setThreadPoolMaxThreadCount(size_t maxThreads);
status_t enableOnewaySpamDetection(bool enable);
// Set the name of the current thread to look like a threadpool
diff --git a/libs/binder/include_rpc_unstable/binder_rpc_unstable.hpp b/libs/binder/include_rpc_unstable/binder_rpc_unstable.hpp
index a157792..7d0acd1 100644
--- a/libs/binder/include_rpc_unstable/binder_rpc_unstable.hpp
+++ b/libs/binder/include_rpc_unstable/binder_rpc_unstable.hpp
@@ -40,12 +40,13 @@
[[nodiscard]] ARpcServer* ARpcServer_newVsock(AIBinder* service, unsigned int cid,
unsigned int port);
-// Starts a Unix domain RPC server with a given init-managed Unix domain `name`
+// Starts a Unix domain RPC server with an open raw socket file descriptor
// and a given root IBinder object.
-// The socket should be created in init.rc with the same `name`.
+// The socket should be created and bound to an address.
// Returns an opaque handle to the running server instance, or null if the server
// could not be started.
-[[nodiscard]] ARpcServer* ARpcServer_newInitUnixDomain(AIBinder* service, const char* name);
+// The socket will be closed by the server once the server goes out of scope.
+[[nodiscard]] ARpcServer* ARpcServer_newBoundSocket(AIBinder* service, int socketFd);
// Starts an RPC server that bootstraps sessions using an existing Unix domain
// socket pair, with a given root IBinder object.
diff --git a/libs/binder/libbinder_rpc_unstable.cpp b/libs/binder/libbinder_rpc_unstable.cpp
index a167f23..f51cd9b 100644
--- a/libs/binder/libbinder_rpc_unstable.cpp
+++ b/libs/binder/libbinder_rpc_unstable.cpp
@@ -105,22 +105,15 @@
return createObjectHandle<ARpcServer>(server);
}
-ARpcServer* ARpcServer_newInitUnixDomain(AIBinder* service, const char* name) {
+ARpcServer* ARpcServer_newBoundSocket(AIBinder* service, int socketFd) {
auto server = RpcServer::make();
- auto fd = unique_fd(android_get_control_socket(name));
+ auto fd = unique_fd(socketFd);
if (!fd.ok()) {
- LOG(ERROR) << "Failed to get fd for the socket:" << name;
+ LOG(ERROR) << "Invalid socket fd " << socketFd;
return nullptr;
}
- // Control socket fds are inherited from init, so they don't have O_CLOEXEC set.
- // But we don't want any child processes to inherit the socket we are running
- // the server on, so attempt to set the flag now.
- if (fcntl(fd, F_SETFD, FD_CLOEXEC) != 0) {
- LOG(WARNING) << "Failed to set CLOEXEC on control socket with name " << name
- << " error: " << errno;
- }
if (status_t status = server->setupRawSocketServer(std::move(fd)); status != OK) {
- LOG(ERROR) << "Failed to set up Unix Domain RPC server with name " << name
+ LOG(ERROR) << "Failed to set up RPC server with fd " << socketFd
<< " error: " << statusToString(status).c_str();
return nullptr;
}
diff --git a/libs/binder/libbinder_rpc_unstable.map.txt b/libs/binder/libbinder_rpc_unstable.map.txt
index 63679c2..50f7deb 100644
--- a/libs/binder/libbinder_rpc_unstable.map.txt
+++ b/libs/binder/libbinder_rpc_unstable.map.txt
@@ -3,7 +3,7 @@
ARpcServer_free;
ARpcServer_join;
ARpcServer_newInet;
- ARpcServer_newInitUnixDomain;
+ ARpcServer_newBoundSocket;
ARpcServer_newVsock;
ARpcServer_shutdown;
ARpcServer_start;
diff --git a/libs/binder/ndk/include_platform/android/binder_process.h b/libs/binder/ndk/include_platform/android/binder_process.h
index 3fbe90d..68528e1 100644
--- a/libs/binder/ndk/include_platform/android/binder_process.h
+++ b/libs/binder/ndk/include_platform/android/binder_process.h
@@ -24,7 +24,14 @@
__BEGIN_DECLS
/**
- * This creates a threadpool for incoming binder transactions if it has not already been created.
+ * This creates a threadpool for incoming binder transactions if it has not already been created,
+ * spawning one thread, and allowing the kernel to lazily start threads according to the count
+ * that is specified in ABinderProcess_setThreadPoolMaxThreadCount.
+ *
+ * For instance, if ABinderProcess_setThreadPoolMaxThreadCount(3) is called,
+ * ABinderProcess_startThreadPool() is called (+1 thread) then the main thread calls
+ * ABinderProcess_joinThreadPool() (+1 thread), up to *5* total threads will be started
+ * (2 directly, and 3 more if the kernel starts them lazily).
*
* When using this, it is expected that ABinderProcess_setupPolling and
* ABinderProcess_handlePolledCommands are not used.
@@ -36,7 +43,12 @@
/**
* This sets the maximum number of threads that can be started in the threadpool. By default, after
* startThreadPool is called, this is 15. If it is called additional times, it will only prevent
- * the kernel from starting new threads and will not delete already existing threads.
+ * the kernel from starting new threads and will not delete already existing threads. This should
+ * be called once before startThreadPool. The number of threads can never decrease.
+ *
+ * This count refers to the number of threads that will be created lazily by the kernel, in
+ * addition to the threads created by ABinderProcess_startThreadPool or
+ * ABinderProcess_joinThreadPool.
*
* Do not use this from a library. Apps setup their own threadpools, and otherwise, the main
* function should be responsible for configuring the threadpool for the entire application.
@@ -50,8 +62,9 @@
*/
bool ABinderProcess_isThreadPoolStarted(void);
/**
- * This adds the current thread to the threadpool. This may cause the threadpool to exceed the
- * maximum size.
+ * This adds the current thread to the threadpool. This thread will be in addition to the thread
+ * started by ABinderProcess_startThreadPool and the lazy kernel-started threads specified by
+ * ABinderProcess_setThreadPoolMaxThreadCount.
*
* Do not use this from a library. Apps setup their own threadpools, and otherwise, the main
* function should be responsible for configuring the threadpool for the entire application.
diff --git a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp
index eba0556..27ce615 100644
--- a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp
+++ b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp
@@ -283,8 +283,8 @@
TEST(NdkBinder, CheckServiceThatDoesExist) {
AIBinder* binder = AServiceManager_checkService(kExistingNonNdkService);
- EXPECT_NE(nullptr, binder);
- EXPECT_EQ(STATUS_OK, AIBinder_ping(binder));
+ ASSERT_NE(nullptr, binder) << "Could not get " << kExistingNonNdkService;
+ EXPECT_EQ(STATUS_OK, AIBinder_ping(binder)) << "Could not ping " << kExistingNonNdkService;
AIBinder_decStrong(binder);
}
diff --git a/libs/binder/rust/rpcbinder/src/server.rs b/libs/binder/rust/rpcbinder/src/server.rs
index c87876a..81f68f5 100644
--- a/libs/binder/rust/rpcbinder/src/server.rs
+++ b/libs/binder/rust/rpcbinder/src/server.rs
@@ -57,26 +57,17 @@
}
/// Creates a binder RPC server, serving the supplied binder service implementation on the given
- /// socket file name. The socket should be initialized in init.rc with the same name.
- pub fn new_init_unix_domain(
- mut service: SpIBinder,
- socket_name: &str,
- ) -> Result<RpcServer, Error> {
- let socket_name = match CString::new(socket_name) {
- Ok(s) => s,
- Err(e) => {
- log::error!("Cannot convert {} to CString. Error: {:?}", socket_name, e);
- return Err(Error::from(ErrorKind::InvalidInput));
- }
- };
+ /// socket file descriptor. The socket should be bound to an address before calling this
+ /// function.
+ pub fn new_bound_socket(mut service: SpIBinder, socket_fd: OwnedFd) -> Result<RpcServer, Error> {
let service = service.as_native_mut();
// SAFETY: Service ownership is transferring to the server and won't be valid afterward.
// Plus the binder objects are threadsafe.
+ // The server takes ownership of the socket FD.
unsafe {
- Self::checked_from_ptr(binder_rpc_unstable_bindgen::ARpcServer_newInitUnixDomain(
- service,
- socket_name.as_ptr(),
+ Self::checked_from_ptr(binder_rpc_unstable_bindgen::ARpcServer_newBoundSocket(
+ service, socket_fd.into_raw_fd(),
))
}
}
diff --git a/libs/binder/rust/src/state.rs b/libs/binder/rust/src/state.rs
index cc18741..b35511d 100644
--- a/libs/binder/rust/src/state.rs
+++ b/libs/binder/rust/src/state.rs
@@ -23,6 +23,9 @@
impl ProcessState {
/// Start the Binder IPC thread pool
+ ///
+ /// Starts 1 thread, plus allows the kernel to lazily start up to 'num_threads'
+ /// additional threads as specified by set_thread_pool_max_thread_count.
pub fn start_thread_pool() {
unsafe {
// Safety: Safe FFI
@@ -33,8 +36,7 @@
/// Set the maximum number of threads that can be started in the threadpool.
///
/// By default, after startThreadPool is called, this is 15. If it is called
- /// additional times, it will only prevent the kernel from starting new
- /// threads and will not delete already existing threads.
+ /// additional times, the thread pool size can only be increased.
pub fn set_thread_pool_max_thread_count(num_threads: u32) {
unsafe {
// Safety: Safe FFI
@@ -43,6 +45,9 @@
}
/// Block on the Binder IPC thread pool
+ ///
+ /// This adds additional threads in addition to what is created by
+ /// set_thread_pool_max_thread_count and start_thread_pool.
pub fn join_thread_pool() {
unsafe {
// Safety: Safe FFI
diff --git a/libs/binder/rust/tests/binderRustNdkInteropTest.cpp b/libs/binder/rust/tests/binderRustNdkInteropTest.cpp
index 59ca6ed..663b9bb 100644
--- a/libs/binder/rust/tests/binderRustNdkInteropTest.cpp
+++ b/libs/binder/rust/tests/binderRustNdkInteropTest.cpp
@@ -54,14 +54,12 @@
EXPECT_EQ(STATUS_OK, AIBinder_ping(binder.get()));
auto interface = aidl::IBinderRustNdkInteropTest::fromBinder(binder);
- // TODO(b/167723746): this test requires that fromBinder allow association
- // with an already associated local binder by treating it as remote.
- EXPECT_EQ(interface, nullptr);
+ EXPECT_NE(interface, nullptr);
- // std::string in("testing");
- // std::string out;
- // EXPECT_TRUE(interface->echo(in, &out).isOk());
- // EXPECT_EQ(in, out);
+ std::string in("testing");
+ std::string out;
+ EXPECT_TRUE(interface->echo(in, &out).isOk());
+ EXPECT_EQ(in, out);
}
int main(int argc, char** argv) {
diff --git a/libs/binder/rust/tests/parcel_fuzzer/random_parcel/fuzz_service_test/Android.bp b/libs/binder/rust/tests/parcel_fuzzer/random_parcel/fuzz_service_test/Android.bp
index 89126ca..2537ce0 100644
--- a/libs/binder/rust/tests/parcel_fuzzer/random_parcel/fuzz_service_test/Android.bp
+++ b/libs/binder/rust/tests/parcel_fuzzer/random_parcel/fuzz_service_test/Android.bp
@@ -19,11 +19,6 @@
srcs: [
"service_fuzzer.rs",
],
- shared_libs: [
- "libbinder",
- "libbinder_ndk",
- "libutils",
- ],
rustlibs: [
"libbinder_rs",
"libbinder_random_parcel_rs",
diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp
index abc423b..e021af0 100644
--- a/libs/binder/tests/binderLibTest.cpp
+++ b/libs/binder/tests/binderLibTest.cpp
@@ -82,7 +82,7 @@
static constexpr int kSchedPolicy = SCHED_RR;
static constexpr int kSchedPriority = 7;
static constexpr int kSchedPriorityMore = 8;
-static constexpr int kKernelThreads = 15;
+static constexpr int kKernelThreads = 17; // anything different than the default
static String16 binderLibTestServiceName = String16("test.binderLib");
@@ -1357,17 +1357,20 @@
EXPECT_THAT(server->transact(BINDER_LIB_TEST_GET_MAX_THREAD_COUNT, data, &reply),
StatusEq(NO_ERROR));
int32_t replyi = reply.readInt32();
- // Expect 16 threads: kKernelThreads = 15 + Pool thread == 16
- EXPECT_TRUE(replyi == kKernelThreads || replyi == kKernelThreads + 1);
+ // see getThreadPoolMaxTotalThreadCount for why there is a race
+ EXPECT_TRUE(replyi == kKernelThreads + 1 || replyi == kKernelThreads + 2) << replyi;
+
EXPECT_THAT(server->transact(BINDER_LIB_TEST_PROCESS_LOCK, data, &reply), NO_ERROR);
/*
- * This will use all threads in the pool expect the main pool thread.
- * The service should run fine without locking, and the thread count should
- * not exceed 16 (15 Max + pool thread).
+ * This will use all threads in the pool but one. There are actually kKernelThreads+2
+ * available in the other process (startThreadPool, joinThreadPool, + the kernel-
+ * started threads from setThreadPoolMaxThreadCount
+ *
+ * Adding one more will cause it to deadlock.
*/
std::vector<std::thread> ts;
- for (size_t i = 0; i < kKernelThreads; i++) {
+ for (size_t i = 0; i < kKernelThreads + 1; i++) {
ts.push_back(std::thread([&] {
Parcel local_reply;
EXPECT_THAT(server->transact(BINDER_LIB_TEST_LOCK_UNLOCK, data, &local_reply),
@@ -1375,8 +1378,13 @@
}));
}
- data.writeInt32(500);
- // Give a chance for all threads to be used
+ // make sure all of the above calls will be queued in parallel. Otherwise, most of
+ // the time, the below call will pre-empt them (presumably because we have the
+ // scheduler timeslice already + scheduler hint).
+ sleep(1);
+
+ data.writeInt32(1000);
+ // Give a chance for all threads to be used (kKernelThreads + 1 thread in use)
EXPECT_THAT(server->transact(BINDER_LIB_TEST_UNLOCK_AFTER_MS, data, &reply), NO_ERROR);
for (auto &t : ts) {
@@ -1386,7 +1394,7 @@
EXPECT_THAT(server->transact(BINDER_LIB_TEST_GET_MAX_THREAD_COUNT, data, &reply),
StatusEq(NO_ERROR));
replyi = reply.readInt32();
- EXPECT_EQ(replyi, kKernelThreads + 1);
+ EXPECT_EQ(replyi, kKernelThreads + 2);
}
TEST_F(BinderLibTest, ThreadPoolStarted) {
diff --git a/libs/binder/tests/binderParcelUnitTest.cpp b/libs/binder/tests/binderParcelUnitTest.cpp
index 359c783..0a0dae0 100644
--- a/libs/binder/tests/binderParcelUnitTest.cpp
+++ b/libs/binder/tests/binderParcelUnitTest.cpp
@@ -29,6 +29,7 @@
using android::status_t;
using android::String16;
using android::String8;
+using android::base::unique_fd;
using android::binder::Status;
TEST(Parcel, NonNullTerminatedString8) {
@@ -112,6 +113,166 @@
EXPECT_EQ(ret[1], STDIN_FILENO);
}
+TEST(Parcel, AppendFromEmpty) {
+ Parcel p1;
+ Parcel p2;
+ p2.writeInt32(2);
+
+ ASSERT_EQ(OK, p1.appendFrom(&p2, 0, p2.dataSize()));
+
+ p1.setDataPosition(0);
+ ASSERT_EQ(2, p1.readInt32());
+
+ p2.setDataPosition(0);
+ ASSERT_EQ(2, p2.readInt32());
+}
+
+TEST(Parcel, AppendPlainData) {
+ Parcel p1;
+ p1.writeInt32(1);
+ Parcel p2;
+ p2.writeInt32(2);
+
+ ASSERT_EQ(OK, p1.appendFrom(&p2, 0, p2.dataSize()));
+
+ p1.setDataPosition(0);
+ ASSERT_EQ(1, p1.readInt32());
+ ASSERT_EQ(2, p1.readInt32());
+
+ p2.setDataPosition(0);
+ ASSERT_EQ(2, p2.readInt32());
+}
+
+TEST(Parcel, AppendPlainDataPartial) {
+ Parcel p1;
+ p1.writeInt32(1);
+ Parcel p2;
+ p2.writeInt32(2);
+ p2.writeInt32(3);
+ p2.writeInt32(4);
+
+ // only copy 8 bytes (two int32's worth)
+ ASSERT_EQ(OK, p1.appendFrom(&p2, 0, 8));
+
+ p1.setDataPosition(0);
+ ASSERT_EQ(1, p1.readInt32());
+ ASSERT_EQ(2, p1.readInt32());
+ ASSERT_EQ(3, p1.readInt32());
+ ASSERT_EQ(0, p1.readInt32()); // not 4, end of Parcel
+
+ p2.setDataPosition(0);
+ ASSERT_EQ(2, p2.readInt32());
+}
+
+TEST(Parcel, AppendWithBinder) {
+ sp<IBinder> b1 = sp<BBinder>::make();
+ sp<IBinder> b2 = sp<BBinder>::make();
+
+ Parcel p1;
+ p1.writeInt32(1);
+ p1.writeStrongBinder(b1);
+ Parcel p2;
+ p2.writeInt32(2);
+ p2.writeStrongBinder(b2);
+
+ ASSERT_EQ(OK, p1.appendFrom(&p2, 0, p2.dataSize()));
+
+ p1.setDataPosition(0);
+ ASSERT_EQ(1, p1.readInt32());
+ ASSERT_EQ(b1, p1.readStrongBinder());
+ ASSERT_EQ(2, p1.readInt32());
+ ASSERT_EQ(b2, p1.readStrongBinder());
+ ASSERT_EQ(2, p1.objectsCount());
+
+ p2.setDataPosition(0);
+ ASSERT_EQ(2, p2.readInt32());
+ ASSERT_EQ(b2, p2.readStrongBinder());
+}
+
+TEST(Parcel, AppendWithBinderPartial) {
+ sp<IBinder> b1 = sp<BBinder>::make();
+ sp<IBinder> b2 = sp<BBinder>::make();
+
+ Parcel p1;
+ p1.writeInt32(1);
+ p1.writeStrongBinder(b1);
+ Parcel p2;
+ p2.writeInt32(2);
+ p2.writeStrongBinder(b2);
+
+ ASSERT_EQ(OK, p1.appendFrom(&p2, 0, 8)); // BAD: 4 bytes into strong binder
+
+ p1.setDataPosition(0);
+ ASSERT_EQ(1, p1.readInt32());
+ ASSERT_EQ(b1, p1.readStrongBinder());
+ ASSERT_EQ(2, p1.readInt32());
+ ASSERT_EQ(1935813253, p1.readInt32()); // whatever garbage that is there (ABI)
+ ASSERT_EQ(1, p1.objectsCount());
+
+ p2.setDataPosition(0);
+ ASSERT_EQ(2, p2.readInt32());
+ ASSERT_EQ(b2, p2.readStrongBinder());
+}
+
+TEST(Parcel, AppendWithFd) {
+ unique_fd fd1 = unique_fd(dup(0));
+ unique_fd fd2 = unique_fd(dup(0));
+
+ Parcel p1;
+ p1.writeInt32(1);
+ p1.writeDupFileDescriptor(0); // with ownership
+ p1.writeFileDescriptor(fd1.get()); // without ownership
+ Parcel p2;
+ p2.writeInt32(2);
+ p2.writeDupFileDescriptor(0); // with ownership
+ p2.writeFileDescriptor(fd2.get()); // without ownership
+
+ ASSERT_EQ(OK, p1.appendFrom(&p2, 0, p2.dataSize()));
+
+ p1.setDataPosition(0);
+ ASSERT_EQ(1, p1.readInt32());
+ ASSERT_NE(-1, p1.readFileDescriptor());
+ ASSERT_NE(-1, p1.readFileDescriptor());
+ ASSERT_EQ(2, p1.readInt32());
+ ASSERT_NE(-1, p1.readFileDescriptor());
+ ASSERT_NE(-1, p1.readFileDescriptor());
+ ASSERT_EQ(4, p1.objectsCount());
+
+ p2.setDataPosition(0);
+ ASSERT_EQ(2, p2.readInt32());
+ ASSERT_NE(-1, p1.readFileDescriptor());
+ ASSERT_NE(-1, p1.readFileDescriptor());
+}
+
+TEST(Parcel, AppendWithFdPartial) {
+ unique_fd fd1 = unique_fd(dup(0));
+ unique_fd fd2 = unique_fd(dup(0));
+
+ Parcel p1;
+ p1.writeInt32(1);
+ p1.writeDupFileDescriptor(0); // with ownership
+ p1.writeFileDescriptor(fd1.get()); // without ownership
+ Parcel p2;
+ p2.writeInt32(2);
+ p2.writeDupFileDescriptor(0); // with ownership
+ p2.writeFileDescriptor(fd2.get()); // without ownership
+
+ ASSERT_EQ(OK, p1.appendFrom(&p2, 0, 8)); // BAD: 4 bytes into binder
+
+ p1.setDataPosition(0);
+ ASSERT_EQ(1, p1.readInt32());
+ ASSERT_NE(-1, p1.readFileDescriptor());
+ ASSERT_NE(-1, p1.readFileDescriptor());
+ ASSERT_EQ(2, p1.readInt32());
+ ASSERT_EQ(1717840517, p1.readInt32()); // whatever garbage that is there (ABI)
+ ASSERT_EQ(2, p1.objectsCount());
+
+ p2.setDataPosition(0);
+ ASSERT_EQ(2, p2.readInt32());
+ ASSERT_NE(-1, p1.readFileDescriptor());
+ ASSERT_NE(-1, p1.readFileDescriptor());
+}
+
// Tests a second operation results in a parcel at the same location as it
// started.
void parcelOpSameLength(const std::function<void(Parcel*)>& a, const std::function<void(Parcel*)>& b) {
diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp
index 1ff1de4..d352ce5 100644
--- a/libs/binder/tests/binderRpcTest.cpp
+++ b/libs/binder/tests/binderRpcTest.cpp
@@ -691,7 +691,11 @@
EXPECT_EQ(nullptr, session.promote());
- sleep(1); // give time for remote session to shutdown
+ // now that it has died, wait for the remote session to shutdown
+ std::vector<int32_t> remoteCounts;
+ do {
+ EXPECT_OK(proc.rootIface->countBinders(&remoteCounts));
+ } while (remoteCounts.size() > 1);
}
TEST_P(BinderRpc, SingleDeathRecipient) {
diff --git a/libs/nativewindow/include/android/hardware_buffer_aidl.h b/libs/nativewindow/include/android/hardware_buffer_aidl.h
index 9fea21e..e269f0d 100644
--- a/libs/nativewindow/include/android/hardware_buffer_aidl.h
+++ b/libs/nativewindow/include/android/hardware_buffer_aidl.h
@@ -34,6 +34,10 @@
#include <android/hardware_buffer.h>
#include <sys/cdefs.h>
+#ifdef __cplusplus
+#include <string>
+#endif
+
__BEGIN_DECLS
/**
@@ -119,6 +123,13 @@
inline AHardwareBuffer* _Nullable get() const { return mBuffer; }
inline explicit operator bool () const { return mBuffer != nullptr; }
+ inline bool operator!=(const HardwareBuffer& rhs) const { return get() != rhs.get(); }
+ inline bool operator<(const HardwareBuffer& rhs) const { return get() < rhs.get(); }
+ inline bool operator<=(const HardwareBuffer& rhs) const { return get() <= rhs.get(); }
+ inline bool operator==(const HardwareBuffer& rhs) const { return get() == rhs.get(); }
+ inline bool operator>(const HardwareBuffer& rhs) const { return get() > rhs.get(); }
+ inline bool operator>=(const HardwareBuffer& rhs) const { return get() >= rhs.get(); }
+
HardwareBuffer& operator=(HardwareBuffer&& other) noexcept {
reset(other.release());
return *this;
@@ -135,6 +146,15 @@
return ret;
}
+ inline std::string toString() const {
+ if (!mBuffer) {
+ return "<HardwareBuffer: Invalid>";
+ }
+ uint64_t id = 0;
+ AHardwareBuffer_getId(mBuffer, &id);
+ return "<HardwareBuffer " + std::to_string(id) + ">";
+ }
+
private:
HardwareBuffer(const HardwareBuffer& other) = delete;
HardwareBuffer& operator=(const HardwareBuffer& other) = delete;
diff --git a/services/inputflinger/dispatcher/InputDispatcher.cpp b/services/inputflinger/dispatcher/InputDispatcher.cpp
index 96164c0..9b62894 100644
--- a/services/inputflinger/dispatcher/InputDispatcher.cpp
+++ b/services/inputflinger/dispatcher/InputDispatcher.cpp
@@ -5879,9 +5879,9 @@
StringPrintf("%s does not have a focused window", application->getName().c_str());
updateLastAnrStateLocked(*application, reason);
- auto command = [this, application = std::move(application)]() REQUIRES(mLock) {
+ auto command = [this, app = std::move(application)]() REQUIRES(mLock) {
scoped_unlock unlock(mLock);
- mPolicy->notifyNoFocusedWindowAnr(application);
+ mPolicy->notifyNoFocusedWindowAnr(app);
};
postCommandLocked(std::move(command));
}
@@ -5942,9 +5942,9 @@
void InputDispatcher::sendWindowUnresponsiveCommandLocked(const sp<IBinder>& token,
std::optional<int32_t> pid,
std::string reason) {
- auto command = [this, token, pid, reason = std::move(reason)]() REQUIRES(mLock) {
+ auto command = [this, token, pid, r = std::move(reason)]() REQUIRES(mLock) {
scoped_unlock unlock(mLock);
- mPolicy->notifyWindowUnresponsive(token, pid, reason);
+ mPolicy->notifyWindowUnresponsive(token, pid, r);
};
postCommandLocked(std::move(command));
}
diff --git a/services/inputflinger/dispatcher/include/InputDispatcherPolicyInterface.h b/services/inputflinger/dispatcher/include/InputDispatcherPolicyInterface.h
index 575b3d7..fff1b03 100644
--- a/services/inputflinger/dispatcher/include/InputDispatcherPolicyInterface.h
+++ b/services/inputflinger/dispatcher/include/InputDispatcherPolicyInterface.h
@@ -26,11 +26,10 @@
namespace android {
-
/*
* Input dispatcher policy interface.
*
- * The input reader policy is used by the input reader to interact with the Window Manager
+ * The input dispatcher policy is used by the input dispatcher to interact with the Window Manager
* and other system components.
*
* The actual implementation is partially supported by callbacks into the DVM