From 994a11fbb861a512de6b37c1df4a262a30ad5393 Mon Sep 17 00:00:00 2001 From: Emilian Peev Date: Wed, 9 Aug 2023 17:49:24 +0000 Subject: [PATCH] Camera: Avoid caching extension service connections Under certain condition the camera extension proxy service can be frozen along with the client process. Depending on the lifecycle of the client the service may not get unfrozen afterwards resulting in a still valid service connection but invalid/failing binder. To avoid this ensure that the extension service connection is destroyed as soon as there are no more client sessions and calls. Bug: 287618467 Test: Manual using Camera2Extension app, Camera CTS Change-Id: I0cd62c067f575846ef01684958a844f180b24c8d --- .../CameraExtensionCharacteristics.java | 55 +++++++++++++------ .../CameraAdvancedExtensionSessionImpl.java | 37 +++++++------ .../camera2/impl/CameraDeviceImpl.java | 17 +++++- .../impl/CameraExtensionSessionImpl.java | 28 +++++----- 4 files changed, 90 insertions(+), 47 deletions(-) diff --git a/core/java/android/hardware/camera2/CameraExtensionCharacteristics.java b/core/java/android/hardware/camera2/CameraExtensionCharacteristics.java index 6baf91d720c3d..ea951a55bfca2 100644 --- a/core/java/android/hardware/camera2/CameraExtensionCharacteristics.java +++ b/core/java/android/hardware/camera2/CameraExtensionCharacteristics.java @@ -236,9 +236,10 @@ public final class CameraExtensionCharacteristics { private static final CameraExtensionManagerGlobal GLOBAL_CAMERA_MANAGER = new CameraExtensionManagerGlobal(); private final Object mLock = new Object(); - private final int PROXY_SERVICE_DELAY_MS = 1000; + private final int PROXY_SERVICE_DELAY_MS = 2000; private InitializerFuture mInitFuture = null; private ServiceConnection mConnection = null; + private int mConnectionCount = 0; private ICameraExtensionsProxyService mProxy = null; private boolean mSupportsAdvancedExtensions = false; @@ -249,6 +250,15 @@ public final class CameraExtensionCharacteristics { return GLOBAL_CAMERA_MANAGER; } + private void releaseProxyConnectionLocked(Context ctx) { + if (mConnection != null ) { + ctx.unbindService(mConnection); + mConnection = null; + mProxy = null; + mConnectionCount = 0; + } + } + private void connectToProxyLocked(Context ctx) { if (mConnection == null) { Intent intent = new Intent(); @@ -270,7 +280,6 @@ public final class CameraExtensionCharacteristics { mConnection = new ServiceConnection() { @Override public void onServiceDisconnected(ComponentName component) { - mInitFuture.setStatus(false); mConnection = null; mProxy = null; } @@ -348,23 +357,32 @@ public final class CameraExtensionCharacteristics { public boolean registerClient(Context ctx, IBinder token) { synchronized (mLock) { + boolean ret = false; connectToProxyLocked(ctx); if (mProxy == null) { return false; } + mConnectionCount++; try { - return mProxy.registerClient(token); + ret = mProxy.registerClient(token); } catch (RemoteException e) { Log.e(TAG, "Failed to initialize extension! Extension service does " + " not respond!"); } + if (!ret) { + mConnectionCount--; + } - return false; + if (mConnectionCount <= 0) { + releaseProxyConnectionLocked(ctx); + } + + return ret; } } - public void unregisterClient(IBinder token) { + public void unregisterClient(Context ctx, IBinder token) { synchronized (mLock) { if (mProxy != null) { try { @@ -372,6 +390,11 @@ public final class CameraExtensionCharacteristics { } catch (RemoteException e) { Log.e(TAG, "Failed to de-initialize extension! Extension service does" + " not respond!"); + } finally { + mConnectionCount--; + if (mConnectionCount <= 0) { + releaseProxyConnectionLocked(ctx); + } } } } @@ -446,8 +469,8 @@ public final class CameraExtensionCharacteristics { /** * @hide */ - public static void unregisterClient(IBinder token) { - CameraExtensionManagerGlobal.get().unregisterClient(token); + public static void unregisterClient(Context ctx, IBinder token) { + CameraExtensionManagerGlobal.get().unregisterClient(ctx, token); } /** @@ -578,7 +601,7 @@ public final class CameraExtensionCharacteristics { } } } finally { - unregisterClient(token); + unregisterClient(mContext, token); } return Collections.unmodifiableList(ret); @@ -626,7 +649,7 @@ public final class CameraExtensionCharacteristics { Log.e(TAG, "Failed to query the extension for postview availability! Extension " + "service does not respond!"); } finally { - unregisterClient(token); + unregisterClient(mContext, token); } return false; @@ -722,7 +745,7 @@ public final class CameraExtensionCharacteristics { + "service does not respond!"); return Collections.emptyList(); } finally { - unregisterClient(token); + unregisterClient(mContext, token); } } @@ -791,7 +814,7 @@ public final class CameraExtensionCharacteristics { + " not respond!"); return new ArrayList<>(); } finally { - unregisterClient(token); + unregisterClient(mContext, token); } } @@ -872,7 +895,7 @@ public final class CameraExtensionCharacteristics { } } } finally { - unregisterClient(token); + unregisterClient(mContext, token); } } catch (RemoteException e) { Log.e(TAG, "Failed to query the extension supported sizes! Extension service does" @@ -957,7 +980,7 @@ public final class CameraExtensionCharacteristics { Log.e(TAG, "Failed to query the extension capture latency! Extension service does" + " not respond!"); } finally { - unregisterClient(token); + unregisterClient(mContext, token); } return null; @@ -998,7 +1021,7 @@ public final class CameraExtensionCharacteristics { Log.e(TAG, "Failed to query the extension progress callbacks! Extension service does" + " not respond!"); } finally { - unregisterClient(token); + unregisterClient(mContext, token); } return false; @@ -1075,7 +1098,7 @@ public final class CameraExtensionCharacteristics { } catch (RemoteException e) { throw new IllegalStateException("Failed to query the available capture request keys!"); } finally { - unregisterClient(token); + unregisterClient(mContext, token); } return Collections.unmodifiableSet(ret); @@ -1155,7 +1178,7 @@ public final class CameraExtensionCharacteristics { } catch (RemoteException e) { throw new IllegalStateException("Failed to query the available capture result keys!"); } finally { - unregisterClient(token); + unregisterClient(mContext, token); } return Collections.unmodifiableSet(ret); diff --git a/core/java/android/hardware/camera2/impl/CameraAdvancedExtensionSessionImpl.java b/core/java/android/hardware/camera2/impl/CameraAdvancedExtensionSessionImpl.java index ae700a0a3c417..f0fb2f94332d5 100644 --- a/core/java/android/hardware/camera2/impl/CameraAdvancedExtensionSessionImpl.java +++ b/core/java/android/hardware/camera2/impl/CameraAdvancedExtensionSessionImpl.java @@ -90,7 +90,7 @@ public final class CameraAdvancedExtensionSessionImpl extends CameraExtensionSes private final HashMap mReaderMap = new HashMap<>(); private RequestProcessor mRequestProcessor = new RequestProcessor(); private final int mSessionId; - private final IBinder mToken; + private IBinder mToken = null; private Surface mClientRepeatingRequestSurface; private Surface mClientCaptureSurface; @@ -103,6 +103,8 @@ public final class CameraAdvancedExtensionSessionImpl extends CameraExtensionSes private boolean mInitialized; private boolean mSessionClosed; + private final Context mContext; + // Lock to synchronize cross-thread access to device public interface final Object mInterfaceLock; @@ -113,14 +115,9 @@ public final class CameraAdvancedExtensionSessionImpl extends CameraExtensionSes public static CameraAdvancedExtensionSessionImpl createCameraAdvancedExtensionSession( @NonNull android.hardware.camera2.impl.CameraDeviceImpl cameraDevice, @NonNull Map characteristicsMap, - @NonNull Context ctx, @NonNull ExtensionSessionConfiguration config, int sessionId) + @NonNull Context ctx, @NonNull ExtensionSessionConfiguration config, int sessionId, + @NonNull IBinder token) throws CameraAccessException, RemoteException { - final IBinder token = new Binder(TAG + " : " + sessionId); - boolean success = CameraExtensionCharacteristics.registerClient(ctx, token); - if (!success) { - throw new UnsupportedOperationException("Unsupported extension!"); - } - String cameraId = cameraDevice.getId(); CameraExtensionCharacteristics extensionChars = new CameraExtensionCharacteristics(ctx, cameraId, characteristicsMap); @@ -204,8 +201,9 @@ public final class CameraAdvancedExtensionSessionImpl extends CameraExtensionSes IAdvancedExtenderImpl extender = CameraExtensionCharacteristics.initializeAdvancedExtension( config.getExtension()); extender.init(cameraId, characteristicsMapNative); - CameraAdvancedExtensionSessionImpl ret = new CameraAdvancedExtensionSessionImpl(extender, - cameraDevice, characteristicsMapNative, repeatingRequestSurface, + + CameraAdvancedExtensionSessionImpl ret = new CameraAdvancedExtensionSessionImpl(ctx, + extender, cameraDevice, characteristicsMapNative, repeatingRequestSurface, burstCaptureSurface, postviewSurface, config.getStateCallback(), config.getExecutor(), sessionId, token); @@ -217,13 +215,16 @@ public final class CameraAdvancedExtensionSessionImpl extends CameraExtensionSes return ret; } - private CameraAdvancedExtensionSessionImpl(@NonNull IAdvancedExtenderImpl extender, + private CameraAdvancedExtensionSessionImpl(Context ctx, + @NonNull IAdvancedExtenderImpl extender, @NonNull CameraDeviceImpl cameraDevice, Map characteristicsMap, @Nullable Surface repeatingRequestSurface, @Nullable Surface burstCaptureSurface, @Nullable Surface postviewSurface, @NonNull StateCallback callback, @NonNull Executor executor, - int sessionId, @NonNull IBinder token) { + int sessionId, + @NonNull IBinder token) { + mContext = ctx; mAdvancedExtender = extender; mCameraDevice = cameraDevice; mCharacteristicsMap = characteristicsMap; @@ -578,12 +579,16 @@ public final class CameraAdvancedExtensionSessionImpl extends CameraExtensionSes mSessionProcessor = null; } - CameraExtensionCharacteristics.unregisterClient(mToken); - if (mInitialized || (mCaptureSession != null)) { - notifyClose = true; - CameraExtensionCharacteristics.releaseSession(); + + if (mToken != null) { + if (mInitialized || (mCaptureSession != null)) { + notifyClose = true; + CameraExtensionCharacteristics.releaseSession(); + } + CameraExtensionCharacteristics.unregisterClient(mContext, mToken); } mInitialized = false; + mToken = null; for (ImageReader reader : mReaderMap.values()) { reader.close(); diff --git a/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java b/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java index d3bde4b4b8a86..181ab2cf34219 100644 --- a/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java +++ b/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java @@ -2550,19 +2550,32 @@ public class CameraDeviceImpl extends CameraDevice HashMap characteristicsMap = new HashMap<>( mPhysicalIdsToChars); characteristicsMap.put(mCameraId, mCharacteristics); + boolean initializationFailed = true; + IBinder token = new Binder(TAG + " : " + mNextSessionId++); try { + boolean ret = CameraExtensionCharacteristics.registerClient(mContext, token); + if (!ret) { + token = null; + throw new UnsupportedOperationException("Unsupported extension!"); + } + if (CameraExtensionCharacteristics.areAdvancedExtensionsSupported()) { mCurrentAdvancedExtensionSession = CameraAdvancedExtensionSessionImpl.createCameraAdvancedExtensionSession( this, characteristicsMap, mContext, extensionConfiguration, - mNextSessionId++); + mNextSessionId, token); } else { mCurrentExtensionSession = CameraExtensionSessionImpl.createCameraExtensionSession( this, characteristicsMap, mContext, extensionConfiguration, - mNextSessionId++); + mNextSessionId, token); } + initializationFailed = false; } catch (RemoteException e) { throw new CameraAccessException(CameraAccessException.CAMERA_ERROR); + } finally { + if (initializationFailed && (token != null)) { + CameraExtensionCharacteristics.unregisterClient(mContext, token); + } } } } diff --git a/core/java/android/hardware/camera2/impl/CameraExtensionSessionImpl.java b/core/java/android/hardware/camera2/impl/CameraExtensionSessionImpl.java index 1db4808b64309..7b7d7de031e41 100644 --- a/core/java/android/hardware/camera2/impl/CameraExtensionSessionImpl.java +++ b/core/java/android/hardware/camera2/impl/CameraExtensionSessionImpl.java @@ -91,7 +91,7 @@ public final class CameraExtensionSessionImpl extends CameraExtensionSession { private final Set mSupportedRequestKeys; private final Set mSupportedResultKeys; private final ExtensionSessionStatsAggregator mStatsAggregator; - private final IBinder mToken; + private IBinder mToken = null; private boolean mCaptureResultsSupported; private CameraCaptureSession mCaptureSession = null; @@ -119,6 +119,8 @@ public final class CameraExtensionSessionImpl extends CameraExtensionSession { // will do so internally. private boolean mInternalRepeatingRequestEnabled = true; + private final Context mContext; + // Lock to synchronize cross-thread access to device public interface final Object mInterfaceLock; @@ -135,14 +137,9 @@ public final class CameraExtensionSessionImpl extends CameraExtensionSession { @NonNull Map characteristicsMap, @NonNull Context ctx, @NonNull ExtensionSessionConfiguration config, - int sessionId) + int sessionId, + @NonNull IBinder token) throws CameraAccessException, RemoteException { - final IBinder token = new Binder(TAG + " : " + sessionId); - boolean success = CameraExtensionCharacteristics.registerClient(ctx, token); - if (!success) { - throw new UnsupportedOperationException("Unsupported extension!"); - } - String cameraId = cameraDevice.getId(); CameraExtensionCharacteristics extensionChars = new CameraExtensionCharacteristics(ctx, cameraId, characteristicsMap); @@ -234,6 +231,7 @@ public final class CameraExtensionSessionImpl extends CameraExtensionSession { characteristicsMap.get(cameraId).getNativeMetadata()); CameraExtensionSessionImpl session = new CameraExtensionSessionImpl( + ctx, extenders.second, extenders.first, supportedPreviewSizes, @@ -256,7 +254,7 @@ public final class CameraExtensionSessionImpl extends CameraExtensionSession { return session; } - public CameraExtensionSessionImpl(@NonNull IImageCaptureExtenderImpl imageExtender, + public CameraExtensionSessionImpl(Context ctx, @NonNull IImageCaptureExtenderImpl imageExtender, @NonNull IPreviewExtenderImpl previewExtender, @NonNull List previewSizes, @NonNull android.hardware.camera2.impl.CameraDeviceImpl cameraDevice, @@ -269,6 +267,7 @@ public final class CameraExtensionSessionImpl extends CameraExtensionSession { @NonNull IBinder token, @NonNull Set requestKeys, @Nullable Set resultKeys) { + mContext = ctx; mImageExtender = imageExtender; mPreviewExtender = previewExtender; mCameraDevice = cameraDevice; @@ -878,12 +877,15 @@ public final class CameraExtensionSessionImpl extends CameraExtensionSession { + " respond!"); } - CameraExtensionCharacteristics.unregisterClient(mToken); - if (mInitialized || (mCaptureSession != null)) { - notifyClose = true; - CameraExtensionCharacteristics.releaseSession(); + if (mToken != null) { + if (mInitialized || (mCaptureSession != null)) { + notifyClose = true; + CameraExtensionCharacteristics.releaseSession(); + } + CameraExtensionCharacteristics.unregisterClient(mContext, mToken); } mInitialized = false; + mToken = null; if (mRepeatingRequestImageCallback != null) { mRepeatingRequestImageCallback.close();