Fix proxy memory leak.
am: 25508806b8
Change-Id: I3595ec43463190457c17e883ec5c1e224cf0ccb8
diff --git a/base/HidlInternal.cpp b/base/HidlInternal.cpp
index 3bb27f8..b0d867f 100644
--- a/base/HidlInternal.cpp
+++ b/base/HidlInternal.cpp
@@ -37,21 +37,21 @@
// ----------------------------------------------------------------------
// HidlInstrumentor implementation.
-HidlInstrumentor::HidlInstrumentor(
- const std::string &package,
- const std::string &interface)
- : mInstrumentationLibPackage(package), mInterfaceName(interface) {
+HidlInstrumentor::HidlInstrumentor(const std::string& package, const std::string& interface)
+ : mEnableInstrumentation(false),
+ mInstrumentationLibPackage(package),
+ mInterfaceName(interface) {
configureInstrumentation(false);
}
HidlInstrumentor:: ~HidlInstrumentor() {}
void HidlInstrumentor::configureInstrumentation(bool log) {
- bool enable_instrumentation = property_get_bool(
+ bool enableInstrumentation = property_get_bool(
"hal.instrumentation.enable",
false);
- if (enable_instrumentation != mEnableInstrumentation) {
- mEnableInstrumentation = enable_instrumentation;
+ if (enableInstrumentation != mEnableInstrumentation) {
+ mEnableInstrumentation = enableInstrumentation;
if (mEnableInstrumentation) {
if (log) {
LOG(INFO) << "Enable instrumentation.";
@@ -70,11 +70,11 @@
std::vector<InstrumentationCallback> *instrumentationCallbacks) {
#ifdef LIBHIDL_TARGET_DEBUGGABLE
std::vector<std::string> instrumentationLibPaths;
- char instrumentation_lib_path[PROPERTY_VALUE_MAX];
+ char instrumentationLibPath[PROPERTY_VALUE_MAX];
if (property_get("hal.instrumentation.lib.path",
- instrumentation_lib_path,
+ instrumentationLibPath,
"") > 0) {
- instrumentationLibPaths.push_back(instrumentation_lib_path);
+ instrumentationLibPaths.push_back(instrumentationLibPath);
} else {
instrumentationLibPaths.push_back(HAL_LIBRARY_PATH_SYSTEM);
instrumentationLibPaths.push_back(HAL_LIBRARY_PATH_VENDOR);
@@ -103,7 +103,7 @@
dlerror(); /* Clear any existing error */
- using cb_fun = void (*)(
+ using cbFun = void (*)(
const InstrumentationEvent,
const char *,
const char *,
@@ -123,7 +123,7 @@
continue;
}
}
- auto cb = (cb_fun)dlsym(handle, ("HIDL_INSTRUMENTATION_FUNCTION_"
+ auto cb = (cbFun)dlsym(handle, ("HIDL_INSTRUMENTATION_FUNCTION_"
+ package + "_" + mInterfaceName).c_str());
if ((error = dlerror()) != NULL) {
LOG(WARNING)
diff --git a/base/HidlSupport.cpp b/base/HidlSupport.cpp
index 7580693..4de7f7c 100644
--- a/base/HidlSupport.cpp
+++ b/base/HidlSupport.cpp
@@ -99,6 +99,7 @@
}
void hidl_handle::setTo(native_handle_t* handle, bool shouldOwn) {
+ freeHandle();
mHandle = handle;
mOwnsHandle = shouldOwn;
}
diff --git a/base/include/hidl/HidlSupport.h b/base/include/hidl/HidlSupport.h
index d7c3b83..6807860 100644
--- a/base/include/hidl/HidlSupport.h
+++ b/base/include/hidl/HidlSupport.h
@@ -94,14 +94,14 @@
hidl_handle(const hidl_handle &other);
// move constructor.
- hidl_handle(hidl_handle &&other);
+ hidl_handle(hidl_handle &&other) noexcept;
// assignment operators
hidl_handle &operator=(const hidl_handle &other);
hidl_handle &operator=(const native_handle_t *native_handle);
- hidl_handle &operator=(hidl_handle &&other);
+ hidl_handle &operator=(hidl_handle &&other) noexcept;
void setTo(native_handle_t* handle, bool shouldOwn = false);
@@ -133,7 +133,7 @@
hidl_string(const std::string &);
// move constructor.
- hidl_string(hidl_string &&);
+ hidl_string(hidl_string &&) noexcept;
const char *c_str() const;
size_t size() const;
@@ -146,7 +146,7 @@
// copy from an std::string.
hidl_string &operator=(const std::string &);
// move assignment operator.
- hidl_string &operator=(hidl_string &&other);
+ hidl_string &operator=(hidl_string &&other) noexcept;
// cast to std::string.
operator std::string() const;
@@ -235,12 +235,12 @@
}
// move constructor
- hidl_memory(hidl_memory&& other) {
+ hidl_memory(hidl_memory&& other) noexcept {
*this = std::move(other);
}
// move assignment
- hidl_memory &operator=(hidl_memory &&other) {
+ hidl_memory &operator=(hidl_memory &&other) noexcept {
if (this != &other) {
mHandle = std::move(other.mHandle);
mSize = other.mSize;
@@ -293,7 +293,7 @@
*this = other;
}
- hidl_vec(hidl_vec<T> &&other)
+ hidl_vec(hidl_vec<T> &&other) noexcept
: mOwnsBuffer(false) {
*this = std::move(other);
}
@@ -354,7 +354,7 @@
return mBuffer;
}
- hidl_vec &operator=(hidl_vec &&other) {
+ hidl_vec &operator=(hidl_vec &&other) noexcept {
if (mOwnsBuffer) {
delete[] mBuffer;
}
diff --git a/libhidlmemory/mapping.cpp b/libhidlmemory/mapping.cpp
index f4bb21e..3cb6485 100644
--- a/libhidlmemory/mapping.cpp
+++ b/libhidlmemory/mapping.cpp
@@ -15,6 +15,10 @@
*/
#define LOG_TAG "libhidlmemory"
+#include <map>
+#include <mutex>
+#include <string>
+
#include <hidlmemory/mapping.h>
#include <android-base/logging.h>
@@ -23,14 +27,31 @@
using android::sp;
using android::hidl::memory::V1_0::IMemory;
+using android::hidl::memory::V1_0::IMapper;
namespace android {
namespace hardware {
-sp<IMemory> mapMemory(const hidl_memory &memory) {
- using android::hidl::memory::V1_0::IMapper;
+static std::map<std::string, sp<IMapper>> gMappersByName;
+static std::mutex gMutex;
- sp<IMapper> mapper = IMapper::getService(memory.name(), true /* getStub */);
+static inline sp<IMapper> getMapperService(const std::string& name) {
+ std::unique_lock<std::mutex> _lock(gMutex);
+ auto iter = gMappersByName.find(name);
+ if (iter != gMappersByName.end()) {
+ return iter->second;
+ }
+
+ sp<IMapper> mapper = IMapper::getService(name, true /* getStub */);
+ if (mapper != nullptr) {
+ gMappersByName[name] = mapper;
+ }
+ return mapper;
+}
+
+sp<IMemory> mapMemory(const hidl_memory& memory) {
+
+ sp<IMapper> mapper = getMapperService(memory.name());
if (mapper == nullptr) {
LOG(ERROR) << "Could not fetch mapper for " << memory.name() << " shared memory";
diff --git a/manifest.xml b/manifest.xml
index c258bce..799b660 100644
--- a/manifest.xml
+++ b/manifest.xml
@@ -35,4 +35,49 @@
<instance>default</instance>
</interface>
</hal>
+ <hal>
+ <name>android.frameworks.displayservice</name>
+ <transport>hwbinder</transport>
+ <version>1.0</version>
+ <interface>
+ <name>IDisplayService</name>
+ <instance>default</instance>
+ </interface>
+ </hal>
+ <hal>
+ <name>android.frameworks.schedulerservice</name>
+ <transport>hwbinder</transport>
+ <version>1.0</version>
+ <interface>
+ <name>ISchedulingPolicyService</name>
+ <instance>default</instance>
+ </interface>
+ </hal>
+ <hal>
+ <name>android.frameworks.sensorservice</name>
+ <transport>hwbinder</transport>
+ <version>1.0</version>
+ <interface>
+ <name>ISensorManager</name>
+ <instance>default</instance>
+ </interface>
+ </hal>
+ <hal>
+ <name>android.system.wifi.keystore</name>
+ <transport>hwbinder</transport>
+ <version>1.0</version>
+ <interface>
+ <name>IKeystore</name>
+ <instance>default</instance>
+ </interface>
+ </hal>
+ <hal>
+ <name>android.hardware.graphics.composer</name>
+ <transport>hwbinder</transport>
+ <version>2.1</version>
+ <interface>
+ <name>IComposer</name>
+ <instance>vr</instance>
+ </interface>
+ </hal>
</manifest>
diff --git a/transport/HidlTransportSupport.cpp b/transport/HidlTransportSupport.cpp
index a5ec8e2..ea2e32c 100644
--- a/transport/HidlTransportSupport.cpp
+++ b/transport/HidlTransportSupport.cpp
@@ -14,7 +14,6 @@
* limitations under the License.
*/
#include <hidl/HidlTransportSupport.h>
-
#include <hidl/HidlBinderSupport.h>
namespace android {
@@ -29,5 +28,30 @@
joinBinderRpcThreadpool();
}
+bool setMinSchedulerPolicy(const sp<::android::hidl::base::V1_0::IBase>& service,
+ int policy, int priority) {
+ if (service->isRemote()) {
+ ALOGE("Can't set scheduler policy on remote service.");
+ return false;
+ }
+
+ if (policy != SCHED_NORMAL && policy != SCHED_FIFO && policy != SCHED_RR) {
+ ALOGE("Invalid scheduler policy %d", policy);
+ return false;
+ }
+
+ if (policy == SCHED_NORMAL && (priority < -20 || priority > 19)) {
+ ALOGE("Invalid priority for SCHED_NORMAL: %d", priority);
+ return false;
+ } else if (priority < 1 || priority > 99) {
+ ALOGE("Invalid priority for real-time policy: %d", priority);
+ return false;
+ }
+
+ details::gServicePrioMap.set(service, { policy, priority });
+
+ return true;
}
-}
\ No newline at end of file
+
+}
+}
diff --git a/transport/ServiceManagement.cpp b/transport/ServiceManagement.cpp
index 0b7ec34..b14479e 100644
--- a/transport/ServiceManagement.cpp
+++ b/transport/ServiceManagement.cpp
@@ -16,6 +16,7 @@
#define LOG_TAG "ServiceManagement"
+#include <android/dlext.h>
#include <condition_variable>
#include <dlfcn.h>
#include <dirent.h>
@@ -44,6 +45,10 @@
#define RE_PATH RE_COMPONENT "(?:[.]" RE_COMPONENT ")*"
static const std::regex gLibraryFileNamePattern("(" RE_PATH "@[0-9]+[.][0-9]+)-impl(.*?).so");
+extern "C" {
+ android_namespace_t* android_get_exported_namespace(const char*);
+}
+
using android::base::WaitForProperty;
using android::hidl::manager::V1_0::IServiceManager;
@@ -267,6 +272,7 @@
const std::string prefix = packageAndVersion + "-impl";
const std::string sym = "HIDL_FETCH_" + ifaceName;
+ const android_namespace_t* sphal_namespace = android_get_exported_namespace("sphal");
const int dlMode = RTLD_LAZY;
void *handle = nullptr;
@@ -283,7 +289,29 @@
for (const std::string &lib : libs) {
const std::string fullPath = path + lib;
- handle = dlopen(fullPath.c_str(), dlMode);
+ // If sphal namespace is available, try to load from the
+ // namespace first. If it fails, fall back to the original
+ // dlopen, which loads from the current namespace.
+ if (sphal_namespace != nullptr && path != HAL_LIBRARY_PATH_SYSTEM) {
+ const android_dlextinfo dlextinfo = {
+ .flags = ANDROID_DLEXT_USE_NAMESPACE,
+ // const_cast is dirty but required because
+ // library_namespace field is non-const.
+ .library_namespace = const_cast<android_namespace_t*>(sphal_namespace),
+ };
+ handle = android_dlopen_ext(fullPath.c_str(), dlMode, &dlextinfo);
+ if (handle == nullptr) {
+ const char* error = dlerror();
+ LOG(WARNING) << "Failed to dlopen " << lib << " from sphal namespace:"
+ << (error == nullptr ? "unknown error" : error);
+ } else {
+ LOG(DEBUG) << lib << " loaded from sphal namespace.";
+ }
+ }
+ if (handle == nullptr) {
+ handle = dlopen(fullPath.c_str(), dlMode);
+ }
+
if (handle == nullptr) {
const char* error = dlerror();
LOG(ERROR) << "Failed to dlopen " << lib << ": "
diff --git a/transport/Static.cpp b/transport/Static.cpp
index 496c8f0..18cb475 100644
--- a/transport/Static.cpp
+++ b/transport/Static.cpp
@@ -32,6 +32,8 @@
ConcurrentMap<std::string, std::function<sp<IBinder>(void *)>>
gBnConstructorMap{};
+ConcurrentMap<wp<::android::hidl::base::V1_0::IBase>, SchedPrio> gServicePrioMap{};
+
ConcurrentMap<std::string, std::function<sp<::android::hidl::base::V1_0::IBase>(void *)>>
gBsConstructorMap;
diff --git a/transport/allocator/1.0/IAllocator.hal b/transport/allocator/1.0/IAllocator.hal
index 814c69d..6f531dd 100644
--- a/transport/allocator/1.0/IAllocator.hal
+++ b/transport/allocator/1.0/IAllocator.hal
@@ -29,4 +29,14 @@
* @return memory Unmapped memory object.
*/
allocate(uint64_t size) generates (bool success, memory mem);
+
+ /**
+ * Return memory must have instance name corresponding to this type of memory.
+ *
+ * @param size Size of memory to allocate in bytes.
+ * @param count Number of memory instances to allocate.
+ * @return success Whether allocation succeeded (returns false if any allocation failed).
+ * @return batch Unmapped memory objects.
+ */
+ batchAllocate(uint64_t size, uint64_t count) generates (bool success, vec<memory> batch);
};
\ No newline at end of file
diff --git a/transport/allocator/1.0/default/AshmemAllocator.cpp b/transport/allocator/1.0/default/AshmemAllocator.cpp
index ce6dbf7..0bd4f81 100644
--- a/transport/allocator/1.0/default/AshmemAllocator.cpp
+++ b/transport/allocator/1.0/default/AshmemAllocator.cpp
@@ -14,6 +14,9 @@
* limitations under the License.
*/
+#define LOG_TAG "AshmemAllocator"
+#include <android-base/logging.h>
+
#include "AshmemAllocator.h"
#include <cutils/ashmem.h>
@@ -24,21 +27,70 @@
namespace V1_0 {
namespace implementation {
-// Methods from ::android::hidl::allocator::V1_0::IAllocator follow.
-Return<void> AshmemAllocator::allocate(uint64_t size, allocate_cb _hidl_cb) {
+static hidl_memory allocateOne(uint64_t size) {
int fd = ashmem_create_region("AshmemAllocator_hidl", size);
if (fd < 0) {
- _hidl_cb(false /* success */, hidl_memory());
- return Void();
+ LOG(WARNING) << "ashmem_create_region(" << size << ") fails with " << fd;
+ return hidl_memory();
}
native_handle_t* handle = native_handle_create(1, 0);
handle->data[0] = fd;
- hidl_memory memory("ashmem", handle, size);
+ LOG(WARNING) << "ashmem_create_region(" << size << ") returning hidl_memory(" << handle
+ << ", " << size << ")";
+ return hidl_memory("ashmem", handle, size);
+}
- _hidl_cb(true /* success */, memory);
- native_handle_close(handle);
- native_handle_delete(handle);
+static void cleanup(hidl_memory&& memory) {
+ if (memory.handle() == nullptr) {
+ return;
+ }
+
+ native_handle_close(const_cast<native_handle_t *>(memory.handle()));
+ native_handle_delete(const_cast<native_handle_t *>(memory.handle()));
+}
+
+Return<void> AshmemAllocator::allocate(uint64_t size, allocate_cb _hidl_cb) {
+ hidl_memory memory = allocateOne(size);
+ _hidl_cb(memory.handle() != nullptr /* success */, memory);
+ cleanup(std::move(memory));
+
+ return Void();
+}
+
+Return<void> AshmemAllocator::batchAllocate(uint64_t size, uint64_t count, batchAllocate_cb _hidl_cb) {
+ // resize fails if count > 2^32
+ if (count > UINT32_MAX) {
+ _hidl_cb(false /* success */, {});
+ return Void();
+ }
+
+ hidl_vec<hidl_memory> batch;
+ batch.resize(count);
+
+ uint64_t allocated;
+ for (allocated = 0; allocated < count; allocated++) {
+ batch[allocated] = allocateOne(size);
+
+ if (batch[allocated].handle() == nullptr) {
+ LOG(WARNING) << "batchAllocate(" << size << ", " << count << ") fails @ #" << allocated;
+ break;
+ }
+ }
+
+ // batch[i].handle() != nullptr for i in [0, allocated - 1].
+ // batch[i].handle() == nullptr for i in [allocated, count - 1].
+
+ if (allocated < count) {
+ _hidl_cb(false /* success */, {});
+ } else {
+ _hidl_cb(true /* success */, batch);
+ }
+
+ for (uint64_t i = 0; i < allocated; i++) {
+ cleanup(std::move(batch[i]));
+ }
+
return Void();
}
diff --git a/transport/allocator/1.0/default/AshmemAllocator.h b/transport/allocator/1.0/default/AshmemAllocator.h
index 307cb5a..131417d 100644
--- a/transport/allocator/1.0/default/AshmemAllocator.h
+++ b/transport/allocator/1.0/default/AshmemAllocator.h
@@ -39,7 +39,7 @@
struct AshmemAllocator : public IAllocator {
// Methods from ::android::hidl::allocator::V1_0::IAllocator follow.
Return<void> allocate(uint64_t size, allocate_cb _hidl_cb) override;
-
+ Return<void> batchAllocate(uint64_t size, uint64_t count, batchAllocate_cb _hidl_cb) override;
};
} // namespace implementation
diff --git a/transport/current.txt b/transport/current.txt
new file mode 100644
index 0000000..a6ecd81
--- /dev/null
+++ b/transport/current.txt
@@ -0,0 +1,13 @@
+# Do not change this file except to add new interfaces. Changing
+# pre-existing interfaces will fail VTS and break framework-only OTAs
+
+# HALs released in Android O
+
+fc6cbbc8a22edabd4b58f8949e591359d3138d16a82506052e25ac43e1dbda68 android.hidl.allocator@1.0::IAllocator
+bddab6184d7a346da6a07dc0828cf19a696f4caa3611c51f2e14565a14b40fd9 android.hidl.base@1.0::IBase
+500ec34f1b0826a93c4abe45b23c4d85565d8041acaf3cf9fb23c09702967567 android.hidl.base@1.0::types
+4d046a598e85f1c2d383c3a9096c3c0578e97458072ee7e67f704e99d5fb0d3f android.hidl.manager@1.0::IServiceManager
+50552b700ef67c7ed8c8d776d323f8c629a9b43b965e10a916a66f3c946c50fb android.hidl.manager@1.0::IServiceNotification
+2b885b5dec97391c82f35e64180686dc4c8f78b2b0a01732f8536385654f27c8 android.hidl.memory@1.0::IMapper
+4632246017013e75536fa6ee47db286b24a323fb92c37c6b14bb0ab796b7a16b android.hidl.memory@1.0::IMemory
+7c9fe352af04af659bd51ab6f5495115575bc063ddf684fc6d0dec1f4a4b4b7c android.hidl.token@1.0::ITokenManager
diff --git a/transport/include/hidl/HidlTransportSupport.h b/transport/include/hidl/HidlTransportSupport.h
index 3cac1e9..0c174f7 100644
--- a/transport/include/hidl/HidlTransportSupport.h
+++ b/transport/include/hidl/HidlTransportSupport.h
@@ -17,6 +17,7 @@
#ifndef ANDROID_HIDL_TRANSPORT_SUPPORT_H
#define ANDROID_HIDL_TRANSPORT_SUPPORT_H
+#include <android/hidl/base/1.0/IBase.h>
#include <hidl/HidlBinderSupport.h>
#include <hidl/HidlSupport.h>
#include <hidl/HidlTransportUtils.h>
@@ -47,6 +48,20 @@
*/
void joinRpcThreadpool();
+/**
+ * Sets a minimum scheduler policy for all transactions coming into this
+ * service.
+ *
+ * This method MUST be called before passing this service to another process
+ * and/or registering it with registerAsService().
+ *
+ * @param service the service to set the policy for
+ * @param policy scheduler policy as defined in linux UAPI
+ * @param priority priority. [-20..19] for SCHED_NORMAL, [1..99] for RT
+ */
+bool setMinSchedulerPolicy(const sp<::android::hidl::base::V1_0::IBase>& service,
+ int policy, int priority);
+
namespace details {
// cast the interface IParent to IChild.
diff --git a/transport/include/hidl/Static.h b/transport/include/hidl/Static.h
index e6c9139..0133ff7 100644
--- a/transport/include/hidl/Static.h
+++ b/transport/include/hidl/Static.h
@@ -28,12 +28,19 @@
namespace hardware {
namespace details {
+struct SchedPrio {
+ int sched_policy;
+ int prio;
+};
+
// For HidlBinderSupport and autogenerated code
// value function receives reinterpret_cast<void *>(static_cast<IFoo *>(foo)),
// returns sp<IBinder>
extern ConcurrentMap<std::string,
std::function<sp<IBinder>(void *)>> gBnConstructorMap;
+extern ConcurrentMap<wp<::android::hidl::base::V1_0::IBase>, SchedPrio> gServicePrioMap;
+
// For HidlPassthroughSupport and autogenerated code
// value function receives reinterpret_cast<void *>(static_cast<IFoo *>(foo)),
// returns sp<IBase>