Merge "Camera: Fix offline session close locking"
diff --git a/services/camera/libcameraservice/api2/CameraDeviceClient.cpp b/services/camera/libcameraservice/api2/CameraDeviceClient.cpp
index 2a8a103..b219cc8 100644
--- a/services/camera/libcameraservice/api2/CameraDeviceClient.cpp
+++ b/services/camera/libcameraservice/api2/CameraDeviceClient.cpp
@@ -1851,6 +1851,10 @@
         mCompositeStreamMap.clear();
         mInputStream = {false, 0, 0, 0, 0};
     } else {
+        // In case we failed to register the offline client, ensure that it still initialized
+        // so that all failing requests can return back correctly once the object is released.
+        offlineClient->initialize(nullptr /*cameraProviderManager*/, String8()/*monitorTags*/);
+
         switch(ret) {
             case BAD_VALUE:
                 return STATUS_ERROR_FMT(CameraService::ERROR_ILLEGAL_ARGUMENT,
diff --git a/services/camera/libcameraservice/api2/CameraOfflineSessionClient.cpp b/services/camera/libcameraservice/api2/CameraOfflineSessionClient.cpp
index beb655b..2cb3397 100644
--- a/services/camera/libcameraservice/api2/CameraOfflineSessionClient.cpp
+++ b/services/camera/libcameraservice/api2/CameraOfflineSessionClient.cpp
@@ -29,6 +29,11 @@
 status_t CameraOfflineSessionClient::initialize(sp<CameraProviderManager>, const String8&) {
     ATRACE_CALL();
 
+    if (mFrameProcessor.get() != nullptr) {
+        // Already initialized
+        return OK;
+    }
+
     // Verify ops permissions
     auto res = startCameraOps();
     if (res != OK) {
diff --git a/services/camera/libcameraservice/device3/Camera3OfflineSession.cpp b/services/camera/libcameraservice/device3/Camera3OfflineSession.cpp
index 7cfa255..1e7bd57 100644
--- a/services/camera/libcameraservice/device3/Camera3OfflineSession.cpp
+++ b/services/camera/libcameraservice/device3/Camera3OfflineSession.cpp
@@ -81,7 +81,6 @@
 Camera3OfflineSession::~Camera3OfflineSession() {
     ATRACE_CALL();
     ALOGV("%s: Tearing down offline session for camera id %s", __FUNCTION__, mId.string());
-    disconnectImpl();
 }
 
 const String8& Camera3OfflineSession::getId() const {
@@ -96,7 +95,6 @@
 
 status_t Camera3OfflineSession::disconnect() {
     ATRACE_CALL();
-    disconnectSession();
     return disconnectImpl();
 }
 
@@ -132,6 +130,8 @@
         streams.push_back(mInputStream);
     }
 
+    closeSessionLocked();
+
     FlushInflightReqStates states {
         mId, mOfflineReqsLock, mOfflineReqs, mUseHalBufManager,
         listener, *this, mBufferRecords, *this, mSessionStatsBuilder};
@@ -140,6 +140,7 @@
 
     {
         std::lock_guard<std::mutex> lock(mLock);
+        releaseSessionLocked();
         mOutputStreams.clear();
         mInputStream.clear();
         mStatus = STATUS_CLOSED;
diff --git a/services/camera/libcameraservice/device3/Camera3OfflineSession.h b/services/camera/libcameraservice/device3/Camera3OfflineSession.h
index 5ee6ca5..e780043 100644
--- a/services/camera/libcameraservice/device3/Camera3OfflineSession.h
+++ b/services/camera/libcameraservice/device3/Camera3OfflineSession.h
@@ -274,7 +274,12 @@
     void setErrorStateLockedV(const char *fmt, va_list args);
 
     status_t disconnectImpl();
-    virtual void disconnectSession() = 0;
+
+    // Clients need to ensure that 'mInterfaceLock' is acquired before calling this method
+    virtual void closeSessionLocked() = 0;
+
+    // Clients need to ensure that 'mLock' is acquired before calling this method
+    virtual void releaseSessionLocked() = 0;
 
 }; // class Camera3OfflineSession
 
diff --git a/services/camera/libcameraservice/device3/aidl/AidlCamera3OfflineSession.cpp b/services/camera/libcameraservice/device3/aidl/AidlCamera3OfflineSession.cpp
index 8ff0b07..affdda6 100644
--- a/services/camera/libcameraservice/device3/aidl/AidlCamera3OfflineSession.cpp
+++ b/services/camera/libcameraservice/device3/aidl/AidlCamera3OfflineSession.cpp
@@ -48,7 +48,7 @@
 AidlCamera3OfflineSession::~AidlCamera3OfflineSession() {
     ATRACE_CALL();
     ALOGV("%s: Tearing down aidl offline session for camera id %s", __FUNCTION__, mId.string());
-    AidlCamera3OfflineSession::disconnectSession();
+    Camera3OfflineSession::disconnectImpl();
 }
 
 status_t AidlCamera3OfflineSession::initialize(wp<NotificationListener> listener) {
@@ -245,12 +245,17 @@
     return ::ndk::ScopedAStatus::ok();
 }
 
-void AidlCamera3OfflineSession::disconnectSession() {
-  std::lock_guard<std::mutex> lock(mLock);
-  if (mSession != nullptr) {
-      mSession->close();
-  }
-  mSession.reset();
+void AidlCamera3OfflineSession::closeSessionLocked() {
+    if (mSession != nullptr) {
+        auto err = mSession->close();
+        if (!err.isOk()) {
+            ALOGE("%s: Close transaction error: %s", __FUNCTION__, err.getDescription().c_str());
+        }
+    }
+}
+
+void AidlCamera3OfflineSession::releaseSessionLocked() {
+    mSession.reset();
 }
 
 }; // namespace android
diff --git a/services/camera/libcameraservice/device3/aidl/AidlCamera3OfflineSession.h b/services/camera/libcameraservice/device3/aidl/AidlCamera3OfflineSession.h
index d107af6..b31ffb7 100644
--- a/services/camera/libcameraservice/device3/aidl/AidlCamera3OfflineSession.h
+++ b/services/camera/libcameraservice/device3/aidl/AidlCamera3OfflineSession.h
@@ -131,7 +131,9 @@
 
     std::shared_ptr<AidlCameraDeviceCallbacks> mCallbacks;
 
-    virtual void disconnectSession() override;
+    virtual void closeSessionLocked() override;
+
+    virtual void releaseSessionLocked() override;
 
 }; // class AidlCamera3OfflineSession
 
diff --git a/services/camera/libcameraservice/device3/hidl/HidlCamera3OfflineSession.cpp b/services/camera/libcameraservice/device3/hidl/HidlCamera3OfflineSession.cpp
index 2b4f8a1..de01ac9 100644
--- a/services/camera/libcameraservice/device3/hidl/HidlCamera3OfflineSession.cpp
+++ b/services/camera/libcameraservice/device3/hidl/HidlCamera3OfflineSession.cpp
@@ -39,7 +39,7 @@
 HidlCamera3OfflineSession::~HidlCamera3OfflineSession() {
     ATRACE_CALL();
     ALOGV("%s: Tearing down hidl offline session for camera id %s", __FUNCTION__, mId.string());
-    HidlCamera3OfflineSession::disconnectSession();
+    Camera3OfflineSession::disconnectImpl();
 }
 
 status_t HidlCamera3OfflineSession::initialize(wp<NotificationListener> listener) {
@@ -226,13 +226,17 @@
     return hardware::Void();
 }
 
-void HidlCamera3OfflineSession::disconnectSession() {
-  // TODO: Make sure this locking is correct.
-  std::lock_guard<std::mutex> lock(mLock);
-  if (mSession != nullptr) {
-      mSession->close();
-  }
-  mSession.clear();
+void HidlCamera3OfflineSession::closeSessionLocked() {
+    if (mSession != nullptr) {
+        auto err = mSession->close();
+        if (!err.isOk()) {
+            ALOGE("%s: Close transaction error: %s", __FUNCTION__, err.description().c_str());
+        }
+    }
+}
+
+void HidlCamera3OfflineSession::releaseSessionLocked() {
+    mSession.clear();
 }
 
 }; // namespace android
diff --git a/services/camera/libcameraservice/device3/hidl/HidlCamera3OfflineSession.h b/services/camera/libcameraservice/device3/hidl/HidlCamera3OfflineSession.h
index 597cc5d..d22a447 100644
--- a/services/camera/libcameraservice/device3/hidl/HidlCamera3OfflineSession.h
+++ b/services/camera/libcameraservice/device3/hidl/HidlCamera3OfflineSession.h
@@ -101,7 +101,9 @@
     // FMQ to write result on. Must be guarded by mProcessCaptureResultLock.
     std::unique_ptr<ResultMetadataQueue> mResultMetadataQueue;
 
-    virtual void disconnectSession() override;
+    virtual void closeSessionLocked() override;
+
+    virtual void releaseSessionLocked() override;
 }; // class Camera3OfflineSession
 
 }; // namespace android