camera: add a lock for the request tracker
The request tracker manages "in flights" buffers and frames. These
two containers are modified by calling the tracker's member functions.
Such functions are called from at least three threads: (see
`v4l2_camera.cpp` for more details):
- "main" thread;
- separate buffer enqueue thread ("Enqueue buffers");
- separate buffer dequeue thread ("Dequeue buffers").
It is proposed to introduce additional mutex for the request tracker
to prevent a race condition.
An alternative approach is introducing an additional mutex for the
shared containers inside the `RequresTracker` class. But such approach
requires more changes and may lead to problems with design in the
future.
This commit contains a few additional changes which are not directly
related to the topic:
- as far as more than one local lock is used all of them are renamed;
- the `mFlushLock` member is removed because it is completely covered
by the new mutex.
Test: try to use a camera with more than one buffer and get SIGSEGV
crash of the `cameraserver` in the `RequestTracker` class.
Change-Id: I2c1901722289cee9f0cb05a11a4dabe70d18f2ba
Signed-off-by: Sergii Piatakov <sergii.piatakov@globallogic.com>
diff --git a/modules/camera/3_4/camera.cpp b/modules/camera/3_4/camera.cpp
index 387134f..3d2e225 100644
--- a/modules/camera/3_4/camera.cpp
+++ b/modules/camera/3_4/camera.cpp
@@ -76,7 +76,7 @@
{
ALOGI("%s:%d: Opening camera device", __func__, mId);
ATRACE_CALL();
- android::Mutex::Autolock al(mDeviceLock);
+ android::Mutex::Autolock dl(mDeviceLock);
if (mBusy) {
ALOGE("%s:%d: Error! Camera device already opened", __func__, mId);
@@ -113,7 +113,7 @@
int Camera::loadStaticInfo() {
// Using a lock here ensures |mStaticInfo| will only ever be set once,
// even in concurrent situations.
- android::Mutex::Autolock al(mStaticInfoLock);
+ android::Mutex::Autolock sl(mStaticInfoLock);
if (mStaticInfo) {
return 0;
@@ -143,7 +143,7 @@
{
ALOGI("%s:%d: Closing camera device", __func__, mId);
ATRACE_CALL();
- android::Mutex::Autolock al(mDeviceLock);
+ android::Mutex::Autolock dl(mDeviceLock);
if (!mBusy) {
ALOGE("%s:%d: Error! Camera device not open", __func__, mId);
@@ -173,7 +173,8 @@
int Camera::configureStreams(camera3_stream_configuration_t *stream_config)
{
- android::Mutex::Autolock al(mDeviceLock);
+ android::Mutex::Autolock dl(mDeviceLock);
+ android::Mutex::Autolock tl(mInFlightTrackerLock);
ALOGV("%s:%d: stream_config=%p", __func__, mId, stream_config);
ATRACE_CALL();
@@ -302,7 +303,7 @@
int res;
// TODO(b/32917568): A capture request submitted or ongoing during a flush
// should be returned with an error; for now they are mutually exclusive.
- android::Mutex::Autolock al(mFlushLock);
+ android::Mutex::Autolock tl(mInFlightTrackerLock);
ATRACE_CALL();
@@ -376,6 +377,8 @@
void Camera::completeRequest(std::shared_ptr<CaptureRequest> request, int err)
{
+ android::Mutex::Autolock tl(mInFlightTrackerLock);
+
if (!mInFlightTracker->Remove(request)) {
ALOGE("%s:%d: Completed request %p is not being tracked. "
"It may have been cleared out during a flush.",
@@ -424,7 +427,7 @@
// is called concurrently with this (in either order).
// Since the callback to completeRequest also may happen on a separate
// thread, this function should behave nicely concurrently with that too.
- android::Mutex::Autolock al(mFlushLock);
+ android::Mutex::Autolock tl(mInFlightTrackerLock);
std::set<std::shared_ptr<CaptureRequest>> requests;
mInFlightTracker->Clear(&requests);
@@ -517,7 +520,7 @@
{
ALOGV("%s:%d: Dumping to fd %d", __func__, mId, fd);
ATRACE_CALL();
- android::Mutex::Autolock al(mDeviceLock);
+ android::Mutex::Autolock dl(mDeviceLock);
dprintf(fd, "Camera ID: %d (Busy: %d)\n", mId, mBusy);