Camera: Release extension resources after closing handler thread

To avoid possible concurrency issues, always quit the handler
thread before releasing any extension resources.
In case camera is still running, camera callbacks can still
be handled during release which can result in concurrency
exceptions.

Bug: 195595369
Test: atest -c
cts/tests/camera/src/android/hardware/camera2/cts/CameraExtensionSessionTest.java

Change-Id: I473a4cb04e5ea27b5c8f8221a2390219a7915e73
This commit is contained in:
Emilian Peev
2021-08-04 15:38:25 -07:00
parent a72b3e92e9
commit 2543bb488b
3 changed files with 37 additions and 40 deletions

View File

@@ -446,16 +446,12 @@ public final class CameraAdvancedExtensionSessionImpl extends CameraExtensionSes
}
}
@Override
protected void finalize() throws Throwable {
if (mHandlerThread != null) {
mHandlerThread.quitSafely();
}
super.finalize();
}
public void release(boolean skipCloseNotification) {
boolean notifyClose = false;
public void release() {
synchronized (mInterfaceLock) {
mHandlerThread.quitSafely();
if (mSessionProcessor != null) {
try {
mSessionProcessor.deInitSession();
@@ -469,6 +465,7 @@ public final class CameraAdvancedExtensionSessionImpl extends CameraExtensionSes
if (mExtensionClientId >= 0) {
CameraExtensionCharacteristics.unregisterClient(mExtensionClientId);
if (mInitialized) {
notifyClose = true;
CameraExtensionCharacteristics.releaseSession();
}
}
@@ -482,6 +479,16 @@ public final class CameraAdvancedExtensionSessionImpl extends CameraExtensionSes
mClientRepeatingRequestSurface = null;
mClientCaptureSurface = null;
}
if (notifyClose && !skipCloseNotification) {
final long ident = Binder.clearCallingIdentity();
try {
mExecutor.execute(() -> mCallbacks.onClosed(
CameraAdvancedExtensionSessionImpl.this));
} finally {
Binder.restoreCallingIdentity(ident);
}
}
}
private void notifyConfigurationFailure() {
@@ -491,7 +498,7 @@ public final class CameraAdvancedExtensionSessionImpl extends CameraExtensionSes
}
}
release();
release(true /*skipCloseNotification*/);
final long ident = Binder.clearCallingIdentity();
try {
@@ -507,15 +514,7 @@ public final class CameraAdvancedExtensionSessionImpl extends CameraExtensionSes
android.hardware.camera2.CameraCaptureSession.StateCallback {
@Override
public void onClosed(@NonNull CameraCaptureSession session) {
release();
final long ident = Binder.clearCallingIdentity();
try {
mExecutor.execute(() -> mCallbacks.onClosed(
CameraAdvancedExtensionSessionImpl.this));
} finally {
Binder.restoreCallingIdentity(ident);
}
release(false /*skipCloseNotification*/);
}
@Override

View File

@@ -697,12 +697,12 @@ public class CameraDeviceImpl extends CameraDevice
}
if (mCurrentExtensionSession != null) {
mCurrentExtensionSession.release();
mCurrentExtensionSession.release(false /*skipCloseNotification*/);
mCurrentExtensionSession = null;
}
if (mCurrentAdvancedExtensionSession != null) {
mCurrentAdvancedExtensionSession.release();
mCurrentAdvancedExtensionSession.release(false /*skipCloseNotification*/);
mCurrentAdvancedExtensionSession = null;
}
@@ -1352,12 +1352,12 @@ public class CameraDeviceImpl extends CameraDevice
}
if (mCurrentExtensionSession != null) {
mCurrentExtensionSession.release();
mCurrentExtensionSession.release(true /*skipCloseNotification*/);
mCurrentExtensionSession = null;
}
if (mCurrentAdvancedExtensionSession != null) {
mCurrentAdvancedExtensionSession.release();
mCurrentAdvancedExtensionSession.release(true /*skipCloseNotification*/);
mCurrentAdvancedExtensionSession = null;
}

View File

@@ -630,18 +630,13 @@ public final class CameraExtensionSessionImpl extends CameraExtensionSession {
new CameraExtensionUtils.HandlerExecutor(mHandler), requestHandler);
}
@Override
protected void finalize() throws Throwable {
if (mHandlerThread != null) {
mHandlerThread.quitSafely();
}
super.finalize();
}
/** @hide */
public void release() {
public void release(boolean skipCloseNotification) {
boolean notifyClose = false;
synchronized (mInterfaceLock) {
mInternalRepeatingRequestEnabled = false;
mHandlerThread.quitSafely();
try {
mPreviewExtender.onDeInit();
@@ -654,6 +649,7 @@ public final class CameraExtensionSessionImpl extends CameraExtensionSession {
if (mExtensionClientId >= 0) {
CameraExtensionCharacteristics.unregisterClient(mExtensionClientId);
if (mInitialized) {
notifyClose = true;
CameraExtensionCharacteristics.releaseSession();
}
}
@@ -704,6 +700,15 @@ public final class CameraExtensionSessionImpl extends CameraExtensionSession {
mCameraRepeatingSurface = mClientRepeatingRequestSurface = null;
mCameraBurstSurface = mClientCaptureSurface = null;
}
if (notifyClose && !skipCloseNotification) {
final long ident = Binder.clearCallingIdentity();
try {
mExecutor.execute(() -> mCallbacks.onClosed(CameraExtensionSessionImpl.this));
} finally {
Binder.restoreCallingIdentity(ident);
}
}
}
private void notifyConfigurationFailure() {
@@ -713,7 +718,7 @@ public final class CameraExtensionSessionImpl extends CameraExtensionSession {
}
}
release();
release(true /*skipCloseNotification*/);
final long ident = Binder.clearCallingIdentity();
try {
@@ -745,14 +750,7 @@ public final class CameraExtensionSessionImpl extends CameraExtensionSession {
android.hardware.camera2.CameraCaptureSession.StateCallback {
@Override
public void onClosed(@NonNull CameraCaptureSession session) {
release();
final long ident = Binder.clearCallingIdentity();
try {
mExecutor.execute(() -> mCallbacks.onClosed(CameraExtensionSessionImpl.this));
} finally {
Binder.restoreCallingIdentity(ident);
}
release(false /*skipCloseNotification*/);
}
@Override