Merge "Revert "Remove gBn/sConstructorMap from headers.""
diff --git a/base/HidlSupport.cpp b/base/HidlSupport.cpp
index 8f3c057..a69faa2 100644
--- a/base/HidlSupport.cpp
+++ b/base/HidlSupport.cpp
@@ -254,6 +254,14 @@
if (size > UINT32_MAX) {
LOG(FATAL) << "string size can't exceed 2^32 bytes: " << size;
}
+
+ // When the binder driver copies this data into its buffer, it must
+ // have a zero byte there because the remote process will have a pointer
+ // directly into the read-only binder buffer. If we manually copy the
+ // data now to add a zero, then we lose the efficiency of this method.
+ // Checking here (it's also checked in the parceling code later).
+ CHECK(data[size] == '\0');
+
clear();
mBuffer = data;
diff --git a/base/include/hidl/HidlSupport.h b/base/include/hidl/HidlSupport.h
index f09eb63..d1221fe 100644
--- a/base/include/hidl/HidlSupport.h
+++ b/base/include/hidl/HidlSupport.h
@@ -155,6 +155,8 @@
// Reference an external char array. Ownership is _not_ transferred.
// Caller is responsible for ensuring that underlying memory is valid
// for the lifetime of this hidl_string.
+ //
+ // size == strlen(data)
void setToExternal(const char *data, size_t size);
// offsetof(hidl_string, mBuffer) exposed since mBuffer is private.
diff --git a/libhidlcache/libhidlcache_test.cpp b/libhidlcache/libhidlcache_test.cpp
index ce21c6f..e514460 100644
--- a/libhidlcache/libhidlcache_test.cpp
+++ b/libhidlcache/libhidlcache_test.cpp
@@ -20,6 +20,7 @@
#include <android/hidl/memory/1.0/IMemory.h>
#include <android/hidl/memory/token/1.0/IMemoryToken.h>
#include <gtest/gtest.h>
+#include <hidlcache/MemoryDealer.h>
#include <hidlcache/mapping.h>
#include <hidlmemory/HidlMemoryToken.h>
#include <hidlmemory/mapping.h>
@@ -105,6 +106,51 @@
hardware::HidlCacheWhiteBoxTest();
}
+TEST_F(HidlCacheTest, MemoryDealer) {
+ using ::android::hardware::HidlMemory;
+ using ::android::hardware::hidl_memory;
+ using ::android::hardware::HidlMemoryDealer;
+ using ::android::hidl::allocator::V1_0::IAllocator;
+ using ::android::hidl::memory::block::V1_0::MemoryBlock;
+
+ sp<IAllocator> ashmemAllocator;
+
+ ashmemAllocator = IAllocator::getService("ashmem");
+ sp<HidlMemory> m1;
+ sp<HidlMemory> m2;
+ // test MemoryDealer
+ EXPECT_OK(ashmemAllocator->allocate(2048, [&m1](bool success, const hidl_memory& mem) {
+ ASSERT_TRUE(success);
+ m1 = HidlMemory::getInstance(mem);
+ }));
+
+ EXPECT_OK(ashmemAllocator->allocate(4096, [&m2](bool success, const hidl_memory& mem) {
+ ASSERT_TRUE(success);
+ m2 = HidlMemory::getInstance(mem);
+ }));
+
+ sp<HidlMemoryDealer> dealer;
+
+ // m1 does not statisfy the alignment requirement and should fail.
+ dealer = HidlMemoryDealer::getInstance(*m1);
+ EXPECT_EQ(nullptr, dealer.get());
+
+ dealer = HidlMemoryDealer::getInstance(*m2);
+ EXPECT_NE(nullptr, dealer.get());
+
+ EXPECT_EQ(dealer->heap()->getSize(), 4096ULL);
+ MemoryBlock blk = dealer->allocate(1024);
+ EXPECT_TRUE(HidlMemoryDealer::isOk(blk));
+ MemoryBlock blk2 = dealer->allocate(2048);
+ EXPECT_TRUE(HidlMemoryDealer::isOk(blk2));
+
+ MemoryBlock blk3 = dealer->allocate(2048);
+ EXPECT_FALSE(HidlMemoryDealer::isOk(blk3));
+ dealer->deallocate(blk2.offset);
+ blk3 = dealer->allocate(2048);
+ EXPECT_TRUE(HidlMemoryDealer::isOk(blk3));
+}
+
int main(int argc, char** argv) {
::testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS();
diff --git a/transport/Android.bp b/transport/Android.bp
index 77cfe27..5b0c11c 100644
--- a/transport/Android.bp
+++ b/transport/Android.bp
@@ -38,7 +38,6 @@
"libvndksupport",
],
export_shared_lib_headers: [
- "libbase",
"libutils",
"libhidlbase",
],
diff --git a/transport/ServiceManagement.cpp b/transport/ServiceManagement.cpp
index 4bdeb6e..edd2af3 100644
--- a/transport/ServiceManagement.cpp
+++ b/transport/ServiceManagement.cpp
@@ -30,6 +30,7 @@
#include <hidl/HidlBinderSupport.h>
#include <hidl/HidlInternal.h>
+#include <hidl/HidlTransportUtils.h>
#include <hidl/ServiceManagement.h>
#include <hidl/Status.h>
@@ -357,7 +358,12 @@
return true; // this module doesn't provide this instance name
}
- registerReference(fqName, name);
+ // Actual fqname might be a subclass.
+ // This assumption is tested in vts_treble_vintf_test
+ using ::android::hardware::details::getDescriptor;
+ std::string actualFqName = getDescriptor(ret.get());
+ CHECK(actualFqName.size() > 0);
+ registerReference(actualFqName, name);
return false;
});
@@ -511,12 +517,9 @@
}
~Waiter() {
- if (mRegisteredForNotifications) {
- if (!mSm->unregisterForNotifications(mInterfaceName, mInstanceName, this).
- withDefault(false)) {
- LOG(ERROR) << "Could not unregister service notification for "
- << mInterfaceName << "/" << mInstanceName << ".";
- }
+ if (!mDoneCalled) {
+ LOG(FATAL)
+ << "Waiter still registered for notifications, call done() before dropping ref!";
}
}
@@ -567,7 +570,23 @@
std::unique_lock<std::mutex> lock(mMutex);
mRegistered = false;
}
-private:
+
+ // done() must be called before dropping the last strong ref to the Waiter, to make
+ // sure we can properly unregister with hwservicemanager.
+ void done() {
+ if (mRegisteredForNotifications) {
+ if (!mSm->unregisterForNotifications(mInterfaceName, mInstanceName, this)
+ .withDefault(false)) {
+ LOG(ERROR) << "Could not unregister service notification for " << mInterfaceName
+ << "/" << mInstanceName << ".";
+ } else {
+ mRegisteredForNotifications = false;
+ }
+ }
+ mDoneCalled = true;
+ }
+
+ private:
const std::string mInterfaceName;
const std::string mInstanceName;
const sp<IServiceManager1_1>& mSm;
@@ -575,12 +594,14 @@
std::condition_variable mCondition;
bool mRegistered = false;
bool mRegisteredForNotifications = false;
+ bool mDoneCalled = false;
};
void waitForHwService(
const std::string &interface, const std::string &instanceName) {
sp<Waiter> waiter = new Waiter(interface, instanceName, defaultServiceManager1_1());
waiter->wait();
+ waiter->done();
}
// Prints relevant error/warning messages for error return values from
@@ -620,6 +641,7 @@
using Transport = ::android::hidl::manager::V1_0::IServiceManager::Transport;
using ::android::hidl::base::V1_0::IBase;
using ::android::hidl::manager::V1_0::IServiceManager;
+ sp<Waiter> waiter;
const sp<IServiceManager1_1> sm = defaultServiceManager1_1();
if (sm == nullptr) {
@@ -655,8 +677,10 @@
const bool vintfLegacy = (transport == Transport::EMPTY);
#endif // ENFORCE_VINTF_MANIFEST
- sp<Waiter> waiter = new Waiter(descriptor, instance, sm);
while (!getStub && (vintfHwbinder || vintfLegacy)) {
+ if (waiter == nullptr) {
+ waiter = new Waiter(descriptor, instance, sm);
+ }
waiter->reset(); // don't reorder this -- see comments on reset()
Return<sp<IBase>> ret = sm->get(descriptor, instance);
if (!ret.isOk()) {
@@ -670,6 +694,7 @@
details::canCastInterface(base.get(), descriptor.c_str(), true /* emitError */);
if (canCastRet.isOk() && canCastRet) {
+ waiter->done();
return base; // still needs to be wrapped by Bp class.
}
@@ -683,6 +708,10 @@
waiter->wait();
}
+ if (waiter != nullptr) {
+ waiter->done();
+ }
+
if (getStub || vintfPassthru || vintfLegacy) {
const sp<IServiceManager> pm = getPassthroughServiceManager();
if (pm != nullptr) {
diff --git a/vintfdata/manifest.xml b/vintfdata/manifest.xml
index f34bfb1..e20add5 100644
--- a/vintfdata/manifest.xml
+++ b/vintfdata/manifest.xml
@@ -65,7 +65,7 @@
<hal format="hidl">
<name>android.system.net.netd</name>
<transport>hwbinder</transport>
- <version>1.0</version>
+ <version>1.1</version>
<interface>
<name>INetd</name>
<instance>default</instance>