Merge "binder: SharedRefBase uses enable_shared_from_this"
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 6c44726..70a3906 100644
--- a/libs/binder/ndk/include_cpp/android/binder_interface_utils.h
+++ b/libs/binder/ndk/include_cpp/android/binder_interface_utils.h
@@ -43,31 +43,23 @@
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 {
+class SharedRefBase : public std::enable_shared_from_this<SharedRefBase> {
public:
SharedRefBase() {}
- virtual ~SharedRefBase() {
- std::call_once(mFlagThis, [&]() {
- __assert(__FILE__, __LINE__, "SharedRefBase: no ref created during lifetime");
- });
- }
+ virtual ~SharedRefBase() {}
/**
* 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() {
- std::shared_ptr<SharedRefBase> thiz = mThis.lock();
-
- std::call_once(mFlagThis, [&]() { mThis = thiz = std::shared_ptr<SharedRefBase>(this); });
-
- return thiz;
- }
+ std::shared_ptr<SharedRefBase> ref() { return shared_from_this(); }
/**
* Convenience method for a ref (see above) which automatically casts to the desired child type.
@@ -86,8 +78,13 @@
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
T* t = new T(std::forward<Args>(args)...);
#pragma clang diagnostic pop
- // warning: Potential leak of memory pointed to by 't' [clang-analyzer-unix.Malloc]
- return t->template ref<T>(); // NOLINT(clang-analyzer-unix.Malloc)
+
+ // 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);
}
static void operator delete(void* p) { std::free(p); }
@@ -100,13 +97,9 @@
#if !defined(__ANDROID_API__) || __ANDROID_API__ >= 30 || defined(__ANDROID_APEX__)
private:
#else
- [[deprecated("Prefer SharedRefBase::make<T>(...) if possible.")]]
+ [[deprecated("Prefer SharedRefBase::make<T>(...) or std::make_shared<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 5ad390e..94cc086 100644
--- a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp
+++ b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp
@@ -54,6 +54,15 @@
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();
@@ -122,6 +131,7 @@
}
uint64_t contextTestValue = kContextTestValue;
+ bool* deleted = nullptr;
};
int generatedService() {
@@ -224,6 +234,27 @@
return true;
}
+TEST(NdkBinder, MakeShared) {
+ const char* kInstance = "make_shared_test_instance";
+ bool deleted = false;
+
+ {
+ 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, GetServiceThatDoesntExist) {
sp<IFoo> foo = IFoo::getService("asdfghkl;");
EXPECT_EQ(nullptr, foo.get());