Camera: Fix deadlock caused by mBatchLock
Both getBufferLocked and returnBufferLocked() could be acquiring the
mBatchLock, resulting in deadlock.
Test: Camera CTS, Vendor testing
Bug: 193204736
Change-Id: I4f4c97995136f326597ee996d4f84e2b34187d1a
diff --git a/services/camera/libcameraservice/device3/Camera3OutputStream.cpp b/services/camera/libcameraservice/device3/Camera3OutputStream.cpp
index 225dee9..cd90b51 100644
--- a/services/camera/libcameraservice/device3/Camera3OutputStream.cpp
+++ b/services/camera/libcameraservice/device3/Camera3OutputStream.cpp
@@ -657,17 +657,17 @@
size_t remainingBuffers = (mState == STATE_PREPARING ? mTotalBufferCount :
camera_stream::max_buffers) - mHandoutTotalBufferCount;
mLock.unlock();
- std::unique_lock<std::mutex> batchLock(mBatchLock);
nsecs_t dequeueStart = systemTime(SYSTEM_TIME_MONOTONIC);
- if (mBatchSize == 1) {
+ size_t batchSize = mBatchSize.load();
+ if (batchSize == 1) {
sp<ANativeWindow> anw = consumer;
res = anw->dequeueBuffer(anw.get(), anb, fenceFd);
} else {
+ std::unique_lock<std::mutex> batchLock(mBatchLock);
res = OK;
if (mBatchedBuffers.size() == 0) {
- size_t batchSize = mBatchSize;
if (remainingBuffers == 0) {
ALOGE("%s: cannot get buffer while all buffers are handed out", __FUNCTION__);
return INVALID_OPERATION;
@@ -675,13 +675,17 @@
if (batchSize > remainingBuffers) {
batchSize = remainingBuffers;
}
+ batchLock.unlock();
// Refill batched buffers
- mBatchedBuffers.resize(batchSize);
- res = consumer->dequeueBuffers(&mBatchedBuffers);
+ std::vector<Surface::BatchBuffer> batchedBuffers;
+ batchedBuffers.resize(batchSize);
+ res = consumer->dequeueBuffers(&batchedBuffers);
+ batchLock.lock();
if (res != OK) {
ALOGE("%s: batch dequeueBuffers call failed! %s (%d)",
__FUNCTION__, strerror(-res), res);
- mBatchedBuffers.clear();
+ } else {
+ mBatchedBuffers = std::move(batchedBuffers);
}
}
@@ -692,7 +696,6 @@
mBatchedBuffers.pop_back();
}
}
- batchLock.unlock();
nsecs_t dequeueEnd = systemTime(SYSTEM_TIME_MONOTONIC);
mDequeueBufferLatency.add(dequeueStart, dequeueEnd);
@@ -1129,7 +1132,6 @@
status_t Camera3OutputStream::setBatchSize(size_t batchSize) {
Mutex::Autolock l(mLock);
- std::lock_guard<std::mutex> lock(mBatchLock);
if (batchSize == 0) {
ALOGE("%s: invalid batch size 0", __FUNCTION__);
return BAD_VALUE;
@@ -1145,31 +1147,36 @@
return INVALID_OPERATION;
}
- if (batchSize != mBatchSize) {
- if (mBatchedBuffers.size() != 0) {
- ALOGE("%s: change batch size from %zu to %zu dynamically is not supported",
- __FUNCTION__, mBatchSize, batchSize);
- return INVALID_OPERATION;
- }
-
- if (camera_stream::max_buffers < batchSize) {
- ALOGW("%s: batch size is capped by max_buffers %d", __FUNCTION__,
- camera_stream::max_buffers);
- batchSize = camera_stream::max_buffers;
- }
- mBatchSize = batchSize;
+ if (camera_stream::max_buffers < batchSize) {
+ ALOGW("%s: batch size is capped by max_buffers %d", __FUNCTION__,
+ camera_stream::max_buffers);
+ batchSize = camera_stream::max_buffers;
}
+
+ size_t defaultBatchSize = 1;
+ if (!mBatchSize.compare_exchange_strong(defaultBatchSize, batchSize)) {
+ ALOGE("%s: change batch size from %zu to %zu dynamically is not supported",
+ __FUNCTION__, defaultBatchSize, batchSize);
+ return INVALID_OPERATION;
+ }
+
return OK;
}
void Camera3OutputStream::returnPrefetchedBuffersLocked() {
- std::lock_guard<std::mutex> batchLock(mBatchLock);
- if (mBatchedBuffers.size() != 0) {
- ALOGW("%s: %zu extra prefetched buffers detected. Returning",
- __FUNCTION__, mBatchedBuffers.size());
+ std::vector<Surface::BatchBuffer> batchedBuffers;
- mConsumer->cancelBuffers(mBatchedBuffers);
- mBatchedBuffers.clear();
+ {
+ std::lock_guard<std::mutex> batchLock(mBatchLock);
+ if (mBatchedBuffers.size() != 0) {
+ ALOGW("%s: %zu extra prefetched buffers detected. Returning",
+ __FUNCTION__, mBatchedBuffers.size());
+ batchedBuffers = std::move(mBatchedBuffers);
+ }
+ }
+
+ if (batchedBuffers.size() > 0) {
+ mConsumer->cancelBuffers(batchedBuffers);
}
}
diff --git a/services/camera/libcameraservice/device3/Camera3OutputStream.h b/services/camera/libcameraservice/device3/Camera3OutputStream.h
index 00e4854..ad03b53 100644
--- a/services/camera/libcameraservice/device3/Camera3OutputStream.h
+++ b/services/camera/libcameraservice/device3/Camera3OutputStream.h
@@ -312,15 +312,14 @@
bool mDropBuffers;
- // Protecting batch states below, must be acquired after mLock
- std::mutex mBatchLock;
// The batch size for buffer operation
- size_t mBatchSize = 1;
+ std::atomic_size_t mBatchSize = 1;
+ // Protecting batch states below, must be acquired after mLock
+ std::mutex mBatchLock;
// Prefetched buffers (ready to be handed to client)
std::vector<Surface::BatchBuffer> mBatchedBuffers;
-
// ---- End of mBatchLock protected scope ----
/**