Merge changes I01dbdd8a,Ib78b3c35 into udc-dev am: 687f69993f am: 5fca7e97b4 am: 1f3d8d1015
Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/native/+/23148598
Change-Id: Ib75c8268e8b8a96d16146f4e3b009c1db7307437
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
diff --git a/cmds/servicemanager/ServiceManager.cpp b/cmds/servicemanager/ServiceManager.cpp
index 63f3821..e30cbd5 100644
--- a/cmds/servicemanager/ServiceManager.cpp
+++ b/cmds/servicemanager/ServiceManager.cpp
@@ -301,7 +301,7 @@
}
if (!out && startIfNotFound) {
- tryStartService(name);
+ tryStartService(ctx, name);
}
if (out) {
@@ -372,8 +372,10 @@
}
auto it = mNameToService.find(name);
+ bool prevClients = false;
if (it != mNameToService.end()) {
const Service& existing = it->second;
+ prevClients = existing.hasClients;
// We could do better than this because if the other service dies, it
// may not have an entry here. However, this case is unlikely. We are
@@ -401,10 +403,13 @@
.binder = binder,
.allowIsolated = allowIsolated,
.dumpPriority = dumpPriority,
+ .hasClients = prevClients, // see b/279898063, matters if existing callbacks
+ .guaranteeClient = false, // handled below
.ctx = ctx,
};
if (auto it = mNameToRegistrationCallback.find(name); it != mNameToRegistrationCallback.end()) {
+ // TODO: this is only needed once
// See also getService - handles case where client never gets the service,
// we want the service to quit.
mNameToService[name].guaranteeClient = true;
@@ -633,6 +638,14 @@
void ServiceManager::binderDied(const wp<IBinder>& who) {
for (auto it = mNameToService.begin(); it != mNameToService.end();) {
if (who == it->second.binder) {
+ // TODO: currently, this entry contains the state also
+ // associated with mNameToClientCallback. If we allowed
+ // other processes to register client callbacks, we
+ // would have to preserve hasClients (perhaps moving
+ // that state into mNameToClientCallback, which is complicated
+ // because those callbacks are associated w/ particular binder
+ // objects, though they are indexed by name now, they may
+ // need to be indexed by binder at that point).
it = mNameToService.erase(it);
} else {
++it;
@@ -648,10 +661,11 @@
}
}
-void ServiceManager::tryStartService(const std::string& name) {
- ALOGI("Since '%s' could not be found, trying to start it as a lazy AIDL service. (if it's not "
- "configured to be a lazy service, it may be stuck starting or still starting).",
- name.c_str());
+void ServiceManager::tryStartService(const Access::CallingContext& ctx, const std::string& name) {
+ ALOGI("Since '%s' could not be found (requested by debug pid %d), trying to start it as a lazy "
+ "AIDL service. (if it's not configured to be a lazy service, it may be stuck starting or "
+ "still starting).",
+ name.c_str(), ctx.debugPid);
std::thread([=] {
if (!base::SetProperty("ctl.interface_start", "aidl/" + name)) {
@@ -700,7 +714,10 @@
return Status::fromExceptionCode(Status::EX_ILLEGAL_STATE, "Couldn't linkToDeath.");
}
- // make sure all callbacks have been told about a consistent state - b/278038751
+ // WARNING: binderDied makes an assumption about this. If we open up client
+ // callbacks to other services, certain race conditions may lead to services
+ // getting extra client callback notifications.
+ // Make sure all callbacks have been told about a consistent state - b/278038751
if (serviceIt->second.hasClients) {
cb->onClients(service, true);
}
diff --git a/cmds/servicemanager/ServiceManager.h b/cmds/servicemanager/ServiceManager.h
index 3aa6731..3b925a4 100644
--- a/cmds/servicemanager/ServiceManager.h
+++ b/cmds/servicemanager/ServiceManager.h
@@ -67,7 +67,7 @@
void clear();
protected:
- virtual void tryStartService(const std::string& name);
+ virtual void tryStartService(const Access::CallingContext& ctx, const std::string& name);
private:
struct Service {
diff --git a/cmds/servicemanager/servicemanager.recovery.rc b/cmds/servicemanager/servicemanager.recovery.rc
index b927c01..6354fd7 100644
--- a/cmds/servicemanager/servicemanager.recovery.rc
+++ b/cmds/servicemanager/servicemanager.recovery.rc
@@ -1,5 +1,6 @@
service servicemanager /system/bin/servicemanager
disabled
group system readproc
+ user root
onrestart setprop servicemanager.ready false
seclabel u:r:servicemanager:s0
diff --git a/cmds/servicemanager/test_sm.cpp b/cmds/servicemanager/test_sm.cpp
index cae32e3..97e500d 100644
--- a/cmds/servicemanager/test_sm.cpp
+++ b/cmds/servicemanager/test_sm.cpp
@@ -27,11 +27,14 @@
#include "Access.h"
#include "ServiceManager.h"
-using android::sp;
using android::Access;
using android::BBinder;
using android::IBinder;
using android::ServiceManager;
+using android::sp;
+using android::base::EndsWith;
+using android::base::GetProperty;
+using android::base::StartsWith;
using android::binder::Status;
using android::os::BnServiceCallback;
using android::os::IServiceManager;
@@ -62,7 +65,7 @@
class MockServiceManager : public ServiceManager {
public:
MockServiceManager(std::unique_ptr<Access>&& access) : ServiceManager(std::move(access)) {}
- MOCK_METHOD1(tryStartService, void(const std::string& name));
+ MOCK_METHOD2(tryStartService, void(const Access::CallingContext&, const std::string& name));
};
static sp<ServiceManager> getPermissiveServiceManager() {
@@ -77,9 +80,11 @@
return sm;
}
-static bool isCuttlefish() {
- return android::base::StartsWith(android::base::GetProperty("ro.product.vendor.device", ""),
- "vsoc_");
+// Determines if test device is a cuttlefish phone device
+static bool isCuttlefishPhone() {
+ auto device = GetProperty("ro.product.vendor.device", "");
+ auto product = GetProperty("ro.product.vendor.name", "");
+ return StartsWith(device, "vsoc_") && EndsWith(product, "_phone");
}
TEST(AddService, HappyHappy) {
@@ -314,7 +319,7 @@
}
TEST(Vintf, UpdatableViaApex) {
- if (!isCuttlefish()) GTEST_SKIP() << "Skipping non-Cuttlefish devices";
+ if (!isCuttlefishPhone()) GTEST_SKIP() << "Skipping non-Cuttlefish-phone devices";
auto sm = getPermissiveServiceManager();
std::optional<std::string> updatableViaApex;
@@ -326,7 +331,7 @@
}
TEST(Vintf, UpdatableViaApex_InvalidNameReturnsNullOpt) {
- if (!isCuttlefish()) GTEST_SKIP() << "Skipping non-Cuttlefish devices";
+ if (!isCuttlefishPhone()) GTEST_SKIP() << "Skipping non-Cuttlefish-phone devices";
auto sm = getPermissiveServiceManager();
std::optional<std::string> updatableViaApex;
@@ -337,7 +342,7 @@
}
TEST(Vintf, GetUpdatableNames) {
- if (!isCuttlefish()) GTEST_SKIP() << "Skipping non-Cuttlefish devices";
+ if (!isCuttlefishPhone()) GTEST_SKIP() << "Skipping non-Cuttlefish-phone devices";
auto sm = getPermissiveServiceManager();
std::vector<std::string> names;
@@ -348,7 +353,7 @@
}
TEST(Vintf, GetUpdatableNames_InvalidApexNameReturnsEmpty) {
- if (!isCuttlefish()) GTEST_SKIP() << "Skipping non-Cuttlefish devices";
+ if (!isCuttlefishPhone()) GTEST_SKIP() << "Skipping non-Cuttlefish-phone devices";
auto sm = getPermissiveServiceManager();
std::vector<std::string> names;
diff --git a/libs/binder/Android.bp b/libs/binder/Android.bp
index 49dd9c7..1dff38c 100644
--- a/libs/binder/Android.bp
+++ b/libs/binder/Android.bp
@@ -144,10 +144,6 @@
"-DANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION",
],
product_variables: {
- binder32bit: {
- cflags: ["-DBINDER_IPC_32BIT=1"],
- },
-
debuggable: {
cflags: [
"-DBINDER_RPC_DEV_SERVERS",
@@ -285,14 +281,6 @@
cflags: [
"-DBINDER_WITH_KERNEL_IPC",
],
- arch: {
- // TODO(b/254713216): undefined symbol in BufferedTextOutput::getBuffer
- riscv64: {
- lto: {
- thin: false,
- },
- },
- },
}
cc_library {
@@ -548,6 +536,7 @@
":__subpackages__",
"//packages/modules/Virtualization/javalib/jni",
"//packages/modules/Virtualization/vm_payload",
+ "//packages/modules/Virtualization/demo_native",
"//device/google/cuttlefish/shared/minidroid:__subpackages__",
"//system/software_defined_vehicle:__subpackages__",
],
diff --git a/libs/binder/LazyServiceRegistrar.cpp b/libs/binder/LazyServiceRegistrar.cpp
index f66993f..7644806 100644
--- a/libs/binder/LazyServiceRegistrar.cpp
+++ b/libs/binder/LazyServiceRegistrar.cpp
@@ -324,6 +324,10 @@
return *registrarInstance;
}
+LazyServiceRegistrar LazyServiceRegistrar::createExtraTestInstance() {
+ return LazyServiceRegistrar();
+}
+
status_t LazyServiceRegistrar::registerService(const sp<IBinder>& service, const std::string& name,
bool allowIsolated, int dumpFlags) {
if (!mClientCC->registerService(service, name, allowIsolated, dumpFlags)) {
diff --git a/libs/binder/ProcessState.cpp b/libs/binder/ProcessState.cpp
index 5f1f506..3fa6867 100644
--- a/libs/binder/ProcessState.cpp
+++ b/libs/binder/ProcessState.cpp
@@ -104,14 +104,7 @@
return access("/vendor/bin/vndservicemanager", R_OK) == 0;
}
-sp<ProcessState> ProcessState::init(const char *driver, bool requireDefault)
-{
-#ifdef BINDER_IPC_32BIT
- LOG_ALWAYS_FATAL("32-bit binder IPC is not supported for new devices starting in Android P. If "
- "you do need to use this mode, please see b/232423610 or file an issue with "
- "AOSP upstream as otherwise this will be removed soon.");
-#endif
-
+sp<ProcessState> ProcessState::init(const char* driver, bool requireDefault) {
if (driver == nullptr) {
std::lock_guard<std::mutex> l(gProcessMutex);
if (gProcess) {
diff --git a/libs/binder/RecordedTransaction.cpp b/libs/binder/RecordedTransaction.cpp
index 1c76135..44a9e3b 100644
--- a/libs/binder/RecordedTransaction.cpp
+++ b/libs/binder/RecordedTransaction.cpp
@@ -161,17 +161,6 @@
constexpr uint32_t kMaxChunkDataSize = 0xfffffff0;
typedef uint64_t transaction_checksum_t;
-static android::status_t readChunkDescriptor(borrowed_fd fd, ChunkDescriptor* chunkOut,
- transaction_checksum_t* sum) {
- if (!android::base::ReadFully(fd, chunkOut, sizeof(ChunkDescriptor))) {
- LOG(ERROR) << "Failed to read Chunk Descriptor from fd " << fd.get();
- return android::UNKNOWN_ERROR;
- }
-
- *sum ^= *reinterpret_cast<transaction_checksum_t*>(chunkOut);
- return android::NO_ERROR;
-}
-
std::optional<RecordedTransaction> RecordedTransaction::fromFile(const unique_fd& fd) {
RecordedTransaction t;
ChunkDescriptor chunk;
@@ -192,11 +181,13 @@
LOG(ERROR) << "Not enough file remains to contain expected chunk descriptor";
return std::nullopt;
}
- transaction_checksum_t checksum = 0;
- if (NO_ERROR != readChunkDescriptor(fd, &chunk, &checksum)) {
- LOG(ERROR) << "Failed to read chunk descriptor.";
+
+ if (!android::base::ReadFully(fd, &chunk, sizeof(ChunkDescriptor))) {
+ LOG(ERROR) << "Failed to read ChunkDescriptor from fd " << fd.get() << ". "
+ << strerror(errno);
return std::nullopt;
}
+ transaction_checksum_t checksum = *reinterpret_cast<transaction_checksum_t*>(&chunk);
fdCurrentPosition = lseek(fd.get(), 0, SEEK_CUR);
if (fdCurrentPosition == -1) {
diff --git a/libs/binder/RpcServer.cpp b/libs/binder/RpcServer.cpp
index 9282856..bd85c77 100644
--- a/libs/binder/RpcServer.cpp
+++ b/libs/binder/RpcServer.cpp
@@ -81,6 +81,7 @@
auto aiStart = InetSocketAddress::getAddrInfo(address, port);
if (aiStart == nullptr) return UNKNOWN_ERROR;
for (auto ai = aiStart.get(); ai != nullptr; ai = ai->ai_next) {
+ if (ai->ai_addr == nullptr) continue;
InetSocketAddress socketAddress(ai->ai_addr, ai->ai_addrlen, address, port);
if (status_t status = setupSocketServer(socketAddress); status != OK) {
continue;
@@ -161,6 +162,12 @@
mConnectionFilter = std::move(filter);
}
+void RpcServer::setServerSocketModifier(std::function<void(base::borrowed_fd)>&& modifier) {
+ RpcMutexLockGuard _l(mLock);
+ LOG_ALWAYS_FATAL_IF(mServer.fd != -1, "Already started");
+ mServerSocketModifier = std::move(modifier);
+}
+
sp<IBinder> RpcServer::getRootObject() {
RpcMutexLockGuard _l(mLock);
bool hasWeak = mRootObjectWeak.unsafe_get();
@@ -335,6 +342,8 @@
mJoinThread.reset();
}
+ mServer = RpcTransportFd();
+
LOG_RPC_DETAIL("Finished waiting on shutdown.");
mShutdownTrigger = nullptr;
@@ -556,6 +565,14 @@
ALOGE("Could not create socket at %s: %s", addr.toString().c_str(), strerror(savedErrno));
return -savedErrno;
}
+
+ {
+ RpcMutexLockGuard _l(mLock);
+ if (mServerSocketModifier != nullptr) {
+ mServerSocketModifier(socket_fd);
+ }
+ }
+
if (0 != TEMP_FAILURE_RETRY(bind(socket_fd.get(), addr.addr(), addr.addrSize()))) {
int savedErrno = errno;
ALOGE("Could not bind socket at %s: %s", addr.toString().c_str(), strerror(savedErrno));
diff --git a/libs/binder/TEST_MAPPING b/libs/binder/TEST_MAPPING
index 0e8e187..199574e 100644
--- a/libs/binder/TEST_MAPPING
+++ b/libs/binder/TEST_MAPPING
@@ -16,9 +16,15 @@
"name": "binderDriverInterfaceTest"
},
{
+ "name": "binderRecordReplayTest"
+ },
+ {
"name": "binderHostDeviceTest"
},
{
+ "name": "binderParcelBenchmark"
+ },
+ {
"name": "binderTextOutputTest"
},
{
diff --git a/libs/binder/include/binder/Binder.h b/libs/binder/include/binder/Binder.h
index d960a0b..744da0f 100644
--- a/libs/binder/include/binder/Binder.h
+++ b/libs/binder/include/binder/Binder.h
@@ -105,12 +105,6 @@
[[nodiscard]] status_t setRpcClientDebug(android::base::unique_fd clientFd,
const sp<IBinder>& keepAliveBinder);
- // Start recording transactions to the unique_fd in data.
- // See RecordedTransaction.h for more details.
- [[nodiscard]] status_t startRecordingTransactions(const Parcel& data);
- // Stop the current recording.
- [[nodiscard]] status_t stopRecordingTransactions();
-
protected:
virtual ~BBinder();
@@ -131,6 +125,8 @@
[[nodiscard]] status_t setRpcClientDebug(const Parcel& data);
void removeRpcServerLink(const sp<RpcServerLink>& link);
+ [[nodiscard]] status_t startRecordingTransactions(const Parcel& data);
+ [[nodiscard]] status_t stopRecordingTransactions();
std::atomic<Extras*> mExtras;
diff --git a/libs/binder/include/binder/LazyServiceRegistrar.h b/libs/binder/include/binder/LazyServiceRegistrar.h
index 2e22b84..bda3d19 100644
--- a/libs/binder/include/binder/LazyServiceRegistrar.h
+++ b/libs/binder/include/binder/LazyServiceRegistrar.h
@@ -93,7 +93,17 @@
*/
void reRegister();
- private:
+ /**
+ * Create a second instance of lazy service registrar.
+ *
+ * WARNING: dangerous! DO NOT USE THIS - LazyServiceRegistrar
+ * should be single-instanced, so that the service will only
+ * shut down when all services are unused. A separate instance
+ * is only used to test race conditions.
+ */
+ static LazyServiceRegistrar createExtraTestInstance();
+
+ private:
std::shared_ptr<internal::ClientCounterCallback> mClientCC;
LazyServiceRegistrar();
};
diff --git a/libs/binder/include/binder/Parcel.h b/libs/binder/include/binder/Parcel.h
index 162cd40..e28d374 100644
--- a/libs/binder/include/binder/Parcel.h
+++ b/libs/binder/include/binder/Parcel.h
@@ -34,13 +34,8 @@
#include <binder/IInterface.h>
#include <binder/Parcelable.h>
-#ifdef BINDER_IPC_32BIT
-//NOLINTNEXTLINE(google-runtime-int) b/173188702
-typedef unsigned int binder_size_t;
-#else
//NOLINTNEXTLINE(google-runtime-int) b/173188702
typedef unsigned long long binder_size_t;
-#endif
struct flat_binder_object;
diff --git a/libs/binder/include/binder/RpcServer.h b/libs/binder/include/binder/RpcServer.h
index 1001b64..4816395 100644
--- a/libs/binder/include/binder/RpcServer.h
+++ b/libs/binder/include/binder/RpcServer.h
@@ -184,6 +184,13 @@
void setConnectionFilter(std::function<bool(const void*, size_t)>&& filter);
/**
+ * Set optional modifier of each newly created server socket.
+ *
+ * The only argument is a successfully created file descriptor, not bound to an address yet.
+ */
+ void setServerSocketModifier(std::function<void(base::borrowed_fd)>&& modifier);
+
+ /**
* See RpcTransportCtx::getCertificate
*/
std::vector<uint8_t> getCertificate(RpcCertificateFormat);
@@ -267,6 +274,7 @@
wp<IBinder> mRootObjectWeak;
std::function<sp<IBinder>(const void*, size_t)> mRootObjectFactory;
std::function<bool(const void*, size_t)> mConnectionFilter;
+ std::function<void(base::borrowed_fd)> mServerSocketModifier;
std::map<std::vector<uint8_t>, sp<RpcSession>> mSessions;
std::unique_ptr<FdTrigger> mShutdownTrigger;
RpcConditionVariable mShutdownCv;
diff --git a/libs/binder/rust/src/binder.rs b/libs/binder/rust/src/binder.rs
index d0e35de..b90b40b 100644
--- a/libs/binder/rust/src/binder.rs
+++ b/libs/binder/rust/src/binder.rs
@@ -1122,6 +1122,10 @@
}
impl $crate::binder_impl::Deserialize for $enum {
+ type UninitType = Self;
+ fn uninit() -> Self::UninitType { Self::UninitType::default() }
+ fn from_init(value: Self) -> Self::UninitType { value }
+
fn deserialize(parcel: &$crate::binder_impl::BorrowedParcel<'_>) -> std::result::Result<Self, $crate::StatusCode> {
parcel.read().map(Self)
}
diff --git a/libs/binder/rust/src/error.rs b/libs/binder/rust/src/error.rs
index f6b09ed..ba26062 100644
--- a/libs/binder/rust/src/error.rs
+++ b/libs/binder/rust/src/error.rs
@@ -20,6 +20,7 @@
use std::error;
use std::ffi::{CStr, CString};
use std::fmt::{Debug, Display, Formatter, Result as FmtResult};
+use std::ptr;
use std::result;
pub use sys::binder_status_t as status_t;
@@ -92,7 +93,7 @@
/// track of and chain binder errors along with service specific errors.
///
/// Used in AIDL transactions to represent failed transactions.
-pub struct Status(*mut sys::AStatus);
+pub struct Status(ptr::NonNull<sys::AStatus>);
// Safety: The `AStatus` that the `Status` points to must have an entirely thread-safe API for the
// duration of the `Status` object's lifetime. We ensure this by not allowing mutation of a `Status`
@@ -119,7 +120,7 @@
// Rust takes ownership of the returned pointer.
sys::AStatus_newOk()
};
- Self(ptr)
+ Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer"))
}
/// Create a status object from a service specific error
@@ -147,7 +148,7 @@
sys::AStatus_fromServiceSpecificError(err)
}
};
- Self(ptr)
+ Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer"))
}
/// Creates a status object from a service specific error.
@@ -161,7 +162,7 @@
let ptr = unsafe {
sys::AStatus_fromExceptionCodeWithMessage(exception as i32, message.as_ptr())
};
- Self(ptr)
+ Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer"))
} else {
exception.into()
}
@@ -181,7 +182,7 @@
///
/// This constructor is safe iff `ptr` is a valid pointer to an `AStatus`.
pub(crate) unsafe fn from_ptr(ptr: *mut sys::AStatus) -> Self {
- Self(ptr)
+ Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer"))
}
/// Returns `true` if this status represents a successful transaction.
@@ -326,7 +327,7 @@
// UNKNOWN_ERROR.
sys::AStatus_fromStatus(status)
};
- Self(ptr)
+ Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer"))
}
}
@@ -338,7 +339,7 @@
// Unknown values will be coerced into EX_TRANSACTION_FAILED.
sys::AStatus_fromExceptionCode(code as i32)
};
- Self(ptr)
+ Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer"))
}
}
@@ -367,7 +368,7 @@
// pointee, so we need to delete it here. We know that the pointer
// will be valid here since `Status` always contains a valid pointer
// while it is alive.
- sys::AStatus_delete(self.0);
+ sys::AStatus_delete(self.0.as_mut());
}
}
}
@@ -381,11 +382,15 @@
/// `Status` object is still alive.
unsafe impl AsNative<sys::AStatus> for Status {
fn as_native(&self) -> *const sys::AStatus {
- self.0
+ self.0.as_ptr()
}
fn as_native_mut(&mut self) -> *mut sys::AStatus {
- self.0
+ unsafe {
+ // Safety: The pointer will be valid here since `Status` always
+ // contains a valid and initialized pointer while it is alive.
+ self.0.as_mut()
+ }
}
}
diff --git a/libs/binder/rust/src/parcel/file_descriptor.rs b/libs/binder/rust/src/parcel/file_descriptor.rs
index de6d649..7fe37f3 100644
--- a/libs/binder/rust/src/parcel/file_descriptor.rs
+++ b/libs/binder/rust/src/parcel/file_descriptor.rs
@@ -132,6 +132,14 @@
}
impl Deserialize for ParcelFileDescriptor {
+ type UninitType = Option<Self>;
+ fn uninit() -> Self::UninitType {
+ Self::UninitType::default()
+ }
+ fn from_init(value: Self) -> Self::UninitType {
+ Some(value)
+ }
+
fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> {
Deserialize::deserialize(parcel).transpose().unwrap_or(Err(StatusCode::UNEXPECTED_NULL))
}
diff --git a/libs/binder/rust/src/parcel/parcelable.rs b/libs/binder/rust/src/parcel/parcelable.rs
index 4b658fc..5d8c11c 100644
--- a/libs/binder/rust/src/parcel/parcelable.rs
+++ b/libs/binder/rust/src/parcel/parcelable.rs
@@ -14,7 +14,7 @@
* limitations under the License.
*/
-use crate::binder::{AsNative, FromIBinder, Stability, Strong};
+use crate::binder::{AsNative, FromIBinder, Interface, Stability, Strong};
use crate::error::{status_result, status_t, Result, Status, StatusCode};
use crate::parcel::BorrowedParcel;
use crate::proxy::SpIBinder;
@@ -22,7 +22,7 @@
use std::convert::{TryFrom, TryInto};
use std::ffi::c_void;
-use std::mem::{self, ManuallyDrop, MaybeUninit};
+use std::mem::{self, ManuallyDrop};
use std::os::raw::c_char;
use std::ptr;
use std::slice;
@@ -60,6 +60,26 @@
/// A struct whose instances can be restored from a [`Parcel`].
// Might be able to hook this up as a serde backend in the future?
pub trait Deserialize: Sized {
+ /// Type for the uninitialized value of this type. Will be either `Self`
+ /// if the type implements `Default`, `Option<Self>` otherwise.
+ type UninitType;
+
+ /// Assert at compile-time that `Self` and `Self::UninitType` have the same
+ /// size and alignment. This will either fail to compile or evaluate to `true`.
+ /// The only two macros that work here are `panic!` and `assert!`, so we cannot
+ /// use `assert_eq!`.
+ const ASSERT_UNINIT_SIZE_AND_ALIGNMENT: bool = {
+ assert!(std::mem::size_of::<Self>() == std::mem::size_of::<Self::UninitType>());
+ assert!(std::mem::align_of::<Self>() == std::mem::align_of::<Self::UninitType>());
+ true
+ };
+
+ /// Return an uninitialized or default-initialized value for this type.
+ fn uninit() -> Self::UninitType;
+
+ /// Convert an initialized value of type `Self` into `Self::UninitType`.
+ fn from_init(value: Self) -> Self::UninitType;
+
/// Deserialize an instance from the given [`Parcel`].
fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self>;
@@ -121,7 +141,7 @@
pub trait DeserializeArray: Deserialize {
/// Deserialize an array of type from the given parcel.
fn deserialize_array(parcel: &BorrowedParcel<'_>) -> Result<Option<Vec<Self>>> {
- let mut vec: Option<Vec<MaybeUninit<Self>>> = None;
+ let mut vec: Option<Vec<Self::UninitType>> = None;
let res = unsafe {
// Safety: Safe FFI, vec is the correct opaque type expected by
// allocate_vec and deserialize_element.
@@ -136,8 +156,8 @@
let vec: Option<Vec<Self>> = unsafe {
// Safety: We are assuming that the NDK correctly initialized every
// element of the vector by now, so we know that all the
- // MaybeUninits are now properly initialized. We can transmute from
- // Vec<MaybeUninit<T>> to Vec<T> because MaybeUninit<T> has the same
+ // UninitTypes are now properly initialized. We can transmute from
+ // Vec<T::UninitType> to Vec<T> because T::UninitType has the same
// alignment and size as T, so the pointer to the vector allocation
// will be compatible.
mem::transmute(vec)
@@ -149,14 +169,14 @@
/// Callback to deserialize a parcelable element.
///
/// The opaque array data pointer must be a mutable pointer to an
-/// `Option<Vec<MaybeUninit<T>>>` with at least enough elements for `index` to be valid
+/// `Option<Vec<T::UninitType>>` with at least enough elements for `index` to be valid
/// (zero-based).
unsafe extern "C" fn deserialize_element<T: Deserialize>(
parcel: *const sys::AParcel,
array: *mut c_void,
index: usize,
) -> status_t {
- let vec = &mut *(array as *mut Option<Vec<MaybeUninit<T>>>);
+ let vec = &mut *(array as *mut Option<Vec<T::UninitType>>);
let vec = match vec {
Some(v) => v,
None => return StatusCode::BAD_INDEX as status_t,
@@ -170,7 +190,7 @@
Ok(e) => e,
Err(code) => return code as status_t,
};
- ptr::write(vec[index].as_mut_ptr(), element);
+ vec[index] = T::from_init(element);
StatusCode::OK as status_t
}
@@ -233,15 +253,15 @@
/// # Safety
///
/// The opaque data pointer passed to the array read function must be a mutable
-/// pointer to an `Option<Vec<MaybeUninit<T>>>`. `buffer` will be assigned a mutable pointer
+/// pointer to an `Option<Vec<T::UninitType>>`. `buffer` will be assigned a mutable pointer
/// to the allocated vector data if this function returns true.
-unsafe extern "C" fn allocate_vec_with_buffer<T>(
+unsafe extern "C" fn allocate_vec_with_buffer<T: Deserialize>(
data: *mut c_void,
len: i32,
buffer: *mut *mut T,
) -> bool {
let res = allocate_vec::<T>(data, len);
- let vec = &mut *(data as *mut Option<Vec<MaybeUninit<T>>>);
+ let vec = &mut *(data as *mut Option<Vec<T::UninitType>>);
if let Some(new_vec) = vec {
*buffer = new_vec.as_mut_ptr() as *mut T;
}
@@ -253,20 +273,18 @@
/// # Safety
///
/// The opaque data pointer passed to the array read function must be a mutable
-/// pointer to an `Option<Vec<MaybeUninit<T>>>`.
-unsafe extern "C" fn allocate_vec<T>(data: *mut c_void, len: i32) -> bool {
- let vec = &mut *(data as *mut Option<Vec<MaybeUninit<T>>>);
+/// pointer to an `Option<Vec<T::UninitType>>`.
+unsafe extern "C" fn allocate_vec<T: Deserialize>(data: *mut c_void, len: i32) -> bool {
+ let vec = &mut *(data as *mut Option<Vec<T::UninitType>>);
if len < 0 {
*vec = None;
return true;
}
- let mut new_vec: Vec<MaybeUninit<T>> = Vec::with_capacity(len as usize);
- // Safety: We are filling the vector with uninitialized data here, but this
- // is safe because the vector contains MaybeUninit elements which can be
- // uninitialized. We're putting off the actual unsafe bit, transmuting the
- // vector to a Vec<T> until the contents are initialized.
- new_vec.set_len(len as usize);
+ // Assert at compile time that `T` and `T::UninitType` have the same size and alignment.
+ let _ = T::ASSERT_UNINIT_SIZE_AND_ALIGNMENT;
+ let mut new_vec: Vec<T::UninitType> = Vec::with_capacity(len as usize);
+ new_vec.resize_with(len as usize, T::uninit);
ptr::write(vec, Some(new_vec));
true
@@ -283,8 +301,11 @@
}
/// Safety: All elements in the vector must be properly initialized.
-unsafe fn vec_assume_init<T>(vec: Vec<MaybeUninit<T>>) -> Vec<T> {
- // We can convert from Vec<MaybeUninit<T>> to Vec<T> because MaybeUninit<T>
+unsafe fn vec_assume_init<T: Deserialize>(vec: Vec<T::UninitType>) -> Vec<T> {
+ // Assert at compile time that `T` and `T::UninitType` have the same size and alignment.
+ let _ = T::ASSERT_UNINIT_SIZE_AND_ALIGNMENT;
+
+ // We can convert from Vec<T::UninitType> to Vec<T> because T::UninitType
// has the same alignment and size as T, so the pointer to the vector
// allocation will be compatible.
let mut vec = ManuallyDrop::new(vec);
@@ -307,6 +328,9 @@
{Deserialize, $ty:ty, $read_fn:path} => {
impl Deserialize for $ty {
+ type UninitType = Self;
+ fn uninit() -> Self::UninitType { Self::UninitType::default() }
+ fn from_init(value: Self) -> Self::UninitType { value }
fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> {
let mut val = Self::default();
unsafe {
@@ -348,11 +372,11 @@
{DeserializeArray, $ty:ty, $read_array_fn:path} => {
impl DeserializeArray for $ty {
fn deserialize_array(parcel: &BorrowedParcel<'_>) -> Result<Option<Vec<Self>>> {
- let mut vec: Option<Vec<MaybeUninit<Self>>> = None;
+ let mut vec: Option<Vec<Self::UninitType>> = None;
let status = unsafe {
// Safety: `Parcel` always contains a valid pointer to an
// `AParcel`. `allocate_vec<T>` expects the opaque pointer to
- // be of type `*mut Option<Vec<MaybeUninit<T>>>`, so `&mut vec` is
+ // be of type `*mut Option<Vec<T::UninitType>>`, so `&mut vec` is
// correct for it.
$read_array_fn(
parcel.as_native(),
@@ -364,7 +388,7 @@
let vec: Option<Vec<Self>> = unsafe {
// Safety: We are assuming that the NDK correctly
// initialized every element of the vector by now, so we
- // know that all the MaybeUninits are now properly
+ // know that all the UninitTypes are now properly
// initialized.
vec.map(|vec| vec_assume_init(vec))
};
@@ -440,6 +464,14 @@
}
impl Deserialize for u8 {
+ type UninitType = Self;
+ fn uninit() -> Self::UninitType {
+ Self::UninitType::default()
+ }
+ fn from_init(value: Self) -> Self::UninitType {
+ value
+ }
+
fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> {
i8::deserialize(parcel).map(|v| v as u8)
}
@@ -471,6 +503,14 @@
}
impl Deserialize for i16 {
+ type UninitType = Self;
+ fn uninit() -> Self::UninitType {
+ Self::UninitType::default()
+ }
+ fn from_init(value: Self) -> Self::UninitType {
+ value
+ }
+
fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> {
u16::deserialize(parcel).map(|v| v as i16)
}
@@ -547,6 +587,14 @@
}
impl Deserialize for Option<String> {
+ type UninitType = Self;
+ fn uninit() -> Self::UninitType {
+ Self::UninitType::default()
+ }
+ fn from_init(value: Self) -> Self::UninitType {
+ value
+ }
+
fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> {
let mut vec: Option<Vec<u8>> = None;
let status = unsafe {
@@ -575,6 +623,14 @@
impl DeserializeArray for Option<String> {}
impl Deserialize for String {
+ type UninitType = Self;
+ fn uninit() -> Self::UninitType {
+ Self::UninitType::default()
+ }
+ fn from_init(value: Self) -> Self::UninitType {
+ value
+ }
+
fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> {
Deserialize::deserialize(parcel).transpose().unwrap_or(Err(StatusCode::UNEXPECTED_NULL))
}
@@ -611,6 +667,14 @@
}
impl<T: DeserializeArray> Deserialize for Vec<T> {
+ type UninitType = Self;
+ fn uninit() -> Self::UninitType {
+ Self::UninitType::default()
+ }
+ fn from_init(value: Self) -> Self::UninitType {
+ value
+ }
+
fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> {
DeserializeArray::deserialize_array(parcel)
.transpose()
@@ -640,6 +704,14 @@
impl<T: SerializeArray, const N: usize> SerializeArray for [T; N] {}
impl<T: DeserializeArray, const N: usize> Deserialize for [T; N] {
+ type UninitType = [T::UninitType; N];
+ fn uninit() -> Self::UninitType {
+ [(); N].map(|_| T::uninit())
+ }
+ fn from_init(value: Self) -> Self::UninitType {
+ value.map(T::from_init)
+ }
+
fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> {
let vec = DeserializeArray::deserialize_array(parcel)
.transpose()
@@ -664,6 +736,14 @@
}
impl Deserialize for Stability {
+ type UninitType = Self;
+ fn uninit() -> Self::UninitType {
+ Self::UninitType::default()
+ }
+ fn from_init(value: Self) -> Self::UninitType {
+ value
+ }
+
fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> {
i32::deserialize(parcel).and_then(Stability::try_from)
}
@@ -682,6 +762,14 @@
}
impl Deserialize for Status {
+ type UninitType = Option<Self>;
+ fn uninit() -> Self::UninitType {
+ Self::UninitType::default()
+ }
+ fn from_init(value: Self) -> Self::UninitType {
+ Some(value)
+ }
+
fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> {
let mut status_ptr = ptr::null_mut();
let ret_status = unsafe {
@@ -717,12 +805,29 @@
impl<T: Serialize + FromIBinder + ?Sized> SerializeArray for Strong<T> {}
impl<T: FromIBinder + ?Sized> Deserialize for Strong<T> {
+ type UninitType = Option<Strong<T>>;
+ fn uninit() -> Self::UninitType {
+ Self::UninitType::default()
+ }
+ fn from_init(value: Self) -> Self::UninitType {
+ Some(value)
+ }
+
fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> {
let ibinder: SpIBinder = parcel.read()?;
FromIBinder::try_from(ibinder)
}
}
+struct AssertIBinder;
+impl Interface for AssertIBinder {}
+impl FromIBinder for AssertIBinder {
+ // This is only needed so we can assert on the size of Strong<AssertIBinder>
+ fn try_from(_: SpIBinder) -> Result<Strong<Self>> {
+ unimplemented!()
+ }
+}
+
impl<T: FromIBinder + ?Sized> DeserializeOption for Strong<T> {
fn deserialize_option(parcel: &BorrowedParcel<'_>) -> Result<Option<Self>> {
let ibinder: Option<SpIBinder> = parcel.read()?;
@@ -752,6 +857,14 @@
}
impl<T: DeserializeOption> Deserialize for Option<T> {
+ type UninitType = Self;
+ fn uninit() -> Self::UninitType {
+ Self::UninitType::default()
+ }
+ fn from_init(value: Self) -> Self::UninitType {
+ value
+ }
+
fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> {
DeserializeOption::deserialize_option(parcel)
}
@@ -821,6 +934,9 @@
};
($parcelable:ident < $( $param:ident ),* > ) => {
impl < $($param: Default),* > $crate::binder_impl::Deserialize for $parcelable < $($param),* > {
+ type UninitType = Self;
+ fn uninit() -> Self::UninitType { Self::UninitType::default() }
+ fn from_init(value: Self) -> Self::UninitType { value }
fn deserialize(
parcel: &$crate::binder_impl::BorrowedParcel<'_>,
) -> std::result::Result<Self, $crate::StatusCode> {
@@ -876,6 +992,14 @@
}
impl<T: Deserialize> Deserialize for Box<T> {
+ type UninitType = Option<Self>;
+ fn uninit() -> Self::UninitType {
+ Self::UninitType::default()
+ }
+ fn from_init(value: Self) -> Self::UninitType {
+ Some(value)
+ }
+
fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> {
Deserialize::deserialize(parcel).map(Box::new)
}
@@ -900,6 +1024,7 @@
#[test]
fn test_custom_parcelable() {
+ #[derive(Default)]
struct Custom(u32, bool, String, Vec<String>);
impl Serialize for Custom {
@@ -912,6 +1037,14 @@
}
impl Deserialize for Custom {
+ type UninitType = Self;
+ fn uninit() -> Self::UninitType {
+ Self::UninitType::default()
+ }
+ fn from_init(value: Self) -> Self::UninitType {
+ value
+ }
+
fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self> {
Ok(Custom(
parcel.read()?,
diff --git a/libs/binder/rust/src/parcel/parcelable_holder.rs b/libs/binder/rust/src/parcel/parcelable_holder.rs
index c829d37..383cc83 100644
--- a/libs/binder/rust/src/parcel/parcelable_holder.rs
+++ b/libs/binder/rust/src/parcel/parcelable_holder.rs
@@ -169,6 +169,14 @@
}
impl Deserialize for ParcelableHolder {
+ type UninitType = Self;
+ fn uninit() -> Self::UninitType {
+ Self::new(Default::default())
+ }
+ fn from_init(value: Self) -> Self::UninitType {
+ value
+ }
+
fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<Self, StatusCode> {
let status: i32 = parcel.read()?;
if status == NULL_PARCELABLE_FLAG {
diff --git a/libs/binder/rust/src/proxy.rs b/libs/binder/rust/src/proxy.rs
index 254efae..036f6b4 100644
--- a/libs/binder/rust/src/proxy.rs
+++ b/libs/binder/rust/src/proxy.rs
@@ -439,6 +439,14 @@
impl SerializeArray for SpIBinder {}
impl Deserialize for SpIBinder {
+ type UninitType = Option<Self>;
+ fn uninit() -> Self::UninitType {
+ Self::UninitType::default()
+ }
+ fn from_init(value: Self) -> Self::UninitType {
+ Some(value)
+ }
+
fn deserialize(parcel: &BorrowedParcel<'_>) -> Result<SpIBinder> {
parcel.read_binder().transpose().unwrap_or(Err(StatusCode::UNEXPECTED_NULL))
}
diff --git a/libs/binder/rust/tests/parcel_fuzzer/Android.bp b/libs/binder/rust/tests/parcel_fuzzer/Android.bp
index df8a2af..ac96823 100644
--- a/libs/binder/rust/tests/parcel_fuzzer/Android.bp
+++ b/libs/binder/rust/tests/parcel_fuzzer/Android.bp
@@ -21,6 +21,7 @@
"waghpawan@google.com",
"smoreland@google.com",
],
+ triage_assignee: "waghpawan@google.com",
// hotlist "AIDL fuzzers bugs" on buganizer
hotlists: ["4637097"],
},
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 5cb406a..89126ca 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,6 +19,11 @@
srcs: [
"service_fuzzer.rs",
],
+ shared_libs: [
+ "libbinder",
+ "libbinder_ndk",
+ "libutils",
+ ],
rustlibs: [
"libbinder_rs",
"libbinder_random_parcel_rs",
@@ -29,6 +34,7 @@
"waghpawan@google.com",
"smoreland@google.com",
],
+ triage_assignee: "waghpawan@google.com",
// hotlist "AIDL fuzzers bugs" on buganizer
hotlists: ["4637097"],
},
diff --git a/libs/binder/tests/Android.bp b/libs/binder/tests/Android.bp
index 873e955..41856f9 100644
--- a/libs/binder/tests/Android.bp
+++ b/libs/binder/tests/Android.bp
@@ -32,28 +32,8 @@
}
cc_test {
- name: "binderDriverInterfaceTest_IPC_32",
- defaults: ["binder_test_defaults"],
- srcs: ["binderDriverInterfaceTest.cpp"],
- header_libs: ["libbinder_headers"],
- compile_multilib: "32",
- multilib: {
- lib32: {
- suffix: "",
- },
- },
- cflags: ["-DBINDER_IPC_32BIT=1"],
- test_suites: ["vts"],
-}
-
-cc_test {
name: "binderDriverInterfaceTest",
defaults: ["binder_test_defaults"],
- product_variables: {
- binder32bit: {
- cflags: ["-DBINDER_IPC_32BIT=1"],
- },
- },
header_libs: ["libbinder_headers"],
srcs: ["binderDriverInterfaceTest.cpp"],
test_suites: [
@@ -62,30 +42,6 @@
],
}
-cc_test {
- name: "binderLibTest_IPC_32",
- defaults: ["binder_test_defaults"],
- srcs: ["binderLibTest.cpp"],
- shared_libs: [
- "libbase",
- "libbinder",
- "liblog",
- "libutils",
- ],
- static_libs: [
- "libgmock",
- ],
- compile_multilib: "32",
- multilib: {
- lib32: {
- suffix: "",
- },
- },
- cflags: ["-DBINDER_IPC_32BIT=1"],
- test_suites: ["vts"],
- require_root: true,
-}
-
// unit test only, which can run on host and doesn't use /dev/binder
cc_test {
name: "binderUnitTest",
@@ -111,13 +67,39 @@
}
cc_test {
- name: "binderLibTest",
- defaults: ["binder_test_defaults"],
- product_variables: {
- binder32bit: {
- cflags: ["-DBINDER_IPC_32BIT=1"],
+ name: "binderRecordReplayTest",
+ srcs: ["binderRecordReplayTest.cpp"],
+ shared_libs: [
+ "libbinder",
+ "libcutils",
+ "libutils",
+ ],
+ static_libs: [
+ "binderRecordReplayTestIface-cpp",
+ "binderReadParcelIface-cpp",
+ ],
+ test_suites: ["general-tests"],
+ require_root: true,
+}
+
+aidl_interface {
+ name: "binderRecordReplayTestIface",
+ unstable: true,
+ srcs: [
+ "IBinderRecordReplayTest.aidl",
+ ],
+ imports: ["binderReadParcelIface"],
+ backend: {
+ java: {
+ enabled: true,
+ platform_apis: true,
},
},
+}
+
+cc_test {
+ name: "binderLibTest",
+ defaults: ["binder_test_defaults"],
srcs: ["binderLibTest.cpp"],
shared_libs: [
@@ -716,6 +698,7 @@
"liblog",
"libutils",
],
+ test_suites: ["general-tests"],
}
cc_test_host {
@@ -818,3 +801,15 @@
hotlists: ["4637097"],
},
}
+
+cc_defaults {
+ name: "fuzzer_disable_leaks",
+ fuzz_config: {
+ asan_options: [
+ "detect_leaks=0",
+ ],
+ hwasan_options: [
+ "detect_leaks=0",
+ ],
+ },
+}
diff --git a/libs/binder/tests/IBinderRecordReplayTest.aidl b/libs/binder/tests/IBinderRecordReplayTest.aidl
new file mode 100644
index 0000000..bd6b03c
--- /dev/null
+++ b/libs/binder/tests/IBinderRecordReplayTest.aidl
@@ -0,0 +1,72 @@
+/*
+ * Copyright (C) 2023 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.
+ */
+import parcelables.SingleDataParcelable;
+
+interface IBinderRecordReplayTest {
+ void setByte(byte input);
+ byte getByte();
+
+ void setChar(char input);
+ char getChar();
+
+ void setBoolean(boolean input);
+ boolean getBoolean();
+
+ void setInt(int input);
+ int getInt();
+
+ void setFloat(float input);
+ float getFloat();
+
+ void setLong(long input);
+ long getLong();
+
+ void setDouble(double input);
+ double getDouble();
+
+ void setString(String input);
+ String getString();
+
+ void setSingleDataParcelable(in SingleDataParcelable p);
+ SingleDataParcelable getSingleDataParcelable();
+
+ void setByteArray(in byte[] input);
+ byte[] getByteArray();
+
+ void setCharArray(in char[] input);
+ char[] getCharArray();
+
+ void setBooleanArray(in boolean[] input);
+ boolean[] getBooleanArray();
+
+ void setIntArray(in int[] input);
+ int[] getIntArray();
+
+ void setFloatArray(in float[] input);
+ float[] getFloatArray();
+
+ void setLongArray(in long[] input);
+ long[] getLongArray();
+
+ void setDoubleArray(in double[] input);
+ double[] getDoubleArray();
+
+ void setStringArray(in String[] input);
+ String[] getStringArray();
+
+ void setSingleDataParcelableArray(in SingleDataParcelable[] input);
+ SingleDataParcelable[] getSingleDataParcelableArray();
+}
diff --git a/libs/binder/tests/binderAbiHelper.h b/libs/binder/tests/binderAbiHelper.h
deleted file mode 100644
index 369b55d..0000000
--- a/libs/binder/tests/binderAbiHelper.h
+++ /dev/null
@@ -1,52 +0,0 @@
-/*
- * Copyright (C) 2020 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.
- */
-
-#pragma once
-
-#include <stdlib.h>
-#include <iostream>
-
-#ifdef BINDER_IPC_32BIT
-static constexpr bool kBuild32Abi = true;
-#else
-static constexpr bool kBuild32Abi = false;
-#endif
-
-// TODO: remove when CONFIG_ANDROID_BINDER_IPC_32BIT is no longer supported
-static inline bool ReadKernelConfigIs32BitAbi() {
- // failure case implies we run with standard ABI
- return 0 == system("zcat /proc/config.gz | grep -E \"^CONFIG_ANDROID_BINDER_IPC_32BIT=y$\"");
-}
-
-static inline void ExitIfWrongAbi() {
- bool runtime32Abi = ReadKernelConfigIs32BitAbi();
-
- if (kBuild32Abi != runtime32Abi) {
- std::cout << "[==========] Running 1 test from 1 test suite." << std::endl;
- std::cout << "[----------] Global test environment set-up." << std::endl;
- std::cout << "[----------] 1 tests from BinderLibTest" << std::endl;
- std::cout << "[ RUN ] BinderTest.AbortForWrongAbi" << std::endl;
- std::cout << "[ INFO ] test build abi 32: " << kBuild32Abi << " runtime abi 32: " << runtime32Abi << " so, skipping tests " << std::endl;
- std::cout << "[ OK ] BinderTest.AbortForWrongAbi (0 ms) " << std::endl;
- std::cout << "[----------] 1 tests from BinderTest (0 ms total)" << std::endl;
- std::cout << "" << std::endl;
- std::cout << "[----------] Global test environment tear-down" << std::endl;
- std::cout << "[==========] 1 test from 1 test suite ran. (0 ms total)" << std::endl;
- std::cout << "[ PASSED ] 1 tests." << std::endl;
- exit(0);
- }
-}
-
diff --git a/libs/binder/tests/binderDriverInterfaceTest.cpp b/libs/binder/tests/binderDriverInterfaceTest.cpp
index 8cc3054..cf23a46 100644
--- a/libs/binder/tests/binderDriverInterfaceTest.cpp
+++ b/libs/binder/tests/binderDriverInterfaceTest.cpp
@@ -25,8 +25,6 @@
#include <sys/mman.h>
#include <poll.h>
-#include "binderAbiHelper.h"
-
#define BINDER_DEV_NAME "/dev/binder"
testing::Environment* binder_env;
@@ -362,8 +360,7 @@
binderTestReadEmpty();
}
-int main(int argc, char **argv) {
- ExitIfWrongAbi();
+int main(int argc, char** argv) {
::testing::InitGoogleTest(&argc, argv);
binder_env = AddGlobalTestEnvironment(new BinderDriverInterfaceTestEnv());
diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp
index 8974ad7..abc423b 100644
--- a/libs/binder/tests/binderLibTest.cpp
+++ b/libs/binder/tests/binderLibTest.cpp
@@ -48,7 +48,6 @@
#include <sys/un.h>
#include "../binder_module.h"
-#include "binderAbiHelper.h"
#define ARRAY_SIZE(array) (sizeof array / sizeof array[0])
@@ -2022,9 +2021,7 @@
return 1; /* joinThreadPool should not return */
}
-int main(int argc, char **argv) {
- ExitIfWrongAbi();
-
+int main(int argc, char** argv) {
if (argc == 4 && !strcmp(argv[1], "--servername")) {
binderservername = argv[2];
} else {
diff --git a/libs/binder/tests/binderRecordReplayTest.cpp b/libs/binder/tests/binderRecordReplayTest.cpp
new file mode 100644
index 0000000..db78546
--- /dev/null
+++ b/libs/binder/tests/binderRecordReplayTest.cpp
@@ -0,0 +1,290 @@
+/*
+ * Copyright (C) 2023 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 <BnBinderRecordReplayTest.h>
+#include <android-base/logging.h>
+#include <android-base/unique_fd.h>
+#include <binder/Binder.h>
+#include <binder/BpBinder.h>
+#include <binder/IBinder.h>
+#include <binder/IPCThreadState.h>
+#include <binder/IServiceManager.h>
+#include <binder/RecordedTransaction.h>
+#include <gtest/gtest.h>
+
+#include <sys/prctl.h>
+
+#include "parcelables/SingleDataParcelable.h"
+
+using namespace android;
+using android::binder::Status;
+using android::binder::debug::RecordedTransaction;
+using parcelables::SingleDataParcelable;
+
+const String16 kServerName = String16("binderRecordReplay");
+
+#define GENERATE_GETTER_SETTER_PRIMITIVE(name, T) \
+ Status set##name(T input) { \
+ m##name = input; \
+ return Status::ok(); \
+ } \
+ \
+ Status get##name(T* output) { \
+ *output = m##name; \
+ return Status::ok(); \
+ } \
+ T m##name
+
+#define GENERATE_GETTER_SETTER(name, T) \
+ Status set##name(const T& input) { \
+ m##name = input; \
+ return Status::ok(); \
+ } \
+ \
+ Status get##name(T* output) { \
+ *output = m##name; \
+ return Status::ok(); \
+ } \
+ T m##name
+
+class MyRecordReplay : public BnBinderRecordReplayTest {
+public:
+ GENERATE_GETTER_SETTER_PRIMITIVE(Boolean, bool);
+ GENERATE_GETTER_SETTER_PRIMITIVE(Byte, int8_t);
+ GENERATE_GETTER_SETTER_PRIMITIVE(Int, int);
+ GENERATE_GETTER_SETTER_PRIMITIVE(Char, char16_t);
+ GENERATE_GETTER_SETTER_PRIMITIVE(Long, int64_t);
+ GENERATE_GETTER_SETTER_PRIMITIVE(Float, float);
+ GENERATE_GETTER_SETTER_PRIMITIVE(Double, double);
+
+ GENERATE_GETTER_SETTER(String, String16);
+ GENERATE_GETTER_SETTER(SingleDataParcelable, SingleDataParcelable);
+
+ GENERATE_GETTER_SETTER(BooleanArray, std::vector<bool>);
+ GENERATE_GETTER_SETTER(ByteArray, std::vector<uint8_t>);
+ GENERATE_GETTER_SETTER(IntArray, std::vector<int>);
+ GENERATE_GETTER_SETTER(CharArray, std::vector<char16_t>);
+ GENERATE_GETTER_SETTER(LongArray, std::vector<int64_t>);
+ GENERATE_GETTER_SETTER(FloatArray, std::vector<float>);
+ GENERATE_GETTER_SETTER(DoubleArray, std::vector<double>);
+ GENERATE_GETTER_SETTER(StringArray, std::vector<::android::String16>);
+ GENERATE_GETTER_SETTER(SingleDataParcelableArray, std::vector<SingleDataParcelable>);
+};
+
+class BinderRecordReplayTest : public ::testing::Test {
+public:
+ void SetUp() override {
+ // get the remote service
+ auto binder = defaultServiceManager()->getService(kServerName);
+ ASSERT_NE(nullptr, binder);
+ mInterface = interface_cast<IBinderRecordReplayTest>(binder);
+ mBpBinder = binder->remoteBinder();
+ ASSERT_NE(nullptr, mBpBinder);
+ }
+
+ template <typename T, typename U>
+ void recordReplay(Status (IBinderRecordReplayTest::*set)(T), U recordedValue,
+ Status (IBinderRecordReplayTest::*get)(U*), U changedValue) {
+ base::unique_fd fd(open("/data/local/tmp/binderRecordReplayTest.rec",
+ O_RDWR | O_CREAT | O_CLOEXEC, 0666));
+ ASSERT_TRUE(fd.ok());
+
+ // record a transaction
+ mBpBinder->startRecordingBinder(fd);
+ auto status = (*mInterface.*set)(recordedValue);
+ EXPECT_TRUE(status.isOk());
+ mBpBinder->stopRecordingBinder();
+
+ // test transaction does the thing we expect it to do
+ U output;
+ status = (*mInterface.*get)(&output);
+ EXPECT_TRUE(status.isOk());
+ EXPECT_EQ(output, recordedValue);
+
+ // write over the existing state
+ status = (*mInterface.*set)(changedValue);
+ EXPECT_TRUE(status.isOk());
+
+ status = (*mInterface.*get)(&output);
+ EXPECT_TRUE(status.isOk());
+
+ EXPECT_EQ(output, changedValue);
+
+ // replay transaction
+ ASSERT_EQ(0, lseek(fd.get(), 0, SEEK_SET));
+ std::optional<RecordedTransaction> transaction = RecordedTransaction::fromFile(fd);
+ ASSERT_NE(transaction, std::nullopt);
+
+ // TODO: move logic to replay RecordedTransaction into RecordedTransaction
+ Parcel data;
+ data.setData(transaction->getDataParcel().data(), transaction->getDataParcel().dataSize());
+ auto result =
+ mBpBinder->transact(transaction->getCode(), data, nullptr, transaction->getFlags());
+
+ // make sure recording does the thing we expect it to do
+ EXPECT_EQ(OK, result);
+
+ status = (*mInterface.*get)(&output);
+ EXPECT_TRUE(status.isOk());
+ EXPECT_EQ(output, recordedValue);
+ }
+
+private:
+ sp<BpBinder> mBpBinder;
+ sp<IBinderRecordReplayTest> mInterface;
+};
+
+TEST_F(BinderRecordReplayTest, ReplayByte) {
+ recordReplay(&IBinderRecordReplayTest::setByte, int8_t{122}, &IBinderRecordReplayTest::getByte,
+ int8_t{90});
+}
+
+TEST_F(BinderRecordReplayTest, ReplayBoolean) {
+ recordReplay(&IBinderRecordReplayTest::setBoolean, true, &IBinderRecordReplayTest::getBoolean,
+ false);
+}
+
+TEST_F(BinderRecordReplayTest, ReplayChar) {
+ recordReplay(&IBinderRecordReplayTest::setChar, char16_t{'G'},
+ &IBinderRecordReplayTest::getChar, char16_t{'K'});
+}
+
+TEST_F(BinderRecordReplayTest, ReplayInt) {
+ recordReplay(&IBinderRecordReplayTest::setInt, 3, &IBinderRecordReplayTest::getInt, 5);
+}
+
+TEST_F(BinderRecordReplayTest, ReplayFloat) {
+ recordReplay(&IBinderRecordReplayTest::setFloat, 1.1f, &IBinderRecordReplayTest::getFloat,
+ 22.0f);
+}
+
+TEST_F(BinderRecordReplayTest, ReplayLong) {
+ recordReplay(&IBinderRecordReplayTest::setLong, int64_t{1LL << 55},
+ &IBinderRecordReplayTest::getLong, int64_t{1LL << 12});
+}
+
+TEST_F(BinderRecordReplayTest, ReplayDouble) {
+ recordReplay(&IBinderRecordReplayTest::setDouble, 0.00, &IBinderRecordReplayTest::getDouble,
+ 1.11);
+}
+
+TEST_F(BinderRecordReplayTest, ReplayString) {
+ const ::android::String16& input1 = String16("This is saved string");
+ const ::android::String16& input2 = String16("This is changed string");
+ recordReplay(&IBinderRecordReplayTest::setString, input1, &IBinderRecordReplayTest::getString,
+ input2);
+}
+
+TEST_F(BinderRecordReplayTest, ReplaySingleDataParcelable) {
+ SingleDataParcelable saved, changed;
+ saved.data = 3;
+ changed.data = 5;
+ recordReplay(&IBinderRecordReplayTest::setSingleDataParcelable, saved,
+ &IBinderRecordReplayTest::getSingleDataParcelable, changed);
+}
+
+TEST_F(BinderRecordReplayTest, ReplayByteArray) {
+ std::vector<uint8_t> savedArray = {uint8_t{255}, uint8_t{0}, uint8_t{127}};
+ std::vector<uint8_t> changedArray = {uint8_t{2}, uint8_t{7}, uint8_t{117}};
+ recordReplay(&IBinderRecordReplayTest::setByteArray, savedArray,
+ &IBinderRecordReplayTest::getByteArray, changedArray);
+}
+
+TEST_F(BinderRecordReplayTest, ReplayBooleanArray) {
+ std::vector<bool> savedArray = {true, false, true};
+ std::vector<bool> changedArray = {false, true, false};
+ recordReplay(&IBinderRecordReplayTest::setBooleanArray, savedArray,
+ &IBinderRecordReplayTest::getBooleanArray, changedArray);
+}
+
+TEST_F(BinderRecordReplayTest, ReplayCharArray) {
+ std::vector<char16_t> savedArray = {char16_t{'G'}, char16_t{'L'}, char16_t{'K'}, char16_t{'T'}};
+ std::vector<char16_t> changedArray = {char16_t{'X'}, char16_t{'Y'}, char16_t{'Z'}};
+ recordReplay(&IBinderRecordReplayTest::setCharArray, savedArray,
+ &IBinderRecordReplayTest::getCharArray, changedArray);
+}
+
+TEST_F(BinderRecordReplayTest, ReplayIntArray) {
+ std::vector<int> savedArray = {12, 45, 178};
+ std::vector<int> changedArray = {32, 14, 78, 1899};
+ recordReplay(&IBinderRecordReplayTest::setIntArray, savedArray,
+ &IBinderRecordReplayTest::getIntArray, changedArray);
+}
+
+TEST_F(BinderRecordReplayTest, ReplayFloatArray) {
+ std::vector<float> savedArray = {12.14f, 45.56f, 123.178f};
+ std::vector<float> changedArray = {0.00f, 14.0f, 718.1f, 1899.122f, 3268.123f};
+ recordReplay(&IBinderRecordReplayTest::setFloatArray, savedArray,
+ &IBinderRecordReplayTest::getFloatArray, changedArray);
+}
+
+TEST_F(BinderRecordReplayTest, ReplayLongArray) {
+ std::vector<int64_t> savedArray = {int64_t{1LL << 11}, int64_t{1LL << 55}, int64_t{1LL << 45}};
+ std::vector<int64_t> changedArray = {int64_t{1LL << 1}, int64_t{1LL << 21}, int64_t{1LL << 33},
+ int64_t{1LL << 63}};
+ recordReplay(&IBinderRecordReplayTest::setLongArray, savedArray,
+ &IBinderRecordReplayTest::getLongArray, changedArray);
+}
+
+TEST_F(BinderRecordReplayTest, ReplayDoubleArray) {
+ std::vector<double> savedArray = {12.1412313, 45.561232, 123.1781111};
+ std::vector<double> changedArray = {0.00111, 14.32130, 712312318.19, 1899212.122,
+ 322168.122123};
+ recordReplay(&IBinderRecordReplayTest::setDoubleArray, savedArray,
+ &IBinderRecordReplayTest::getDoubleArray, changedArray);
+}
+
+TEST_F(BinderRecordReplayTest, ReplayStringArray) {
+ std::vector<String16> savedArray = {String16("This is saved value"), String16(),
+ String16("\0\0", 2), String16("\xF3\x01\xAC\xAD\x21\xAF")};
+
+ std::vector<String16> changedArray = {String16("This is changed value"), String16("\1\2", 30)};
+ recordReplay(&IBinderRecordReplayTest::setStringArray, savedArray,
+ &IBinderRecordReplayTest::getStringArray, changedArray);
+}
+
+TEST_F(BinderRecordReplayTest, ReplaySingleDataParcelableArray) {
+ SingleDataParcelable s1, s2, s3, s4, s5;
+ s1.data = 5213;
+ s2.data = 1512;
+ s3.data = 4233;
+ s4.data = 123124;
+ s5.data = 0;
+ std::vector<SingleDataParcelable> saved = {s1, s2, s3};
+ std::vector<SingleDataParcelable> changed = {s4, s5};
+
+ recordReplay(&IBinderRecordReplayTest::setSingleDataParcelableArray, saved,
+ &IBinderRecordReplayTest::getSingleDataParcelableArray, changed);
+}
+
+int main(int argc, char** argv) {
+ ::testing::InitGoogleTest(&argc, argv);
+
+ if (fork() == 0) {
+ prctl(PR_SET_PDEATHSIG, SIGHUP);
+
+ auto server = sp<MyRecordReplay>::make();
+ android::defaultServiceManager()->addService(kServerName, server.get());
+
+ IPCThreadState::self()->joinThreadPool(true);
+ exit(1); // should not reach
+ }
+
+ // not racey, but getService sleeps for 1s
+ usleep(100000);
+
+ return RUN_ALL_TESTS();
+}
diff --git a/libs/binder/tests/binderRpcBenchmark.cpp b/libs/binder/tests/binderRpcBenchmark.cpp
index 5939273..9c96c41 100644
--- a/libs/binder/tests/binderRpcBenchmark.cpp
+++ b/libs/binder/tests/binderRpcBenchmark.cpp
@@ -129,12 +129,33 @@
}
}
+static void SetLabel(benchmark::State& state) {
+ Transport transport = static_cast<Transport>(state.range(0));
+ switch (transport) {
+#ifdef __BIONIC__
+ case KERNEL:
+ state.SetLabel("kernel");
+ break;
+#endif
+ case RPC:
+ state.SetLabel("rpc");
+ break;
+ case RPC_TLS:
+ state.SetLabel("rpc_tls");
+ break;
+ default:
+ LOG(FATAL) << "Unknown transport value: " << transport;
+ }
+}
+
void BM_pingTransaction(benchmark::State& state) {
sp<IBinder> binder = getBinderForOptions(state);
while (state.KeepRunning()) {
CHECK_EQ(OK, binder->pingBinder());
}
+
+ SetLabel(state);
}
BENCHMARK(BM_pingTransaction)->ArgsProduct({kTransportList});
@@ -164,6 +185,8 @@
Status ret = iface->repeatString(str, &out);
CHECK(ret.isOk()) << ret;
}
+
+ SetLabel(state);
}
BENCHMARK(BM_repeatTwoPageString)->ArgsProduct({kTransportList});
@@ -182,6 +205,8 @@
Status ret = iface->repeatBytes(bytes, &out);
CHECK(ret.isOk()) << ret;
}
+
+ SetLabel(state);
}
BENCHMARK(BM_throughputForTransportAndBytes)
->ArgsProduct({kTransportList,
@@ -201,6 +226,8 @@
Status ret = iface->repeatBinder(binder, &out);
CHECK(ret.isOk()) << ret;
}
+
+ SetLabel(state);
}
BENCHMARK(BM_repeatBinder)->ArgsProduct({kTransportList});
@@ -228,11 +255,6 @@
::benchmark::Initialize(&argc, argv);
if (::benchmark::ReportUnrecognizedArguments(argc, argv)) return 1;
- 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::RPC_TLS << " is RPC with TLS" << std::endl;
-
#ifdef __BIONIC__
if (0 == fork()) {
prctl(PR_SET_PDEATHSIG, SIGHUP); // racey, okay
diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp
index 8d13007..d01e9d7 100644
--- a/libs/binder/tests/binderRpcTest.cpp
+++ b/libs/binder/tests/binderRpcTest.cpp
@@ -462,7 +462,7 @@
EXPECT_GE(epochMsAfter, epochMsBefore + 2 * sleepMs);
// Potential flake, but make sure calls are handled in parallel.
- EXPECT_LE(epochMsAfter, epochMsBefore + 3 * sleepMs);
+ EXPECT_LE(epochMsAfter, epochMsBefore + 4 * sleepMs);
}
TEST_P(BinderRpc, ThreadPoolOverSaturated) {
@@ -687,6 +687,8 @@
}
EXPECT_EQ(nullptr, session.promote());
+
+ sleep(1); // give time for remote session to shutdown
}
TEST_P(BinderRpc, SingleDeathRecipient) {
diff --git a/libs/binder/trusty/binderRpcTest/manifest.json b/libs/binder/trusty/binderRpcTest/manifest.json
index d8b080f..1cefac5 100644
--- a/libs/binder/trusty/binderRpcTest/manifest.json
+++ b/libs/binder/trusty/binderRpcTest/manifest.json
@@ -1,6 +1,6 @@
{
"uuid": "9dbe9fb8-60fd-4bdd-af86-03e95d7ad78b",
"app_name": "binderRpcTest",
- "min_heap": 163840,
+ "min_heap": 262144,
"min_stack": 16384
}
diff --git a/libs/gui/Android.bp b/libs/gui/Android.bp
index bf34987..bf2d7b6 100644
--- a/libs/gui/Android.bp
+++ b/libs/gui/Android.bp
@@ -247,6 +247,7 @@
shared_libs: [
"libbinder",
+ "libGLESv2",
],
export_shared_lib_headers: [
@@ -372,7 +373,6 @@
"libbase",
"libcutils",
"libEGL",
- "libGLESv2",
"libhidlbase",
"liblog",
"libnativewindow",
diff --git a/libs/gui/ISurfaceComposer.cpp b/libs/gui/ISurfaceComposer.cpp
index d72f65e..b526a6c 100644
--- a/libs/gui/ISurfaceComposer.cpp
+++ b/libs/gui/ISurfaceComposer.cpp
@@ -59,15 +59,13 @@
virtual ~BpSurfaceComposer();
- status_t setTransactionState(const FrameTimelineInfo& frameTimelineInfo,
- Vector<ComposerState>& state, const Vector<DisplayState>& displays,
- uint32_t flags, const sp<IBinder>& applyToken,
- InputWindowCommands commands, int64_t desiredPresentTime,
- bool isAutoTimestamp,
- const std::vector<client_cache_t>& uncacheBuffers,
- bool hasListenerCallbacks,
- const std::vector<ListenerCallbacks>& listenerCallbacks,
- uint64_t transactionId) override {
+ status_t setTransactionState(
+ const FrameTimelineInfo& frameTimelineInfo, Vector<ComposerState>& state,
+ const Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
+ InputWindowCommands commands, int64_t desiredPresentTime, bool isAutoTimestamp,
+ const std::vector<client_cache_t>& uncacheBuffers, bool hasListenerCallbacks,
+ const std::vector<ListenerCallbacks>& listenerCallbacks, uint64_t transactionId,
+ const std::vector<uint64_t>& mergedTransactionIds) override {
Parcel data, reply;
data.writeInterfaceToken(ISurfaceComposer::getInterfaceDescriptor());
@@ -103,6 +101,11 @@
SAFE_PARCEL(data.writeUint64, transactionId);
+ SAFE_PARCEL(data.writeUint32, static_cast<uint32_t>(mergedTransactionIds.size()));
+ for (auto mergedTransactionId : mergedTransactionIds) {
+ SAFE_PARCEL(data.writeUint64, mergedTransactionId);
+ }
+
if (flags & ISurfaceComposer::eOneWay) {
return remote()->transact(BnSurfaceComposer::SET_TRANSACTION_STATE,
data, &reply, IBinder::FLAG_ONEWAY);
@@ -187,10 +190,16 @@
uint64_t transactionId = -1;
SAFE_PARCEL(data.readUint64, &transactionId);
+ SAFE_PARCEL_READ_SIZE(data.readUint32, &count, data.dataSize());
+ std::vector<uint64_t> mergedTransactions(count);
+ for (size_t i = 0; i < count; i++) {
+ SAFE_PARCEL(data.readUint64, &mergedTransactions[i]);
+ }
+
return setTransactionState(frameTimelineInfo, state, displays, stateFlags, applyToken,
std::move(inputWindowCommands), desiredPresentTime,
isAutoTimestamp, uncacheBuffers, hasListenerCallbacks,
- listenerCallbacks, transactionId);
+ listenerCallbacks, transactionId, mergedTransactions);
}
default: {
return BBinder::onTransact(code, data, reply, flags);
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index c6c7367..0fda358 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -827,6 +827,15 @@
SAFE_PARCEL(parcel->readUint64, &uncacheBuffers[i].id);
}
+ count = static_cast<size_t>(parcel->readUint32());
+ if (count > parcel->dataSize()) {
+ return BAD_VALUE;
+ }
+ std::vector<uint64_t> mergedTransactionIds(count);
+ for (size_t i = 0; i < count; i++) {
+ SAFE_PARCEL(parcel->readUint64, &mergedTransactionIds[i]);
+ }
+
// Parsing was successful. Update the object.
mId = transactionId;
mTransactionNestCount = transactionNestCount;
@@ -842,6 +851,7 @@
mInputWindowCommands = inputWindowCommands;
mApplyToken = applyToken;
mUncacheBuffers = std::move(uncacheBuffers);
+ mMergedTransactionIds = std::move(mergedTransactionIds);
return NO_ERROR;
}
@@ -900,6 +910,11 @@
SAFE_PARCEL(parcel->writeUint64, uncacheBuffer.id);
}
+ SAFE_PARCEL(parcel->writeUint32, static_cast<uint32_t>(mMergedTransactionIds.size()));
+ for (auto mergedTransactionId : mMergedTransactionIds) {
+ SAFE_PARCEL(parcel->writeUint64, mergedTransactionId);
+ }
+
return NO_ERROR;
}
@@ -924,6 +939,22 @@
}
SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::merge(Transaction&& other) {
+ while (mMergedTransactionIds.size() + other.mMergedTransactionIds.size() >
+ MAX_MERGE_HISTORY_LENGTH - 1 &&
+ mMergedTransactionIds.size() > 0) {
+ mMergedTransactionIds.pop_back();
+ }
+ if (other.mMergedTransactionIds.size() == MAX_MERGE_HISTORY_LENGTH) {
+ mMergedTransactionIds.insert(mMergedTransactionIds.begin(),
+ other.mMergedTransactionIds.begin(),
+ other.mMergedTransactionIds.end() - 1);
+ } else if (other.mMergedTransactionIds.size() > 0u) {
+ mMergedTransactionIds.insert(mMergedTransactionIds.begin(),
+ other.mMergedTransactionIds.begin(),
+ other.mMergedTransactionIds.end());
+ }
+ mMergedTransactionIds.insert(mMergedTransactionIds.begin(), other.mId);
+
for (auto const& [handle, composerState] : other.mComposerStates) {
if (mComposerStates.count(handle) == 0) {
mComposerStates[handle] = composerState;
@@ -998,12 +1029,17 @@
mIsAutoTimestamp = true;
clearFrameTimelineInfo(mFrameTimelineInfo);
mApplyToken = nullptr;
+ mMergedTransactionIds.clear();
}
uint64_t SurfaceComposerClient::Transaction::getId() {
return mId;
}
+std::vector<uint64_t> SurfaceComposerClient::Transaction::getMergedTransactionIds() {
+ return mMergedTransactionIds;
+}
+
void SurfaceComposerClient::doUncacheBufferTransaction(uint64_t cacheId) {
sp<ISurfaceComposer> sf(ComposerService::getComposerService());
@@ -1014,7 +1050,7 @@
status_t status = sf->setTransactionState(FrameTimelineInfo{}, composerStates, {},
ISurfaceComposer::eOneWay,
Transaction::getDefaultApplyToken(), {}, systemTime(),
- true, {uncacheBuffer}, false, {}, generateId());
+ true, {uncacheBuffer}, false, {}, generateId(), {});
if (status != NO_ERROR) {
ALOGE_AND_TRACE("SurfaceComposerClient::doUncacheBufferTransaction - %s",
strerror(-status));
@@ -1189,7 +1225,8 @@
sp<ISurfaceComposer> sf(ComposerService::getComposerService());
sf->setTransactionState(mFrameTimelineInfo, composerStates, displayStates, flags, applyToken,
mInputWindowCommands, mDesiredPresentTime, mIsAutoTimestamp,
- mUncacheBuffers, hasListenerCallbacks, listenerCallbacks, mId);
+ mUncacheBuffers, hasListenerCallbacks, listenerCallbacks, mId,
+ mMergedTransactionIds);
mId = generateId();
// Clear the current states and flags
diff --git a/libs/gui/include/gui/ISurfaceComposer.h b/libs/gui/include/gui/ISurfaceComposer.h
index bd21851..7c150d5 100644
--- a/libs/gui/include/gui/ISurfaceComposer.h
+++ b/libs/gui/include/gui/ISurfaceComposer.h
@@ -116,7 +116,7 @@
InputWindowCommands inputWindowCommands, int64_t desiredPresentTime,
bool isAutoTimestamp, const std::vector<client_cache_t>& uncacheBuffer,
bool hasListenerCallbacks, const std::vector<ListenerCallbacks>& listenerCallbacks,
- uint64_t transactionId) = 0;
+ uint64_t transactionId, const std::vector<uint64_t>& mergedTransactionIds) = 0;
};
// ----------------------------------------------------------------------------
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index 8d2cdaf..fb57f63 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -419,6 +419,11 @@
mListenerCallbacks;
std::vector<client_cache_t> mUncacheBuffers;
+ // We keep track of the last MAX_MERGE_HISTORY_LENGTH merged transaction ids.
+ // Ordered most recently merged to least recently merged.
+ static const size_t MAX_MERGE_HISTORY_LENGTH = 10u;
+ std::vector<uint64_t> mMergedTransactionIds;
+
uint64_t mId;
uint32_t mTransactionNestCount = 0;
@@ -482,6 +487,8 @@
// The id is updated every time the transaction is applied.
uint64_t getId();
+ std::vector<uint64_t> getMergedTransactionIds();
+
status_t apply(bool synchronous = false, bool oneWay = false);
// Merge another transaction in to this one, clearing other
// as if it had been applied.
diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp
index 5bc6904..096a43c 100644
--- a/libs/gui/tests/Surface_test.cpp
+++ b/libs/gui/tests/Surface_test.cpp
@@ -695,16 +695,14 @@
mSupportsPresent = supportsPresent;
}
- status_t setTransactionState(const FrameTimelineInfo& /*frameTimelineInfo*/,
- Vector<ComposerState>& /*state*/,
- const Vector<DisplayState>& /*displays*/, uint32_t /*flags*/,
- const sp<IBinder>& /*applyToken*/,
- InputWindowCommands /*inputWindowCommands*/,
- int64_t /*desiredPresentTime*/, bool /*isAutoTimestamp*/,
- const std::vector<client_cache_t>& /*cachedBuffer*/,
- bool /*hasListenerCallbacks*/,
- const std::vector<ListenerCallbacks>& /*listenerCallbacks*/,
- uint64_t /*transactionId*/) override {
+ status_t setTransactionState(
+ const FrameTimelineInfo& /*frameTimelineInfo*/, Vector<ComposerState>& /*state*/,
+ const Vector<DisplayState>& /*displays*/, uint32_t /*flags*/,
+ const sp<IBinder>& /*applyToken*/, InputWindowCommands /*inputWindowCommands*/,
+ int64_t /*desiredPresentTime*/, bool /*isAutoTimestamp*/,
+ const std::vector<client_cache_t>& /*cachedBuffer*/, bool /*hasListenerCallbacks*/,
+ const std::vector<ListenerCallbacks>& /*listenerCallbacks*/, uint64_t /*transactionId*/,
+ const std::vector<uint64_t>& /*mergedTransactionIds*/) override {
return NO_ERROR;
}
diff --git a/opengl/libs/EGL/egl_angle_platform.cpp b/opengl/libs/EGL/egl_angle_platform.cpp
index f1122fd..9a6bb7a 100644
--- a/opengl/libs/EGL/egl_angle_platform.cpp
+++ b/opengl/libs/EGL/egl_angle_platform.cpp
@@ -152,6 +152,7 @@
if (!angleGetDisplayPlatform) {
ALOGE("dlsym lookup of ANGLEGetDisplayPlatform in libEGL_angle failed!");
+ dlclose(so);
return false;
}
@@ -162,6 +163,7 @@
if (!((angleGetDisplayPlatform)(dpy, g_PlatformMethodNames, g_NumPlatformMethods, nullptr,
&platformMethods))) {
ALOGE("ANGLEGetDisplayPlatform call failed!");
+ dlclose(so);
return false;
}
if (platformMethods) {
diff --git a/services/sensorservice/aidl/fuzzer/Android.bp b/services/sensorservice/aidl/fuzzer/Android.bp
index 5301fe9..ed4829a 100644
--- a/services/sensorservice/aidl/fuzzer/Android.bp
+++ b/services/sensorservice/aidl/fuzzer/Android.bp
@@ -11,6 +11,7 @@
name: "libsensorserviceaidl_fuzzer",
defaults: [
"service_fuzzer_defaults",
+ "fuzzer_disable_leaks",
],
host_supported: true,
static_libs: [
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index e841954..ec1a394 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -4405,7 +4405,8 @@
const Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
InputWindowCommands inputWindowCommands, int64_t desiredPresentTime, bool isAutoTimestamp,
const std::vector<client_cache_t>& uncacheBuffers, bool hasListenerCallbacks,
- const std::vector<ListenerCallbacks>& listenerCallbacks, uint64_t transactionId) {
+ const std::vector<ListenerCallbacks>& listenerCallbacks, uint64_t transactionId,
+ const std::vector<uint64_t>& mergedTransactionIds) {
ATRACE_CALL();
IPCThreadState* ipc = IPCThreadState::self();
@@ -4494,7 +4495,8 @@
listenerCallbacks,
originPid,
originUid,
- transactionId};
+ transactionId,
+ mergedTransactionIds};
if (mTransactionTracing) {
mTransactionTracing->addQueuedTransaction(state);
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 8e886fb..ec2da87 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -515,15 +515,13 @@
}
sp<IBinder> getPhysicalDisplayToken(PhysicalDisplayId displayId) const;
- status_t setTransactionState(const FrameTimelineInfo& frameTimelineInfo,
- Vector<ComposerState>& state, const Vector<DisplayState>& displays,
- uint32_t flags, const sp<IBinder>& applyToken,
- InputWindowCommands inputWindowCommands,
- int64_t desiredPresentTime, bool isAutoTimestamp,
- const std::vector<client_cache_t>& uncacheBuffers,
- bool hasListenerCallbacks,
- const std::vector<ListenerCallbacks>& listenerCallbacks,
- uint64_t transactionId) override;
+ status_t setTransactionState(
+ const FrameTimelineInfo& frameTimelineInfo, Vector<ComposerState>& state,
+ const Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
+ InputWindowCommands inputWindowCommands, int64_t desiredPresentTime,
+ bool isAutoTimestamp, const std::vector<client_cache_t>& uncacheBuffers,
+ bool hasListenerCallbacks, const std::vector<ListenerCallbacks>& listenerCallbacks,
+ uint64_t transactionId, const std::vector<uint64_t>& mergedTransactionIds) override;
void bootFinished();
virtual status_t getSupportedFrameTimestamps(std::vector<FrameEvent>* outSupported) const;
sp<IDisplayEventConnection> createDisplayEventConnection(
diff --git a/services/surfaceflinger/Tracing/TransactionProtoParser.cpp b/services/surfaceflinger/Tracing/TransactionProtoParser.cpp
index 19334e3..dafdc8a 100644
--- a/services/surfaceflinger/Tracing/TransactionProtoParser.cpp
+++ b/services/surfaceflinger/Tracing/TransactionProtoParser.cpp
@@ -69,6 +69,13 @@
for (auto& displayState : t.displays) {
proto.mutable_display_changes()->Add(std::move(toProto(displayState)));
}
+
+ proto.mutable_merged_transaction_ids()->Reserve(
+ static_cast<int32_t>(t.mergedTransactionIds.size()));
+ for (auto& mergedTransactionId : t.mergedTransactionIds) {
+ proto.mutable_merged_transaction_ids()->Add(mergedTransactionId);
+ }
+
return proto;
}
diff --git a/services/surfaceflinger/TransactionState.h b/services/surfaceflinger/TransactionState.h
index 62a7dfd..7132a59 100644
--- a/services/surfaceflinger/TransactionState.h
+++ b/services/surfaceflinger/TransactionState.h
@@ -56,7 +56,8 @@
int64_t desiredPresentTime, bool isAutoTimestamp,
std::vector<uint64_t> uncacheBufferIds, int64_t postTime,
bool hasListenerCallbacks, std::vector<ListenerCallbacks> listenerCallbacks,
- int originPid, int originUid, uint64_t transactionId)
+ int originPid, int originUid, uint64_t transactionId,
+ std::vector<uint64_t> mergedTransactionIds)
: frameTimelineInfo(frameTimelineInfo),
states(std::move(composerStates)),
displays(displayStates),
@@ -71,7 +72,8 @@
listenerCallbacks(listenerCallbacks),
originPid(originPid),
originUid(originUid),
- id(transactionId) {}
+ id(transactionId),
+ mergedTransactionIds(std::move(mergedTransactionIds)) {}
// Invokes `void(const layer_state_t&)` visitor for matching layers.
template <typename Visitor>
@@ -131,6 +133,7 @@
int originUid;
uint64_t id;
bool sentFenceTimeoutWarning = false;
+ std::vector<uint64_t> mergedTransactionIds;
};
} // namespace android
diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzer.cpp b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzer.cpp
index ce4d18f..80943b5 100644
--- a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzer.cpp
+++ b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzer.cpp
@@ -185,11 +185,12 @@
bool hasListenerCallbacks = mFdp.ConsumeBool();
std::vector<ListenerCallbacks> listenerCallbacks{};
uint64_t transactionId = mFdp.ConsumeIntegral<uint64_t>();
+ std::vector<uint64_t> mergedTransactionIds{};
mTestableFlinger.setTransactionState(FrameTimelineInfo{}, states, displays, flags, applyToken,
InputWindowCommands{}, desiredPresentTime, isAutoTimestamp,
- {}, hasListenerCallbacks, listenerCallbacks,
- transactionId);
+ {}, hasListenerCallbacks, listenerCallbacks, transactionId,
+ mergedTransactionIds);
}
void SurfaceFlingerFuzzer::setDisplayStateLocked() {
diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h
index 5c152b5..42b3879 100644
--- a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h
+++ b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h
@@ -737,19 +737,18 @@
return mFlinger->mTransactionHandler.mPendingTransactionQueues;
}
- auto setTransactionState(const FrameTimelineInfo& frameTimelineInfo,
- Vector<ComposerState>& states, const Vector<DisplayState>& displays,
- uint32_t flags, const sp<IBinder>& applyToken,
- const InputWindowCommands& inputWindowCommands,
- int64_t desiredPresentTime, bool isAutoTimestamp,
- const std::vector<client_cache_t>& uncacheBuffers,
- bool hasListenerCallbacks,
- std::vector<ListenerCallbacks>& listenerCallbacks,
- uint64_t transactionId) {
+ auto setTransactionState(
+ const FrameTimelineInfo& frameTimelineInfo, Vector<ComposerState>& states,
+ const Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
+ const InputWindowCommands& inputWindowCommands, int64_t desiredPresentTime,
+ bool isAutoTimestamp, const std::vector<client_cache_t>& uncacheBuffers,
+ bool hasListenerCallbacks, std::vector<ListenerCallbacks>& listenerCallbacks,
+ uint64_t transactionId, const std::vector<uint64_t>& mergedTransactionIds) {
return mFlinger->setTransactionState(frameTimelineInfo, states, displays, flags, applyToken,
inputWindowCommands, desiredPresentTime,
isAutoTimestamp, uncacheBuffers, hasListenerCallbacks,
- listenerCallbacks, transactionId);
+ listenerCallbacks, transactionId,
+ mergedTransactionIds);
}
auto flushTransactionQueues() {
diff --git a/services/surfaceflinger/layerproto/transactions.proto b/services/surfaceflinger/layerproto/transactions.proto
index 2c4eb10..b0cee9b 100644
--- a/services/surfaceflinger/layerproto/transactions.proto
+++ b/services/surfaceflinger/layerproto/transactions.proto
@@ -100,6 +100,7 @@
uint64 transaction_id = 6;
repeated LayerState layer_changes = 7;
repeated DisplayState display_changes = 8;
+ repeated uint64 merged_transaction_ids = 9;
}
// Keep insync with layer_state_t
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 3a8e2ff..2a0e865 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -483,19 +483,18 @@
return mFlinger->mTransactionHandler.mPendingTransactionCount.load();
}
- auto setTransactionState(const FrameTimelineInfo& frameTimelineInfo,
- Vector<ComposerState>& states, const Vector<DisplayState>& displays,
- uint32_t flags, const sp<IBinder>& applyToken,
- const InputWindowCommands& inputWindowCommands,
- int64_t desiredPresentTime, bool isAutoTimestamp,
- const std::vector<client_cache_t>& uncacheBuffers,
- bool hasListenerCallbacks,
- std::vector<ListenerCallbacks>& listenerCallbacks,
- uint64_t transactionId) {
+ auto setTransactionState(
+ const FrameTimelineInfo& frameTimelineInfo, Vector<ComposerState>& states,
+ const Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
+ const InputWindowCommands& inputWindowCommands, int64_t desiredPresentTime,
+ bool isAutoTimestamp, const std::vector<client_cache_t>& uncacheBuffers,
+ bool hasListenerCallbacks, std::vector<ListenerCallbacks>& listenerCallbacks,
+ uint64_t transactionId, const std::vector<uint64_t>& mergedTransactionIds) {
return mFlinger->setTransactionState(frameTimelineInfo, states, displays, flags, applyToken,
inputWindowCommands, desiredPresentTime,
isAutoTimestamp, uncacheBuffers, hasListenerCallbacks,
- listenerCallbacks, transactionId);
+ listenerCallbacks, transactionId,
+ mergedTransactionIds);
}
auto setTransactionStateInternal(TransactionState& transaction) {
diff --git a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
index 6a641b3..afb8efb 100644
--- a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
@@ -73,6 +73,7 @@
FrameTimelineInfo frameTimelineInfo;
std::vector<client_cache_t> uncacheBuffers;
uint64_t id = static_cast<uint64_t>(-1);
+ std::vector<uint64_t> mergedTransactionIds;
static_assert(0xffffffffffffffff == static_cast<uint64_t>(-1));
};
@@ -108,7 +109,7 @@
transaction.applyToken, transaction.inputWindowCommands,
transaction.desiredPresentTime, transaction.isAutoTimestamp,
transaction.uncacheBuffers, mHasListenerCallbacks, mCallbacks,
- transaction.id);
+ transaction.id, transaction.mergedTransactionIds);
// If transaction is synchronous, SF applyTransactionState should time out (5s) wating for
// SF to commit the transaction. If this is animation, it should not time out waiting.
@@ -135,7 +136,7 @@
transaction.applyToken, transaction.inputWindowCommands,
transaction.desiredPresentTime, transaction.isAutoTimestamp,
transaction.uncacheBuffers, mHasListenerCallbacks, mCallbacks,
- transaction.id);
+ transaction.id, transaction.mergedTransactionIds);
nsecs_t returnedTime = systemTime();
EXPECT_LE(returnedTime, applicationSentTime + TRANSACTION_TIMEOUT);
@@ -166,7 +167,7 @@
transactionA.applyToken, transactionA.inputWindowCommands,
transactionA.desiredPresentTime, transactionA.isAutoTimestamp,
transactionA.uncacheBuffers, mHasListenerCallbacks, mCallbacks,
- transactionA.id);
+ transactionA.id, transactionA.mergedTransactionIds);
// This thread should not have been blocked by the above transaction
// (5s is the timeout period that applyTransactionState waits for SF to
@@ -181,7 +182,7 @@
transactionB.applyToken, transactionB.inputWindowCommands,
transactionB.desiredPresentTime, transactionB.isAutoTimestamp,
transactionB.uncacheBuffers, mHasListenerCallbacks, mCallbacks,
- transactionB.id);
+ transactionB.id, transactionB.mergedTransactionIds);
// this thread should have been blocked by the above transaction
// if this is an animation, this thread should be blocked for 5s
@@ -218,7 +219,8 @@
transactionA.displays, transactionA.flags, transactionA.applyToken,
transactionA.inputWindowCommands, transactionA.desiredPresentTime,
transactionA.isAutoTimestamp, transactionA.uncacheBuffers,
- mHasListenerCallbacks, mCallbacks, transactionA.id);
+ mHasListenerCallbacks, mCallbacks, transactionA.id,
+ transactionA.mergedTransactionIds);
auto& transactionQueue = mFlinger.getTransactionQueue();
ASSERT_FALSE(transactionQueue.isEmpty());
@@ -238,7 +240,8 @@
transactionA.displays, transactionA.flags, transactionA.applyToken,
transactionA.inputWindowCommands, transactionA.desiredPresentTime,
transactionA.isAutoTimestamp, transactionA.uncacheBuffers,
- mHasListenerCallbacks, mCallbacks, transactionA.id);
+ mHasListenerCallbacks, mCallbacks, transactionA.id,
+ transactionA.mergedTransactionIds);
auto& transactionQueue = mFlinger.getTransactionQueue();
ASSERT_FALSE(transactionQueue.isEmpty());
@@ -251,7 +254,8 @@
mFlinger.setTransactionState(empty.frameTimelineInfo, empty.states, empty.displays, empty.flags,
empty.applyToken, empty.inputWindowCommands,
empty.desiredPresentTime, empty.isAutoTimestamp,
- empty.uncacheBuffers, mHasListenerCallbacks, mCallbacks, empty.id);
+ empty.uncacheBuffers, mHasListenerCallbacks, mCallbacks, empty.id,
+ empty.mergedTransactionIds);
// flush transaction queue should flush as desiredPresentTime has
// passed
@@ -388,7 +392,8 @@
transaction.desiredPresentTime,
transaction.isAutoTimestamp, {}, systemTime(),
mHasListenerCallbacks, mCallbacks, getpid(),
- static_cast<int>(getuid()), transaction.id);
+ static_cast<int>(getuid()), transaction.id,
+ transaction.mergedTransactionIds);
mFlinger.setTransactionStateInternal(transactionState);
}
mFlinger.flushTransactionQueues();
@@ -1083,4 +1088,137 @@
EXPECT_EQ(transactionsReadyToBeApplied.front().id, 42u);
}
+TEST(TransactionHandlerTest, TransactionsKeepTrackOfDirectMerges) {
+ SurfaceComposerClient::Transaction transaction1, transaction2, transaction3, transaction4;
+
+ uint64_t transaction2Id = transaction2.getId();
+ uint64_t transaction3Id = transaction3.getId();
+ EXPECT_NE(transaction2Id, transaction3Id);
+
+ transaction1.merge(std::move(transaction2));
+ transaction1.merge(std::move(transaction3));
+
+ EXPECT_EQ(transaction1.getMergedTransactionIds().size(), 2u);
+ EXPECT_EQ(transaction1.getMergedTransactionIds()[0], transaction3Id);
+ EXPECT_EQ(transaction1.getMergedTransactionIds()[1], transaction2Id);
+}
+
+TEST(TransactionHandlerTest, TransactionsKeepTrackOfIndirectMerges) {
+ SurfaceComposerClient::Transaction transaction1, transaction2, transaction3, transaction4;
+
+ uint64_t transaction2Id = transaction2.getId();
+ uint64_t transaction3Id = transaction3.getId();
+ uint64_t transaction4Id = transaction4.getId();
+ EXPECT_NE(transaction2Id, transaction3Id);
+ EXPECT_NE(transaction2Id, transaction4Id);
+ EXPECT_NE(transaction3Id, transaction4Id);
+
+ transaction4.merge(std::move(transaction2));
+ transaction4.merge(std::move(transaction3));
+
+ EXPECT_EQ(transaction4.getMergedTransactionIds().size(), 2u);
+ EXPECT_EQ(transaction4.getMergedTransactionIds()[0], transaction3Id);
+ EXPECT_EQ(transaction4.getMergedTransactionIds()[1], transaction2Id);
+
+ transaction1.merge(std::move(transaction4));
+
+ EXPECT_EQ(transaction1.getMergedTransactionIds().size(), 3u);
+ EXPECT_EQ(transaction1.getMergedTransactionIds()[0], transaction4Id);
+ EXPECT_EQ(transaction1.getMergedTransactionIds()[1], transaction3Id);
+ EXPECT_EQ(transaction1.getMergedTransactionIds()[2], transaction2Id);
+}
+
+TEST(TransactionHandlerTest, TransactionMergesAreCleared) {
+ SurfaceComposerClient::Transaction transaction1, transaction2, transaction3;
+
+ transaction1.merge(std::move(transaction2));
+ transaction1.merge(std::move(transaction3));
+
+ EXPECT_EQ(transaction1.getMergedTransactionIds().size(), 2u);
+
+ transaction1.clear();
+
+ EXPECT_EQ(transaction1.getMergedTransactionIds().empty(), true);
+}
+
+TEST(TransactionHandlerTest, TransactionMergesAreCapped) {
+ SurfaceComposerClient::Transaction transaction;
+ std::vector<uint64_t> mergedTransactionIds;
+
+ for (uint i = 0; i < 20u; i++) {
+ SurfaceComposerClient::Transaction transactionToMerge;
+ mergedTransactionIds.push_back(transactionToMerge.getId());
+ transaction.merge(std::move(transactionToMerge));
+ }
+
+ // Keeps latest 10 merges in order of merge recency
+ EXPECT_EQ(transaction.getMergedTransactionIds().size(), 10u);
+ for (uint i = 0; i < 10u; i++) {
+ EXPECT_EQ(transaction.getMergedTransactionIds()[i],
+ mergedTransactionIds[mergedTransactionIds.size() - 1 - i]);
+ }
+}
+
+TEST(TransactionHandlerTest, KeepsMergesFromMoreRecentMerge) {
+ SurfaceComposerClient::Transaction transaction1, transaction2, transaction3;
+ std::vector<uint64_t> mergedTransactionIds1, mergedTransactionIds2, mergedTransactionIds3;
+ uint64_t transaction2Id = transaction2.getId();
+ uint64_t transaction3Id = transaction3.getId();
+
+ for (uint i = 0; i < 20u; i++) {
+ SurfaceComposerClient::Transaction transactionToMerge;
+ mergedTransactionIds1.push_back(transactionToMerge.getId());
+ transaction1.merge(std::move(transactionToMerge));
+ }
+
+ for (uint i = 0; i < 5u; i++) {
+ SurfaceComposerClient::Transaction transactionToMerge;
+ mergedTransactionIds2.push_back(transactionToMerge.getId());
+ transaction2.merge(std::move(transactionToMerge));
+ }
+
+ transaction1.merge(std::move(transaction2));
+ EXPECT_EQ(transaction1.getMergedTransactionIds().size(), 10u);
+ EXPECT_EQ(transaction1.getMergedTransactionIds()[0], transaction2Id);
+ for (uint i = 0; i < 5u; i++) {
+ EXPECT_EQ(transaction1.getMergedTransactionIds()[i + 1u],
+ mergedTransactionIds2[mergedTransactionIds2.size() - 1 - i]);
+ }
+ for (uint i = 0; i < 4u; i++) {
+ EXPECT_EQ(transaction1.getMergedTransactionIds()[i + 6u],
+ mergedTransactionIds1[mergedTransactionIds1.size() - 1 - i]);
+ }
+
+ for (uint i = 0; i < 20u; i++) {
+ SurfaceComposerClient::Transaction transactionToMerge;
+ mergedTransactionIds3.push_back(transactionToMerge.getId());
+ transaction3.merge(std::move(transactionToMerge));
+ }
+
+ transaction1.merge(std::move(transaction3));
+ EXPECT_EQ(transaction1.getMergedTransactionIds().size(), 10u);
+ EXPECT_EQ(transaction1.getMergedTransactionIds()[0], transaction3Id);
+ for (uint i = 0; i < 9u; i++) {
+ EXPECT_EQ(transaction1.getMergedTransactionIds()[i + 1],
+ mergedTransactionIds3[mergedTransactionIds3.size() - 1 - i]);
+ }
+}
+
+TEST(TransactionHandlerTest, CanAddTransactionWithFullMergedIds) {
+ SurfaceComposerClient::Transaction transaction1, transaction2;
+ for (uint i = 0; i < 20u; i++) {
+ SurfaceComposerClient::Transaction transactionToMerge;
+ transaction1.merge(std::move(transactionToMerge));
+ }
+
+ EXPECT_EQ(transaction1.getMergedTransactionIds().size(), 10u);
+
+ auto transaction1Id = transaction1.getId();
+ transaction2.merge(std::move(transaction1));
+ EXPECT_EQ(transaction2.getMergedTransactionIds().size(), 10u);
+ auto mergedTransactionIds = transaction2.getMergedTransactionIds();
+ EXPECT_TRUE(std::count(mergedTransactionIds.begin(), mergedTransactionIds.end(),
+ transaction1Id) > 0);
+}
+
} // namespace android
diff --git a/services/surfaceflinger/tests/unittests/TransactionTracingTest.cpp b/services/surfaceflinger/tests/unittests/TransactionTracingTest.cpp
index 82aac7e..92411a7 100644
--- a/services/surfaceflinger/tests/unittests/TransactionTracingTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TransactionTracingTest.cpp
@@ -67,8 +67,14 @@
ASSERT_EQ(actualProto.transactions().size(),
static_cast<int32_t>(expectedTransactions.size()));
for (uint32_t i = 0; i < expectedTransactions.size(); i++) {
- EXPECT_EQ(actualProto.transactions(static_cast<int32_t>(i)).pid(),
- expectedTransactions[i].originPid);
+ const auto expectedTransaction = expectedTransactions[i];
+ const auto protoTransaction = actualProto.transactions(static_cast<int32_t>(i));
+ EXPECT_EQ(protoTransaction.transaction_id(), expectedTransaction.id);
+ EXPECT_EQ(protoTransaction.pid(), expectedTransaction.originPid);
+ for (uint32_t i = 0; i < expectedTransaction.mergedTransactionIds.size(); i++) {
+ EXPECT_EQ(protoTransaction.merged_transaction_ids(static_cast<int32_t>(i)),
+ expectedTransaction.mergedTransactionIds[i]);
+ }
}
}
@@ -92,6 +98,7 @@
TransactionState transaction;
transaction.id = i;
transaction.originPid = static_cast<int32_t>(i);
+ transaction.mergedTransactionIds = std::vector<uint64_t>{i + 100, i + 102};
transactions.emplace_back(transaction);
mTracing.addQueuedTransaction(transaction);
}