Fix crash when virtual camera is unregistered during active session.
Store weak_ptr to VirtualCameraDevice instance in VirtualCameraSession
to detect when the camera was removed as well as to make sure
it's not destroyed when VirtualCameraSession accesses
VirtualCameraDevice.
Bug: 301023410
Test: atest VirtualCameraTest (not closing the session)
Test: atest virtual_camera_tests
Change-Id: Ie87c91f6872b9241dddf64f1a4f1991eb54d3cda
diff --git a/services/camera/virtualcamera/VirtualCameraDevice.cc b/services/camera/virtualcamera/VirtualCameraDevice.cc
index ec72ee3..0657d81 100644
--- a/services/camera/virtualcamera/VirtualCameraDevice.cc
+++ b/services/camera/virtualcamera/VirtualCameraDevice.cc
@@ -322,7 +322,7 @@
ALOGV("%s", __func__);
*_aidl_return = ndk::SharedRefBase::make<VirtualCameraSession>(
- *this, in_callback, mVirtualCameraClientCallback);
+ sharedFromThis(), in_callback, mVirtualCameraClientCallback);
return ndk::ScopedAStatus::ok();
};
@@ -367,6 +367,13 @@
return std::string(kDevicePathPrefix) + std::to_string(mCameraId);
}
+std::shared_ptr<VirtualCameraDevice> VirtualCameraDevice::sharedFromThis() {
+ // SharedRefBase which BnCameraDevice inherits from breaks
+ // std::enable_shared_from_this. This is recommended replacement for
+ // shared_from_this() per documentation in binder_interface_utils.h.
+ return ref<VirtualCameraDevice>();
+}
+
} // namespace virtualcamera
} // namespace companion
} // namespace android
diff --git a/services/camera/virtualcamera/VirtualCameraDevice.h b/services/camera/virtualcamera/VirtualCameraDevice.h
index ee7a3a7..402de6c 100644
--- a/services/camera/virtualcamera/VirtualCameraDevice.h
+++ b/services/camera/virtualcamera/VirtualCameraDevice.h
@@ -98,6 +98,8 @@
uint32_t getCameraId() const { return mCameraId; }
private:
+ std::shared_ptr<VirtualCameraDevice> sharedFromThis();
+
const uint32_t mCameraId;
const std::shared_ptr<
::aidl::android::companion::virtualcamera::IVirtualCameraCallback>
diff --git a/services/camera/virtualcamera/VirtualCameraSession.cc b/services/camera/virtualcamera/VirtualCameraSession.cc
index 9e15871..47780d8 100644
--- a/services/camera/virtualcamera/VirtualCameraSession.cc
+++ b/services/camera/virtualcamera/VirtualCameraSession.cc
@@ -153,7 +153,7 @@
} // namespace
VirtualCameraSession::VirtualCameraSession(
- VirtualCameraDevice& cameraDevice,
+ std::shared_ptr<VirtualCameraDevice> cameraDevice,
std::shared_ptr<ICameraDeviceCallback> cameraDeviceCallback,
std::shared_ptr<IVirtualCameraCallback> virtualCameraClientCallback)
: mCameraDevice(cameraDevice),
@@ -201,6 +201,12 @@
return cameraStatus(Status::ILLEGAL_ARGUMENT);
}
+ std::shared_ptr<VirtualCameraDevice> virtualCamera = mCameraDevice.lock();
+ if (virtualCamera == nullptr) {
+ ALOGW("%s: configure called on already unregistered camera", __func__);
+ return cameraStatus(Status::CAMERA_DISCONNECTED);
+ }
+
mSessionContext.removeStreamsNotInStreamConfiguration(
in_requestedConfiguration);
@@ -213,7 +219,7 @@
int inputWidth;
int inputHeight;
- if (!mCameraDevice.isStreamCombinationSupported(in_requestedConfiguration)) {
+ if (!virtualCamera->isStreamCombinationSupported(in_requestedConfiguration)) {
ALOGE("%s: Requested stream configuration is not supported", __func__);
return cameraStatus(Status::ILLEGAL_ARGUMENT);
}
diff --git a/services/camera/virtualcamera/VirtualCameraSession.h b/services/camera/virtualcamera/VirtualCameraSession.h
index 50962e5..82a7a34 100644
--- a/services/camera/virtualcamera/VirtualCameraSession.h
+++ b/services/camera/virtualcamera/VirtualCameraSession.h
@@ -17,6 +17,7 @@
#ifndef ANDROID_COMPANION_VIRTUALCAMERA_VIRTUALCAMERASESSION_H
#define ANDROID_COMPANION_VIRTUALCAMERA_VIRTUALCAMERASESSION_H
+#include <atomic>
#include <memory>
#include <set>
@@ -46,7 +47,7 @@
// When virtualCameraClientCallback is null, the input surface will be filled
// with test pattern.
VirtualCameraSession(
- VirtualCameraDevice& mCameraDevice,
+ std::shared_ptr<VirtualCameraDevice> mCameraDevice,
std::shared_ptr<
::aidl::android::hardware::camera::device::ICameraDeviceCallback>
cameraDeviceCallback,
@@ -116,7 +117,7 @@
const ::aidl::android::hardware::camera::device::CaptureRequest& request)
EXCLUDES(mLock);
- VirtualCameraDevice& mCameraDevice;
+ std::weak_ptr<VirtualCameraDevice> mCameraDevice;
mutable std::mutex mLock;
diff --git a/services/camera/virtualcamera/tests/VirtualCameraSessionTest.cc b/services/camera/virtualcamera/tests/VirtualCameraSessionTest.cc
index 9edfe77..30bd2b6 100644
--- a/services/camera/virtualcamera/tests/VirtualCameraSessionTest.cc
+++ b/services/camera/virtualcamera/tests/VirtualCameraSessionTest.cc
@@ -20,8 +20,8 @@
#include "VirtualCameraDevice.h"
#include "VirtualCameraSession.h"
#include "aidl/android/companion/virtualcamera/BnVirtualCameraCallback.h"
-#include "aidl/android/companion/virtualcamera/IVirtualCameraCallback.h"
#include "aidl/android/companion/virtualcamera/SupportedStreamConfiguration.h"
+#include "aidl/android/hardware/camera/common/Status.h"
#include "aidl/android/hardware/camera/device/BnCameraDeviceCallback.h"
#include "aidl/android/hardware/camera/device/StreamConfiguration.h"
#include "aidl/android/hardware/graphics/common/PixelFormat.h"
@@ -44,6 +44,7 @@
using ::aidl::android::companion::virtualcamera::BnVirtualCameraCallback;
using ::aidl::android::companion::virtualcamera::Format;
using ::aidl::android::companion::virtualcamera::SupportedStreamConfiguration;
+using ::aidl::android::hardware::camera::common::Status;
using ::aidl::android::hardware::camera::device::BnCameraDeviceCallback;
using ::aidl::android::hardware::camera::device::BufferRequest;
using ::aidl::android::hardware::camera::device::BufferRequestStatus;
@@ -110,7 +111,7 @@
.pixelFormat = Format::YUV_420_888}},
mMockVirtualCameraClientCallback);
mVirtualCameraSession = ndk::SharedRefBase::make<VirtualCameraSession>(
- *mVirtualCameraDevice, mMockCameraDeviceCallback,
+ mVirtualCameraDevice, mMockCameraDeviceCallback,
mMockVirtualCameraClientCallback);
// Explicitly defining default actions below to prevent gmock from
@@ -223,6 +224,22 @@
EXPECT_THAT(aidlReturn, Eq(requests.size()));
}
+TEST_F(VirtualCameraSessionTest, configureAfterCameraRelease) {
+ StreamConfiguration streamConfiguration;
+ streamConfiguration.streams = {
+ createStream(kStreamId, kWidth, kHeight, PixelFormat::YCBCR_420_888)};
+ std::vector<HalStream> halStreams;
+
+ // Release virtual camera.
+ mVirtualCameraDevice.reset();
+
+ // Expect configuration attempt returns CAMERA_DISCONNECTED service specific code.
+ EXPECT_THAT(
+ mVirtualCameraSession->configureStreams(streamConfiguration, &halStreams)
+ .getServiceSpecificError(),
+ Eq(static_cast<int32_t>(Status::CAMERA_DISCONNECTED)));
+}
+
} // namespace
} // namespace virtualcamera
} // namespace companion