Fix multi-thread issues in StreamHandler
In current design, it is possible that the client stops a video stream
before the last analyze thread is scheduled. Because stopping a video
stream sets an analyzer callback as null, the analyze thread will
therefore be crashed as soon as it is scheduled.
To prevent this, this change adds a mutex to protect shared variables
and adds a nullcheck in a analyze thread.
Fix: 150255890
Test: m -j libevssupport and run face enrollment w/ user switching
Change-Id: I161be8d69f8116846f3863a6e60e491ccf1af2ca
diff --git a/evs/support_library/StreamHandler.cpp b/evs/support_library/StreamHandler.cpp
index 5b962cc..483f855 100644
--- a/evs/support_library/StreamHandler.cpp
+++ b/evs/support_library/StreamHandler.cpp
@@ -37,7 +37,9 @@
using ::std::unique_lock;
StreamHandler::StreamHandler(android::sp <IEvsCamera> pCamera) :
- mCamera(pCamera)
+ mCamera(pCamera),
+ mAnalyzeCallback(nullptr),
+ mAnalyzerRunning(false)
{
// We rely on the camera having at least two buffers available since we'll hold one and
// expect the camera to be able to capture a new image in the background.
@@ -174,8 +176,11 @@
// If analyze callback is not null and the analyze thread is
// available, copy the frame and run the analyze callback in
// analyze thread.
- if (mAnalyzeCallback != nullptr && !mAnalyzerRunning) {
- copyAndAnalyzeFrame(mOriginalBuffers[mReadyBuffer]);
+ {
+ std::shared_lock<std::shared_mutex> analyzerLock(mAnalyzerLock);
+ if (mAnalyzeCallback != nullptr && !mAnalyzerRunning) {
+ copyAndAnalyzeFrame(mOriginalBuffers[mReadyBuffer]);
+ }
}
}
}
@@ -209,20 +214,29 @@
void StreamHandler::attachAnalyzeCallback(BaseAnalyzeCallback* callback) {
ALOGD("StreamHandler::attachAnalyzeCallback");
- lock_guard<mutex> lock(mLock);
-
if (mAnalyzeCallback != nullptr) {
ALOGW("Ignored! There should only be one analyze callcack");
return;
}
- mAnalyzeCallback = callback;
+
+ {
+ lock_guard<std::shared_mutex> lock(mAnalyzerLock);
+ mAnalyzeCallback = callback;
+ }
}
void StreamHandler::detachAnalyzeCallback() {
ALOGD("StreamHandler::detachAnalyzeCallback");
- lock_guard<mutex> lock(mLock);
- mAnalyzeCallback = nullptr;
+ // Join a running analyzer thread
+ if (mAnalyzeThread.joinable()) {
+ mAnalyzeThread.join();
+ }
+
+ {
+ lock_guard<std::shared_mutex> lock(mAnalyzerLock);
+ mAnalyzeCallback = nullptr;
+ }
}
bool isSameFormat(const BufferDesc& input, const BufferDesc& output) {
@@ -453,12 +467,15 @@
mAnalyzeThread = std::thread([this, analyzeFrame]() {
ALOGD("StreamHandler: Analyze Thread starts");
- this->mAnalyzeCallback->analyze(analyzeFrame);
- android::GraphicBufferMapper::get().unlock(this->mAnalyzeBuffer.memHandle);
+ std::shared_lock<std::shared_mutex> lock(mAnalyzerLock);
+ if (this->mAnalyzeCallback != nullptr) {
+ this->mAnalyzeCallback->analyze(analyzeFrame);
+ android::GraphicBufferMapper::get().unlock(this->mAnalyzeBuffer.memHandle);
+ }
this->mAnalyzerRunning = false;
+
ALOGD("StreamHandler: Analyze Thread ends");
});
- mAnalyzeThread.detach();
return true;
}
diff --git a/evs/support_library/StreamHandler.h b/evs/support_library/StreamHandler.h
index 4899809..b962cb8 100644
--- a/evs/support_library/StreamHandler.h
+++ b/evs/support_library/StreamHandler.h
@@ -19,6 +19,7 @@
#include <queue>
#include <thread>
+#include <shared_mutex>
#include <ui/GraphicBuffer.h>
#include <android/hardware/automotive/evs/1.0/IEvsCameraStream.h>
#include <android/hardware/automotive/evs/1.0/IEvsCamera.h>
@@ -148,13 +149,14 @@
int mReadyBuffer = -1; // Index of the newest available buffer
BufferDesc mProcessedBuffers[2];
- BufferDesc mAnalyzeBuffer;
+ BufferDesc mAnalyzeBuffer GUARDED_BY(mAnalyzerLock);
BaseRenderCallback* mRenderCallback = nullptr;
- BaseAnalyzeCallback* mAnalyzeCallback = nullptr;
- bool mAnalyzerRunning = false;
+ BaseAnalyzeCallback* mAnalyzeCallback GUARDED_BY(mAnalyzerLock);
+ bool mAnalyzerRunning GUARDED_BY(mAnalyzerLock);
std::thread mAnalyzeThread;
+ std::shared_mutex mAnalyzerLock;
};
} // namespace support