Merge "binderRpcTest: work when vsock not supported"
diff --git a/libs/binder/RpcSession.cpp b/libs/binder/RpcSession.cpp
index 11dc1ab..107210b 100644
--- a/libs/binder/RpcSession.cpp
+++ b/libs/binder/RpcSession.cpp
@@ -506,7 +506,6 @@
// TODO(b/189955605): we should add additional sessions dynamically
// instead of all at once.
- // TODO(b/186470974): first risk of blocking
size_t numThreadsAvailable;
if (status_t status = getRemoteMaxThreads(&numThreadsAvailable); status != OK) {
ALOGE("Could not get max threads after initial session setup: %s",
diff --git a/libs/binder/ndk/include_cpp/android/binder_interface_utils.h b/libs/binder/ndk/include_cpp/android/binder_interface_utils.h
index 70a3906..5de64f8 100644
--- a/libs/binder/ndk/include_cpp/android/binder_interface_utils.h
+++ b/libs/binder/ndk/include_cpp/android/binder_interface_utils.h
@@ -43,23 +43,37 @@
namespace ndk {
/**
+ * analog using std::shared_ptr for internally held refcount
+ *
* ref must be called at least one time during the lifetime of this object. The recommended way to
* construct this object is with SharedRefBase::make.
- *
- * Note - this class used to not inherit from enable_shared_from_this, so
- * std::make_shared works, but it won't be portable against old copies of this
- * class.
*/
-class SharedRefBase : public std::enable_shared_from_this<SharedRefBase> {
+class SharedRefBase {
public:
SharedRefBase() {}
- virtual ~SharedRefBase() {}
+ virtual ~SharedRefBase() {
+ std::call_once(mFlagThis, [&]() {
+ __assert(__FILE__, __LINE__, "SharedRefBase: no ref created during lifetime");
+ });
+
+ if (ref() != nullptr) {
+ __assert(__FILE__, __LINE__,
+ "SharedRefBase: destructed but still able to lock weak_ptr. Is this object "
+ "double-owned?");
+ }
+ }
/**
* A shared_ptr must be held to this object when this is called. This must be called once during
* the lifetime of this object.
*/
- std::shared_ptr<SharedRefBase> ref() { return shared_from_this(); }
+ std::shared_ptr<SharedRefBase> ref() {
+ std::shared_ptr<SharedRefBase> thiz = mThis.lock();
+
+ std::call_once(mFlagThis, [&]() { mThis = thiz = std::shared_ptr<SharedRefBase>(this); });
+
+ return thiz;
+ }
/**
* Convenience method for a ref (see above) which automatically casts to the desired child type.
@@ -78,13 +92,8 @@
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
T* t = new T(std::forward<Args>(args)...);
#pragma clang diagnostic pop
-
- // T may have a private (though necessarily virtual!) destructor, so we
- // can't use refbase. For getting the first ref, we don't use ref()
- // since the internal shared_ptr isn't guaranteed to exist yet.
- SharedRefBase* base = static_cast<SharedRefBase*>(t);
- std::shared_ptr<SharedRefBase> strongBase(base);
- return std::static_pointer_cast<T>(strongBase);
+ // warning: Potential leak of memory pointed to by 't' [clang-analyzer-unix.Malloc]
+ return t->template ref<T>(); // NOLINT(clang-analyzer-unix.Malloc)
}
static void operator delete(void* p) { std::free(p); }
@@ -97,9 +106,13 @@
#if !defined(__ANDROID_API__) || __ANDROID_API__ >= 30 || defined(__ANDROID_APEX__)
private:
#else
- [[deprecated("Prefer SharedRefBase::make<T>(...) or std::make_shared<T>() if possible.")]]
+ [[deprecated("Prefer SharedRefBase::make<T>(...) if possible.")]]
#endif
static void* operator new(size_t s) { return std::malloc(s); }
+
+ private:
+ std::once_flag mFlagThis;
+ std::weak_ptr<SharedRefBase> mThis;
};
/**
diff --git a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp
index 94cc086..b5c06e9 100644
--- a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp
+++ b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp
@@ -54,15 +54,6 @@
constexpr uint64_t kContextTestValue = 0xb4e42fb4d9a1d715;
class MyBinderNdkUnitTest : public aidl::BnBinderNdkUnitTest {
- public:
- MyBinderNdkUnitTest() = default;
- MyBinderNdkUnitTest(bool* deleted) : deleted(deleted) {}
- ~MyBinderNdkUnitTest() {
- if (deleted) {
- *deleted = true;
- }
- }
-
ndk::ScopedAStatus repeatInt(int32_t in, int32_t* out) {
*out = in;
return ndk::ScopedAStatus::ok();
@@ -131,7 +122,6 @@
}
uint64_t contextTestValue = kContextTestValue;
- bool* deleted = nullptr;
};
int generatedService() {
@@ -234,25 +224,15 @@
return true;
}
-TEST(NdkBinder, MakeShared) {
- const char* kInstance = "make_shared_test_instance";
- bool deleted = false;
+TEST(NdkBinder, DetectDoubleOwn) {
+ auto badService = ndk::SharedRefBase::make<MyBinderNdkUnitTest>();
+ EXPECT_DEATH(std::shared_ptr<MyBinderNdkUnitTest>(badService.get()),
+ "Is this object double-owned?");
+}
- {
- auto service = std::make_shared<MyBinderNdkUnitTest>(&deleted);
- auto binder = service->asBinder();
- ASSERT_EQ(EX_NONE, AServiceManager_addService(binder.get(), kInstance));
- auto binder2 = ndk::SpAIBinder(AServiceManager_checkService(kInstance));
- ASSERT_EQ(binder.get(), binder2.get());
-
- // overwrite service
- ASSERT_EQ(EX_NONE,
- AServiceManager_addService(
- std::make_shared<MyBinderNdkUnitTest>(&deleted)->asBinder().get(),
- kInstance));
- }
-
- EXPECT_TRUE(deleted);
+TEST(NdkBinder, DetectNoSharedRefBaseCreated) {
+ EXPECT_DEATH(std::make_shared<MyBinderNdkUnitTest>(),
+ "SharedRefBase: no ref created during lifetime");
}
TEST(NdkBinder, GetServiceThatDoesntExist) {
diff --git a/libs/binder/rust/src/parcel/file_descriptor.rs b/libs/binder/rust/src/parcel/file_descriptor.rs
index 20e9178..179b7c8 100644
--- a/libs/binder/rust/src/parcel/file_descriptor.rs
+++ b/libs/binder/rust/src/parcel/file_descriptor.rs
@@ -23,7 +23,7 @@
use crate::sys;
use std::fs::File;
-use std::os::unix::io::{AsRawFd, FromRawFd};
+use std::os::unix::io::{AsRawFd, FromRawFd, RawFd};
/// Rust version of the Java class android.os.ParcelFileDescriptor
#[derive(Debug)]
@@ -48,6 +48,12 @@
}
}
+impl AsRawFd for ParcelFileDescriptor {
+ fn as_raw_fd(&self) -> RawFd {
+ self.0.as_raw_fd()
+ }
+}
+
impl Serialize for ParcelFileDescriptor {
fn serialize(&self, parcel: &mut Parcel) -> Result<()> {
let fd = self.0.as_raw_fd();
diff --git a/libs/binder/tests/IBinderRpcBenchmark.aidl b/libs/binder/tests/IBinderRpcBenchmark.aidl
index 1457422..2baf680 100644
--- a/libs/binder/tests/IBinderRpcBenchmark.aidl
+++ b/libs/binder/tests/IBinderRpcBenchmark.aidl
@@ -17,4 +17,5 @@
interface IBinderRpcBenchmark {
@utf8InCpp String repeatString(@utf8InCpp String str);
IBinder repeatBinder(IBinder binder);
+ byte[] repeatBytes(in byte[] bytes);
}
diff --git a/libs/binder/tests/binderRpcBenchmark.cpp b/libs/binder/tests/binderRpcBenchmark.cpp
index 8d75a53..0c452ff 100644
--- a/libs/binder/tests/binderRpcBenchmark.cpp
+++ b/libs/binder/tests/binderRpcBenchmark.cpp
@@ -52,8 +52,12 @@
*out = str;
return Status::ok();
}
- Status repeatBinder(const sp<IBinder>& str, sp<IBinder>* out) override {
- *out = str;
+ Status repeatBinder(const sp<IBinder>& binder, sp<IBinder>* out) override {
+ *out = binder;
+ return Status::ok();
+ }
+ Status repeatBytes(const std::vector<uint8_t>& bytes, std::vector<uint8_t>* out) override {
+ *out = bytes;
return Status::ok();
}
};
@@ -63,12 +67,11 @@
RPC,
};
-static void EachTransport(benchmark::internal::Benchmark* b) {
+static const std::initializer_list<int64_t> kTransportList = {
#ifdef __BIONIC__
- b->Args({Transport::KERNEL});
+ Transport::KERNEL,
#endif
- b->Args({Transport::RPC});
-}
+ Transport::RPC};
static sp<RpcSession> gSession = RpcSession::make();
#ifdef __BIONIC__
@@ -98,9 +101,9 @@
CHECK_EQ(OK, binder->pingBinder());
}
}
-BENCHMARK(BM_pingTransaction)->Apply(EachTransport);
+BENCHMARK(BM_pingTransaction)->ArgsProduct({kTransportList});
-void BM_repeatString(benchmark::State& state) {
+void BM_repeatTwoPageString(benchmark::State& state) {
sp<IBinder> binder = getBinderForOptions(state);
sp<IBinderRpcBenchmark> iface = interface_cast<IBinderRpcBenchmark>(binder);
@@ -127,7 +130,27 @@
CHECK(ret.isOk()) << ret;
}
}
-BENCHMARK(BM_repeatString)->Apply(EachTransport);
+BENCHMARK(BM_repeatTwoPageString)->ArgsProduct({kTransportList});
+
+void BM_throughputForTransportAndBytes(benchmark::State& state) {
+ sp<IBinder> binder = getBinderForOptions(state);
+ sp<IBinderRpcBenchmark> iface = interface_cast<IBinderRpcBenchmark>(binder);
+ CHECK(iface != nullptr);
+
+ std::vector<uint8_t> bytes = std::vector<uint8_t>(state.range(1));
+ for (size_t i = 0; i < bytes.size(); i++) {
+ bytes[i] = i % 256;
+ }
+
+ while (state.KeepRunning()) {
+ std::vector<uint8_t> out;
+ Status ret = iface->repeatBytes(bytes, &out);
+ CHECK(ret.isOk()) << ret;
+ }
+}
+BENCHMARK(BM_throughputForTransportAndBytes)
+ ->ArgsProduct({kTransportList,
+ {64, 1024, 2048, 4096, 8182, 16364, 32728, 65535, 65536, 65537}});
void BM_repeatBinder(benchmark::State& state) {
sp<IBinder> binder = gSession->getRootObject();
@@ -144,7 +167,7 @@
CHECK(ret.isOk()) << ret;
}
}
-BENCHMARK(BM_repeatBinder)->Apply(EachTransport);
+BENCHMARK(BM_repeatBinder)->ArgsProduct({kTransportList});
int main(int argc, char** argv) {
::benchmark::Initialize(&argc, argv);
@@ -154,8 +177,8 @@
(void)unlink(addr.c_str());
std::cerr << "Tests suffixes:" << std::endl;
- std::cerr << "\t\\" << Transport::KERNEL << " is KERNEL" << std::endl;
- std::cerr << "\t\\" << Transport::RPC << " is RPC" << std::endl;
+ std::cerr << "\t.../" << Transport::KERNEL << " is KERNEL" << std::endl;
+ std::cerr << "\t.../" << Transport::RPC << " is RPC" << std::endl;
if (0 == fork()) {
prctl(PR_SET_PDEATHSIG, SIGHUP); // racey, okay
