gralloc1: Locking fixes
* Separate buffer lock and descriptor lock
* Add lock for allocation as we touch the handles_map
Change-Id: I2baf7a65f55b04f1bbbfbf78a19c0e288040fab7
CRs-Fixed: 2037173
diff --git a/libgralloc1/gr_buf_mgr.cpp b/libgralloc1/gr_buf_mgr.cpp
index 66a8e17..e0d44e7 100644
--- a/libgralloc1/gr_buf_mgr.cpp
+++ b/libgralloc1/gr_buf_mgr.cpp
@@ -64,7 +64,7 @@
gralloc1_error_t BufferManager::CreateBufferDescriptor(
gralloc1_buffer_descriptor_t *descriptor_id) {
- std::lock_guard<std::mutex> lock(locker_);
+ std::lock_guard<std::mutex> lock(descriptor_lock_);
auto descriptor = std::make_shared<BufferDescriptor>();
descriptors_map_.emplace(descriptor->GetId(), descriptor);
*descriptor_id = descriptor->GetId();
@@ -73,7 +73,7 @@
gralloc1_error_t BufferManager::DestroyBufferDescriptor(
gralloc1_buffer_descriptor_t descriptor_id) {
- std::lock_guard<std::mutex> lock(locker_);
+ std::lock_guard<std::mutex> lock(descriptor_lock_);
const auto descriptor = descriptors_map_.find(descriptor_id);
if (descriptor == descriptors_map_.end()) {
return GRALLOC1_ERROR_BAD_DESCRIPTOR;
@@ -99,6 +99,7 @@
bool test_allocate = !out_buffers;
// Validate descriptors
+ std::lock_guard<std::mutex> descriptor_lock(descriptor_lock_);
std::vector<std::shared_ptr<BufferDescriptor>> descriptors;
for (uint32_t i = 0; i < num_descriptors; i++) {
const auto map_descriptor = descriptors_map_.find(descriptor_ids[i]);
@@ -127,6 +128,7 @@
return status;
}
+ std::lock_guard<std::mutex> buffer_lock(buffer_lock_);
if (shared && (max_buf_index >= 0)) {
// Allocate one and duplicate/copy the handles for each descriptor
if (AllocateBuffer(*descriptors[UINT(max_buf_index)], &out_buffers[max_buf_index])) {
@@ -191,7 +193,7 @@
descriptor.GetConsumerUsage());
out_hnd->id = ++next_id_;
// TODO(user): Base address of shared handle and ion handles
- RegisterHandle(out_hnd, -1, -1);
+ RegisterHandleLocked(out_hnd, -1, -1);
*outbuffer = out_hnd;
}
@@ -217,14 +219,14 @@
return GRALLOC1_ERROR_NONE;
}
-void BufferManager::RegisterHandle(const private_handle_t *hnd,
- int ion_handle,
- int ion_handle_meta) {
+void BufferManager::RegisterHandleLocked(const private_handle_t *hnd,
+ int ion_handle,
+ int ion_handle_meta) {
auto buffer = std::make_shared<Buffer>(hnd, ion_handle, ion_handle_meta);
handles_map_.emplace(std::make_pair(hnd, buffer));
}
-gralloc1_error_t BufferManager::ImportHandle(private_handle_t* hnd) {
+gralloc1_error_t BufferManager::ImportHandleLocked(private_handle_t *hnd) {
ALOGD_IF(DEBUG, "Importing handle:%p id: %" PRIu64, hnd, hnd->id);
int ion_handle = allocator_->ImportBuffer(hnd->fd);
if (ion_handle < 0) {
@@ -240,12 +242,12 @@
// Set base pointers to NULL since the data here was received over binder
hnd->base = 0;
hnd->base_metadata = 0;
- RegisterHandle(hnd, ion_handle, ion_handle_meta);
+ RegisterHandleLocked(hnd, ion_handle, ion_handle_meta);
return GRALLOC1_ERROR_NONE;
}
std::shared_ptr<BufferManager::Buffer>
-BufferManager::GetBufferFromHandle(const private_handle_t *hnd) {
+BufferManager::GetBufferFromHandleLocked(const private_handle_t *hnd) {
auto it = handles_map_.find(hnd);
if (it != handles_map_.end()) {
return it->second;
@@ -276,15 +278,15 @@
}
gralloc1_error_t BufferManager::RetainBuffer(private_handle_t const *hnd) {
- std::lock_guard<std::mutex> lock(locker_);
ALOGD_IF(DEBUG, "Retain buffer handle:%p id: %" PRIu64, hnd, hnd->id);
gralloc1_error_t err = GRALLOC1_ERROR_NONE;
- auto buf = GetBufferFromHandle(hnd);
+ std::lock_guard<std::mutex> lock(buffer_lock_);
+ auto buf = GetBufferFromHandleLocked(hnd);
if (buf != nullptr) {
buf->IncRef();
} else {
private_handle_t *handle = const_cast<private_handle_t *>(hnd);
- err = ImportHandle(handle);
+ err = ImportHandleLocked(handle);
if (err == GRALLOC1_ERROR_NONE) {
// TODO(user): See bug 35955598
if (hnd->flags & private_handle_t::PRIV_FLAGS_SECURE_BUFFER) {
@@ -297,9 +299,9 @@
}
gralloc1_error_t BufferManager::ReleaseBuffer(private_handle_t const *hnd) {
- std::lock_guard<std::mutex> lock(locker_);
ALOGD_IF(DEBUG, "Release buffer handle:%p id: %" PRIu64, hnd, hnd->id);
- auto buf = GetBufferFromHandle(hnd);
+ std::lock_guard<std::mutex> lock(buffer_lock_);
+ auto buf = GetBufferFromHandleLocked(hnd);
if (buf == nullptr) {
ALOGE("Could not find handle: %p id: %" PRIu64, hnd, hnd->id);
return GRALLOC1_ERROR_BAD_HANDLE;
@@ -316,7 +318,7 @@
gralloc1_error_t BufferManager::LockBuffer(const private_handle_t *hnd,
gralloc1_producer_usage_t prod_usage,
gralloc1_consumer_usage_t cons_usage) {
- std::lock_guard<std::mutex> lock(locker_);
+ std::lock_guard<std::mutex> lock(buffer_lock_);
gralloc1_error_t err = GRALLOC1_ERROR_NONE;
ALOGD_IF(DEBUG, "LockBuffer buffer handle:%p id: %" PRIu64, hnd, hnd->id);
@@ -330,7 +332,7 @@
err = MapBuffer(hnd);
}
- auto buf = GetBufferFromHandle(hnd);
+ auto buf = GetBufferFromHandleLocked(hnd);
if (buf == nullptr) {
return GRALLOC1_ERROR_BAD_HANDLE;
}
@@ -358,11 +360,11 @@
}
gralloc1_error_t BufferManager::UnlockBuffer(const private_handle_t *handle) {
- std::lock_guard<std::mutex> lock(locker_);
+ std::lock_guard<std::mutex> lock(buffer_lock_);
gralloc1_error_t status = GRALLOC1_ERROR_NONE;
private_handle_t *hnd = const_cast<private_handle_t *>(handle);
- auto buf = GetBufferFromHandle(hnd);
+ auto buf = GetBufferFromHandleLocked(hnd);
if (buf == nullptr) {
return GRALLOC1_ERROR_BAD_HANDLE;
}
@@ -544,7 +546,7 @@
ColorSpace_t colorSpace = ITU_R_601;
setMetaData(hnd, UPDATE_COLOR_SPACE, reinterpret_cast<void *>(&colorSpace));
*handle = hnd;
- RegisterHandle(hnd, data.ion_handle, e_data.ion_handle);
+ RegisterHandleLocked(hnd, data.ion_handle, e_data.ion_handle);
ALOGD_IF(DEBUG, "Allocated buffer handle: %p id: %" PRIu64, hnd, hnd->id);
if (DEBUG) {
private_handle_t::Dump(hnd);
diff --git a/libgralloc1/gr_buf_mgr.h b/libgralloc1/gr_buf_mgr.h
index 738385d..4476eaf 100644
--- a/libgralloc1/gr_buf_mgr.h
+++ b/libgralloc1/gr_buf_mgr.h
@@ -54,7 +54,7 @@
gralloc1_error_t CallBufferDescriptorFunction(gralloc1_buffer_descriptor_t descriptor_id,
void (BufferDescriptor::*member)(Args...),
Args... args) {
- std::lock_guard<std::mutex> lock(locker_);
+ std::lock_guard<std::mutex> lock(descriptor_lock_);
const auto map_descriptor = descriptors_map_.find(descriptor_id);
if (map_descriptor == descriptors_map_.end()) {
return GRALLOC1_ERROR_BAD_DESCRIPTOR;
@@ -83,10 +83,10 @@
buffer_handle_t *out_buffer);
// Imports the ion fds into the current process. Returns an error for invalid handles
- gralloc1_error_t ImportHandle(private_handle_t* hnd);
+ gralloc1_error_t ImportHandleLocked(private_handle_t *hnd);
// Creates a Buffer from the valid private handle and adds it to the map
- void RegisterHandle(const private_handle_t *hnd, int ion_handle, int ion_handle_meta);
+ void RegisterHandleLocked(const private_handle_t *hnd, int ion_handle, int ion_handle_meta);
// Wrapper structure over private handle
// Values associated with the private handle
@@ -115,12 +115,13 @@
gralloc1_error_t FreeBuffer(std::shared_ptr<Buffer> buf);
// Get the wrapper Buffer object from the handle, returns nullptr if handle is not found
- std::shared_ptr<Buffer> GetBufferFromHandle(const private_handle_t *hnd);
+ std::shared_ptr<Buffer> GetBufferFromHandleLocked(const private_handle_t *hnd);
bool map_fb_mem_ = false;
bool ubwc_for_fb_ = false;
Allocator *allocator_ = NULL;
- std::mutex locker_;
+ std::mutex buffer_lock_;
+ std::mutex descriptor_lock_;
// TODO(user): The private_handle_t is used as a key because the unique ID generated
// from next_id_ is not unique across processes. The correct way to resolve this would
// be to use the allocator over hwbinder