Camera: fix bufferFreed callback object lifecycle issue
Make sure the callback object won't be freed in the middle
of callback execution.
Test: CTS + stress test
Bug: 63683767
Change-Id: I6fb1b754cadb3d499c1c246687d2f60d444d00bb
diff --git a/services/camera/libcameraservice/device3/Camera3Device.cpp b/services/camera/libcameraservice/device3/Camera3Device.cpp
index beec1f4..0807c0a 100644
--- a/services/camera/libcameraservice/device3/Camera3Device.cpp
+++ b/services/camera/libcameraservice/device3/Camera3Device.cpp
@@ -155,7 +155,7 @@
return DEAD_OBJECT;
}
- mInterface = std::make_unique<HalInterface>(session, queue);
+ mInterface = new HalInterface(session, queue);
std::string providerType;
mVendorTagId = manager->getProviderTagIdLocked(mId.string());
@@ -184,7 +184,7 @@
mTagMonitor.initialize(mVendorTagId);
/** Start up request queue thread */
- mRequestThread = new RequestThread(this, mStatusTracker, mInterface.get());
+ mRequestThread = new RequestThread(this, mStatusTracker, mInterface);
res = mRequestThread->run(String8::format("C3Dev-%s-ReqQueue", mId.string()).string());
if (res != OK) {
SET_ERR_L("Unable to start request queue thread: %s (%d)",
@@ -3515,7 +3515,7 @@
Camera3Device::RequestThread::RequestThread(wp<Camera3Device> parent,
sp<StatusTracker> statusTracker,
- HalInterface* interface) :
+ sp<HalInterface> interface) :
Thread(/*canCallJava*/false),
mParent(parent),
mStatusTracker(statusTracker),
diff --git a/services/camera/libcameraservice/device3/Camera3Device.h b/services/camera/libcameraservice/device3/Camera3Device.h
index fb46d7e..0251d62 100644
--- a/services/camera/libcameraservice/device3/Camera3Device.h
+++ b/services/camera/libcameraservice/device3/Camera3Device.h
@@ -337,7 +337,7 @@
std::vector<std::pair<int, uint64_t>> mFreedBuffers;
};
- std::unique_ptr<HalInterface> mInterface;
+ sp<HalInterface> mInterface;
CameraMetadata mDeviceInfo;
@@ -628,7 +628,7 @@
RequestThread(wp<Camera3Device> parent,
sp<camera3::StatusTracker> statusTracker,
- HalInterface* interface);
+ sp<HalInterface> interface);
~RequestThread();
void setNotificationListener(wp<NotificationListener> listener);
@@ -778,7 +778,7 @@
wp<Camera3Device> mParent;
wp<camera3::StatusTracker> mStatusTracker;
- HalInterface* mInterface;
+ sp<HalInterface> mInterface;
wp<NotificationListener> mListener;
diff --git a/services/camera/libcameraservice/device3/Camera3InputStream.cpp b/services/camera/libcameraservice/device3/Camera3InputStream.cpp
index 4eb15ad..35096eb 100644
--- a/services/camera/libcameraservice/device3/Camera3InputStream.cpp
+++ b/services/camera/libcameraservice/device3/Camera3InputStream.cpp
@@ -293,8 +293,9 @@
void Camera3InputStream::onBufferFreed(const wp<GraphicBuffer>& gb) {
const sp<GraphicBuffer> buffer = gb.promote();
if (buffer != nullptr) {
- if (mBufferFreedListener != nullptr) {
- mBufferFreedListener->onBufferFreed(mId, buffer->handle);
+ sp<Camera3StreamBufferFreedListener> callback = mBufferFreedListener.promote();
+ if (callback != nullptr) {
+ callback->onBufferFreed(mId, buffer->handle);
}
} else {
ALOGE("%s: GraphicBuffer is freed before onBufferFreed callback finishes!", __FUNCTION__);
diff --git a/services/camera/libcameraservice/device3/Camera3OutputStream.cpp b/services/camera/libcameraservice/device3/Camera3OutputStream.cpp
index e15aa43..b02cd6a 100644
--- a/services/camera/libcameraservice/device3/Camera3OutputStream.cpp
+++ b/services/camera/libcameraservice/device3/Camera3OutputStream.cpp
@@ -727,7 +727,7 @@
void Camera3OutputStream::onBuffersRemovedLocked(
const std::vector<sp<GraphicBuffer>>& removedBuffers) {
- Camera3StreamBufferFreedListener* callback = mBufferFreedListener;
+ sp<Camera3StreamBufferFreedListener> callback = mBufferFreedListener.promote();
if (callback != nullptr) {
for (auto gb : removedBuffers) {
callback->onBufferFreed(mId, gb->handle);
diff --git a/services/camera/libcameraservice/device3/Camera3Stream.cpp b/services/camera/libcameraservice/device3/Camera3Stream.cpp
index 9297ac8..438a266 100644
--- a/services/camera/libcameraservice/device3/Camera3Stream.cpp
+++ b/services/camera/libcameraservice/device3/Camera3Stream.cpp
@@ -744,7 +744,7 @@
}
void Camera3Stream::setBufferFreedListener(
- Camera3StreamBufferFreedListener* listener) {
+ wp<Camera3StreamBufferFreedListener> listener) {
Mutex::Autolock l(mLock);
// Only allow set listener during stream configuration because stream is guaranteed to be IDLE
// at this state, so setBufferFreedListener won't collide with onBufferFreed callbacks
diff --git a/services/camera/libcameraservice/device3/Camera3Stream.h b/services/camera/libcameraservice/device3/Camera3Stream.h
index b6c8396..0940d62 100644
--- a/services/camera/libcameraservice/device3/Camera3Stream.h
+++ b/services/camera/libcameraservice/device3/Camera3Stream.h
@@ -371,7 +371,7 @@
// Setting listener will remove previous listener (if exists)
virtual void setBufferFreedListener(
- Camera3StreamBufferFreedListener* listener) override;
+ wp<Camera3StreamBufferFreedListener> listener) override;
/**
* Return if the buffer queue of the stream is abandoned.
@@ -416,7 +416,7 @@
android_dataspace dataSpace, camera3_stream_rotation_t rotation,
int setId);
- Camera3StreamBufferFreedListener* mBufferFreedListener;
+ wp<Camera3StreamBufferFreedListener> mBufferFreedListener;
/**
* Interface to be implemented by derived classes
diff --git a/services/camera/libcameraservice/device3/Camera3StreamBufferFreedListener.h b/services/camera/libcameraservice/device3/Camera3StreamBufferFreedListener.h
index 478a752..104cd22 100644
--- a/services/camera/libcameraservice/device3/Camera3StreamBufferFreedListener.h
+++ b/services/camera/libcameraservice/device3/Camera3StreamBufferFreedListener.h
@@ -24,7 +24,7 @@
namespace camera3 {
-class Camera3StreamBufferFreedListener {
+class Camera3StreamBufferFreedListener : public virtual RefBase {
public:
// onBufferFreed is called when a buffer is no longer being managed
// by this stream. This will not be called in events when all
diff --git a/services/camera/libcameraservice/device3/Camera3StreamInterface.h b/services/camera/libcameraservice/device3/Camera3StreamInterface.h
index c695a10..0544a1b 100644
--- a/services/camera/libcameraservice/device3/Camera3StreamInterface.h
+++ b/services/camera/libcameraservice/device3/Camera3StreamInterface.h
@@ -298,7 +298,7 @@
* Client is responsible to keep the listener object alive throughout the lifecycle of this
* Camera3Stream.
*/
- virtual void setBufferFreedListener(Camera3StreamBufferFreedListener* listener) = 0;
+ virtual void setBufferFreedListener(wp<Camera3StreamBufferFreedListener> listener) = 0;
};
} // namespace camera3