Merge "binder: test FrameworksCoreTests_all_binder" into main
diff --git a/cmds/dumpsys/tests/dumpsys_test.cpp b/cmds/dumpsys/tests/dumpsys_test.cpp
index b8e5ce1..17ae0aa 100644
--- a/cmds/dumpsys/tests/dumpsys_test.cpp
+++ b/cmds/dumpsys/tests/dumpsys_test.cpp
@@ -67,6 +67,8 @@
MOCK_METHOD2(unregisterForNotifications, status_t(const String16&,
const sp<LocalRegistrationCallback>&));
MOCK_METHOD0(getServiceDebugInfo, std::vector<ServiceDebugInfo>());
+ MOCK_METHOD1(enableAddServiceCache, void(bool));
+
protected:
MOCK_METHOD0(onAsBinder, IBinder*());
};
diff --git a/libs/binder/Android.bp b/libs/binder/Android.bp
index 0a61178..e10e4dd 100644
--- a/libs/binder/Android.bp
+++ b/libs/binder/Android.bp
@@ -477,9 +477,34 @@
},
}
+soong_config_module_type {
+ name: "libbinder_addservice_cache_config",
+ module_type: "cc_defaults",
+ config_namespace: "libbinder",
+ bool_variables: ["release_libbinder_addservice_cache"],
+ properties: [
+ "cflags",
+ ],
+}
+
+libbinder_addservice_cache_config {
+ name: "libbinder_addservice_cache_flag",
+ soong_config_variables: {
+ release_libbinder_addservice_cache: {
+ cflags: ["-DLIBBINDER_ADDSERVICE_CACHE"],
+ conditions_default: {
+ cflags: ["-DNO_LIBBINDER_ADDSERVICE_CACHE"],
+ },
+ },
+ },
+}
+
cc_defaults {
name: "libbinder_kernel_defaults",
- defaults: ["libbinder_client_cache_flag"],
+ defaults: [
+ "libbinder_client_cache_flag",
+ "libbinder_addservice_cache_flag",
+ ],
srcs: [
"BufferedTextOutput.cpp",
"BackendUnifiedServiceManager.cpp",
diff --git a/libs/binder/BackendUnifiedServiceManager.cpp b/libs/binder/BackendUnifiedServiceManager.cpp
index d32eecd..123dfd2 100644
--- a/libs/binder/BackendUnifiedServiceManager.cpp
+++ b/libs/binder/BackendUnifiedServiceManager.cpp
@@ -30,6 +30,12 @@
constexpr bool kUseCache = false;
#endif
+#ifdef LIBBINDER_ADDSERVICE_CACHE
+constexpr bool kUseCacheInAddService = true;
+#else
+constexpr bool kUseCacheInAddService = false;
+#endif
+
using AidlServiceManager = android::os::IServiceManager;
using android::os::IAccessor;
using binder::Status;
@@ -125,31 +131,36 @@
if (!kUseCache) {
return Status::ok();
}
+
+ if (service.getTag() == os::Service::Tag::binder) {
+ return updateCache(serviceName, service.get<os::Service::Tag::binder>());
+ }
+ return Status::ok();
+}
+
+Status BackendUnifiedServiceManager::updateCache(const std::string& serviceName,
+ const sp<IBinder>& binder) {
std::string traceStr;
if (atrace_is_tag_enabled(ATRACE_TAG_AIDL)) {
traceStr = "BinderCacheWithInvalidation::updateCache : " + serviceName;
}
binder::ScopedTrace aidlTrace(ATRACE_TAG_AIDL, traceStr.c_str());
-
- if (service.getTag() == os::Service::Tag::binder) {
- sp<IBinder> binder = service.get<os::Service::Tag::binder>();
- if (!binder) {
- binder::ScopedTrace
- aidlTrace(ATRACE_TAG_AIDL,
- "BinderCacheWithInvalidation::updateCache failed: binder_null");
- } else if (!binder->isBinderAlive()) {
- binder::ScopedTrace aidlTrace(ATRACE_TAG_AIDL,
- "BinderCacheWithInvalidation::updateCache failed: "
- "isBinderAlive_false");
- } else if (mCacheForGetService->isClientSideCachingEnabled(serviceName)) {
- binder::ScopedTrace aidlTrace(ATRACE_TAG_AIDL,
- "BinderCacheWithInvalidation::updateCache successful");
- return mCacheForGetService->setItem(serviceName, binder);
- } else {
- binder::ScopedTrace aidlTrace(ATRACE_TAG_AIDL,
- "BinderCacheWithInvalidation::updateCache failed: "
- "caching_not_enabled");
- }
+ if (!binder) {
+ binder::ScopedTrace
+ aidlTrace(ATRACE_TAG_AIDL,
+ "BinderCacheWithInvalidation::updateCache failed: binder_null");
+ } else if (!binder->isBinderAlive()) {
+ binder::ScopedTrace aidlTrace(ATRACE_TAG_AIDL,
+ "BinderCacheWithInvalidation::updateCache failed: "
+ "isBinderAlive_false");
+ } else if (mCacheForGetService->isClientSideCachingEnabled(serviceName)) {
+ binder::ScopedTrace aidlTrace(ATRACE_TAG_AIDL,
+ "BinderCacheWithInvalidation::updateCache successful");
+ return mCacheForGetService->setItem(serviceName, binder);
+ } else {
+ binder::ScopedTrace aidlTrace(ATRACE_TAG_AIDL,
+ "BinderCacheWithInvalidation::updateCache failed: "
+ "caching_not_enabled");
}
return Status::ok();
}
@@ -277,7 +288,12 @@
Status BackendUnifiedServiceManager::addService(const ::std::string& name,
const sp<IBinder>& service, bool allowIsolated,
int32_t dumpPriority) {
- return mTheRealServiceManager->addService(name, service, allowIsolated, dumpPriority);
+ Status status = mTheRealServiceManager->addService(name, service, allowIsolated, dumpPriority);
+ // mEnableAddServiceCache is true by default.
+ if (kUseCacheInAddService && mEnableAddServiceCache && status.isOk()) {
+ return updateCache(name, service);
+ }
+ return status;
}
Status BackendUnifiedServiceManager::listServices(int32_t dumpPriority,
::std::vector<::std::string>* _aidl_return) {
diff --git a/libs/binder/BackendUnifiedServiceManager.h b/libs/binder/BackendUnifiedServiceManager.h
index abc0eda..b46bf19 100644
--- a/libs/binder/BackendUnifiedServiceManager.h
+++ b/libs/binder/BackendUnifiedServiceManager.h
@@ -105,8 +105,7 @@
binder::ScopedTrace aidlTrace(ATRACE_TAG_AIDL,
"BinderCacheWithInvalidation::setItem Successfully Cached");
std::lock_guard<std::mutex> lock(mCacheMutex);
- Entry entry = {.service = item, .deathRecipient = deathRecipient};
- mCache[key] = entry;
+ mCache[key] = {.service = item, .deathRecipient = deathRecipient};
return binder::Status::ok();
}
@@ -147,17 +146,20 @@
const sp<IBinder>& service) override;
binder::Status getServiceDebugInfo(::std::vector<os::ServiceDebugInfo>* _aidl_return) override;
+ void enableAddServiceCache(bool value) { mEnableAddServiceCache = value; }
// for legacy ABI
const String16& getInterfaceDescriptor() const override {
return mTheRealServiceManager->getInterfaceDescriptor();
}
private:
+ bool mEnableAddServiceCache = true;
std::shared_ptr<BinderCacheWithInvalidation> mCacheForGetService;
sp<os::IServiceManager> mTheRealServiceManager;
binder::Status toBinderService(const ::std::string& name, const os::Service& in,
os::Service* _out);
binder::Status updateCache(const std::string& serviceName, const os::Service& service);
+ binder::Status updateCache(const std::string& serviceName, const sp<IBinder>& binder);
bool returnIfCached(const std::string& serviceName, os::Service* _out);
};
diff --git a/libs/binder/IServiceManager.cpp b/libs/binder/IServiceManager.cpp
index 39d8c24..61866d7 100644
--- a/libs/binder/IServiceManager.cpp
+++ b/libs/binder/IServiceManager.cpp
@@ -127,6 +127,8 @@
}
IBinder* onAsBinder() override { return IInterface::asBinder(mUnifiedServiceManager).get(); }
+ void enableAddServiceCache(bool value) { mUnifiedServiceManager->enableAddServiceCache(value); }
+
protected:
sp<BackendUnifiedServiceManager> mUnifiedServiceManager;
// AidlRegistrationCallback -> services that its been registered for
diff --git a/libs/binder/include/binder/IServiceManager.h b/libs/binder/include/binder/IServiceManager.h
index 81f7cdb..ca26a57 100644
--- a/libs/binder/include/binder/IServiceManager.h
+++ b/libs/binder/include/binder/IServiceManager.h
@@ -150,6 +150,12 @@
int pid;
};
virtual std::vector<ServiceDebugInfo> getServiceDebugInfo() = 0;
+
+ /**
+ * Directly enable or disable caching binder during addService calls.
+ * Only used for testing.
+ */
+ virtual void enableAddServiceCache(bool value) = 0;
};
LIBBINDER_EXPORTED sp<IServiceManager> defaultServiceManager();
diff --git a/libs/binder/tests/Android.bp b/libs/binder/tests/Android.bp
index 28a3f65..642fbf3 100644
--- a/libs/binder/tests/Android.bp
+++ b/libs/binder/tests/Android.bp
@@ -70,7 +70,10 @@
static_libs: [
"libfakeservicemanager",
],
- defaults: ["libbinder_client_cache_flag"],
+ defaults: [
+ "libbinder_client_cache_flag",
+ "libbinder_addservice_cache_flag",
+ ],
test_suites: ["general-tests"],
require_root: true,
}
diff --git a/libs/binder/tests/binderCacheUnitTest.cpp b/libs/binder/tests/binderCacheUnitTest.cpp
index c5ad793..3195c55 100644
--- a/libs/binder/tests/binderCacheUnitTest.cpp
+++ b/libs/binder/tests/binderCacheUnitTest.cpp
@@ -34,6 +34,12 @@
constexpr bool kUseLibbinderCache = false;
#endif
+#ifdef LIBBINDER_ADDSERVICE_CACHE
+constexpr bool kUseCacheInAddService = true;
+#else
+constexpr bool kUseCacheInAddService = false;
+#endif
+
// A service name which is in the static list of cachable services
const String16 kCachedServiceName = String16("isub");
@@ -74,14 +80,58 @@
innerSm.addService(String16(name.c_str()), service, allowIsolated, dumpPriority));
}
+ void clearServices() { innerSm.clear(); }
+
FakeServiceManager innerSm;
};
+class LibbinderCacheAddServiceTest : public ::testing::Test {
+protected:
+ void SetUp() override {
+ fakeServiceManager = sp<MockAidlServiceManager>::make();
+ mServiceManager = getServiceManagerShimFromAidlServiceManagerForTests(fakeServiceManager);
+ mServiceManager->enableAddServiceCache(true);
+ }
+
+ void TearDown() override {}
+
+public:
+ void cacheAddServiceAndConfirmCacheHit(const sp<IBinder>& binder1) {
+ // Add a service. This also caches it.
+ EXPECT_EQ(OK, mServiceManager->addService(kCachedServiceName, binder1));
+ // remove services from fakeservicemanager
+ fakeServiceManager->clearServices();
+
+ sp<IBinder> result = mServiceManager->checkService(kCachedServiceName);
+ if (kUseCacheInAddService && kUseLibbinderCache) {
+ // If cache is enabled, we should get the binder.
+ EXPECT_EQ(binder1, result);
+ } else {
+ // If cache is disabled, then we should get the null binder
+ EXPECT_EQ(nullptr, result);
+ }
+ }
+ sp<MockAidlServiceManager> fakeServiceManager;
+ sp<android::IServiceManager> mServiceManager;
+};
+
+TEST_F(LibbinderCacheAddServiceTest, AddLocalServiceAndConfirmCacheHit) {
+ sp<IBinder> binder1 = sp<BBinder>::make();
+ cacheAddServiceAndConfirmCacheHit(binder1);
+}
+
+TEST_F(LibbinderCacheAddServiceTest, AddRemoteServiceAndConfirmCacheHit) {
+ sp<IBinder> binder1 = defaultServiceManager()->checkService(kServerName);
+ ASSERT_NE(binder1, nullptr);
+ cacheAddServiceAndConfirmCacheHit(binder1);
+}
+
class LibbinderCacheTest : public ::testing::Test {
protected:
void SetUp() override {
- sp<MockAidlServiceManager> sm = sp<MockAidlServiceManager>::make();
- mServiceManager = getServiceManagerShimFromAidlServiceManagerForTests(sm);
+ fakeServiceManager = sp<MockAidlServiceManager>::make();
+ mServiceManager = getServiceManagerShimFromAidlServiceManagerForTests(fakeServiceManager);
+ mServiceManager->enableAddServiceCache(false);
}
void TearDown() override {}
@@ -108,6 +158,7 @@
}
}
+ sp<MockAidlServiceManager> fakeServiceManager;
sp<android::IServiceManager> mServiceManager;
};
diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp
index ec2f50c..e56b7cf 100644
--- a/libs/binder/tests/binderLibTest.cpp
+++ b/libs/binder/tests/binderLibTest.cpp
@@ -220,6 +220,8 @@
ASSERT_GT(m_serverpid, 0);
sp<IServiceManager> sm = defaultServiceManager();
+ // disable caching during addService.
+ sm->enableAddServiceCache(false);
//printf("%s: pid %d, get service\n", __func__, m_pid);
LIBBINDER_IGNORE("-Wdeprecated-declarations")
m_server = sm->getService(binderLibTestServiceName);
@@ -295,6 +297,9 @@
virtual void SetUp() {
m_server = static_cast<BinderLibTestEnv *>(binder_env)->getServer();
IPCThreadState::self()->restoreCallingWorkSource(0);
+ sp<IServiceManager> sm = defaultServiceManager();
+ // disable caching during addService.
+ sm->enableAddServiceCache(false);
}
virtual void TearDown() {
}
@@ -1644,6 +1649,7 @@
TEST(ServiceNotifications, Unregister) {
auto sm = defaultServiceManager();
+ sm->enableAddServiceCache(false);
using LocalRegistrationCallback = IServiceManager::LocalRegistrationCallback;
class LocalRegistrationCallbackImpl : public virtual LocalRegistrationCallback {
void onServiceRegistration(const String16 &, const sp<IBinder> &) override {}
@@ -2374,6 +2380,8 @@
status_t ret;
sp<IServiceManager> sm = defaultServiceManager();
+ sm->enableAddServiceCache(false);
+
BinderLibTestService* testServicePtr;
{
sp<BinderLibTestService> testService = new BinderLibTestService(index);
diff --git a/libs/fakeservicemanager/include/fakeservicemanager/FakeServiceManager.h b/libs/fakeservicemanager/include/fakeservicemanager/FakeServiceManager.h
index f62241d..f2b2aa7 100644
--- a/libs/fakeservicemanager/include/fakeservicemanager/FakeServiceManager.h
+++ b/libs/fakeservicemanager/include/fakeservicemanager/FakeServiceManager.h
@@ -65,6 +65,7 @@
std::vector<IServiceManager::ServiceDebugInfo> getServiceDebugInfo() override;
+ void enableAddServiceCache(bool /*value*/) override {}
// Clear all of the registered services
void clear();
diff --git a/libs/nativewindow/rust/src/lib.rs b/libs/nativewindow/rust/src/lib.rs
index 9876362..d760285 100644
--- a/libs/nativewindow/rust/src/lib.rs
+++ b/libs/nativewindow/rust/src/lib.rs
@@ -30,8 +30,8 @@
StatusCode,
};
use ffi::{
- AHardwareBuffer, AHardwareBuffer_Desc, AHardwareBuffer_readFromParcel,
- AHardwareBuffer_writeToParcel, ARect,
+ AHardwareBuffer, AHardwareBuffer_Desc, AHardwareBuffer_Plane, AHardwareBuffer_Planes,
+ AHardwareBuffer_readFromParcel, AHardwareBuffer_writeToParcel, ARect,
};
use std::ffi::c_void;
use std::fmt::{self, Debug, Formatter};
@@ -313,6 +313,57 @@
})
}
+ /// Lock a potentially multi-planar hardware buffer for direct CPU access.
+ ///
+ /// # Safety
+ ///
+ /// - If `fence` is `None`, the caller must ensure that all writes to the buffer have completed
+ /// before calling this function.
+ /// - If the buffer has `AHARDWAREBUFFER_FORMAT_BLOB`, multiple threads or process may lock the
+ /// buffer simultaneously, but the caller must ensure that they don't access it simultaneously
+ /// and break Rust's aliasing rules, like any other shared memory.
+ /// - Otherwise if `usage` includes `AHARDWAREBUFFER_USAGE_CPU_WRITE_RARELY` or
+ /// `AHARDWAREBUFFER_USAGE_CPU_WRITE_OFTEN`, the caller must ensure that no other threads or
+ /// processes lock the buffer simultaneously for any usage.
+ /// - Otherwise, the caller must ensure that no other threads lock the buffer for writing
+ /// simultaneously.
+ /// - If `rect` is not `None`, the caller must not modify the buffer outside of that rectangle.
+ pub unsafe fn lock_planes<'a>(
+ &'a self,
+ usage: AHardwareBuffer_UsageFlags,
+ fence: Option<BorrowedFd>,
+ rect: Option<&ARect>,
+ ) -> Result<Vec<PlaneGuard<'a>>, StatusCode> {
+ let fence = if let Some(fence) = fence { fence.as_raw_fd() } else { -1 };
+ let rect = rect.map(ptr::from_ref).unwrap_or(null());
+ let mut planes = AHardwareBuffer_Planes {
+ planeCount: 0,
+ planes: [const { AHardwareBuffer_Plane { data: null_mut(), pixelStride: 0, rowStride: 0 } };
+ 4],
+ };
+
+ // SAFETY: The `AHardwareBuffer` pointer we wrap is always valid, and the various out
+ // pointers are valid because they come from references. Our caller promises that writes have
+ // completed and there will be no simultaneous read/write locks.
+ let status = unsafe {
+ ffi::AHardwareBuffer_lockPlanes(self.0.as_ptr(), usage.0, fence, rect, &mut planes)
+ };
+ status_result(status)?;
+ let plane_count = planes.planeCount.try_into().unwrap();
+ Ok(planes.planes[..plane_count]
+ .iter()
+ .map(|plane| PlaneGuard {
+ guard: HardwareBufferGuard {
+ buffer: self,
+ address: NonNull::new(plane.data)
+ .expect("AHardwareBuffer_lockAndGetInfo set a null outVirtualAddress"),
+ },
+ pixel_stride: plane.pixelStride,
+ row_stride: plane.rowStride,
+ })
+ .collect())
+ }
+
/// Locks the hardware buffer for direct CPU access, returning information about the bytes per
/// pixel and stride as well.
///
@@ -510,6 +561,18 @@
pub stride: u32,
}
+/// A guard for a single plane of a locked `HardwareBuffer`, with additional information about the
+/// stride.
+#[derive(Debug)]
+pub struct PlaneGuard<'a> {
+ /// The locked buffer guard.
+ pub guard: HardwareBufferGuard<'a>,
+ /// The stride in bytes between the color channel for one pixel to the next pixel.
+ pub pixel_stride: u32,
+ /// The stride in bytes between rows in the buffer.
+ pub row_stride: u32,
+}
+
#[cfg(test)]
mod test {
use super::*;
diff --git a/services/surfaceflinger/Scheduler/ISchedulerCallback.h b/services/surfaceflinger/Scheduler/ISchedulerCallback.h
index f430526..eae8d6d 100644
--- a/services/surfaceflinger/Scheduler/ISchedulerCallback.h
+++ b/services/surfaceflinger/Scheduler/ISchedulerCallback.h
@@ -32,7 +32,7 @@
virtual void onChoreographerAttached() = 0;
virtual void onExpectedPresentTimePosted(TimePoint, ftl::NonNull<DisplayModePtr>,
Fps renderRate) = 0;
- virtual void onCommitNotComposited(PhysicalDisplayId pacesetterDisplayId) = 0;
+ virtual void onCommitNotComposited() = 0;
virtual void vrrDisplayIdle(bool idle) = 0;
protected:
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 5ec7e48..60f11b8 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -234,7 +234,7 @@
if (FlagManager::getInstance().vrr_config()) {
compositor.sendNotifyExpectedPresentHint(pacesetterPtr->displayId);
}
- mSchedulerCallback.onCommitNotComposited(pacesetterPtr->displayId);
+ mSchedulerCallback.onCommitNotComposited();
return;
}
}
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index d4d32aa..8fa7e3a 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2580,7 +2580,7 @@
}
{
- ATRACE_NAME("LLM:commitChanges");
+ ATRACE_NAME("LayerLifecycleManager:commitChanges");
mLayerLifecycleManager.commitChanges();
}
@@ -4472,7 +4472,7 @@
scheduleNotifyExpectedPresentHint(displayId);
}
-void SurfaceFlinger::onCommitNotComposited(PhysicalDisplayId pacesetterDisplayId) {
+void SurfaceFlinger::onCommitNotComposited() {
if (FlagManager::getInstance().commit_not_composited()) {
mFrameTimeline->onCommitNotComposited();
}
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 2369043..da797f8 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -701,7 +701,7 @@
void onChoreographerAttached() override;
void onExpectedPresentTimePosted(TimePoint expectedPresentTime, ftl::NonNull<DisplayModePtr>,
Fps renderRate) override;
- void onCommitNotComposited(PhysicalDisplayId pacesetterDisplayId) override
+ void onCommitNotComposited() override
REQUIRES(kMainThreadContext);
void vrrDisplayIdle(bool idle) override;
diff --git a/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h b/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h
index 8f21cdb..0ac4b68 100644
--- a/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h
+++ b/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h
@@ -30,7 +30,7 @@
MOCK_METHOD(void, onChoreographerAttached, (), (override));
MOCK_METHOD(void, onExpectedPresentTimePosted, (TimePoint, ftl::NonNull<DisplayModePtr>, Fps),
(override));
- MOCK_METHOD(void, onCommitNotComposited, (PhysicalDisplayId), (override));
+ MOCK_METHOD(void, onCommitNotComposited, (), (override));
MOCK_METHOD(void, vrrDisplayIdle, (bool), (override));
};
@@ -41,7 +41,7 @@
void triggerOnFrameRateOverridesChanged() override {}
void onChoreographerAttached() override {}
void onExpectedPresentTimePosted(TimePoint, ftl::NonNull<DisplayModePtr>, Fps) override {}
- void onCommitNotComposited(PhysicalDisplayId) override {}
+ void onCommitNotComposited() override {}
void vrrDisplayIdle(bool) override {}
};