From 67884d6edd603c12864e85dc348fc39e3bbe0f70 Mon Sep 17 00:00:00 2001 From: Soonil Nagarkar Date: Thu, 7 Oct 2021 10:48:46 -0700 Subject: [PATCH 1/3] Revert changes to StationaryThrottlingLocationProvider For unknown reasons these changes are causing a memory usage increase affecting Go devices. This reverts commit 6eecc3ce91432db9ee4f3ede0b4efd45f70885f0. Bug: 201306859 Test: presubmits Change-Id: Ifd820ead3099c61abf7403442219f3d1172b8505 --- .../StationaryThrottlingLocationProvider.java | 25 ++++++------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/services/core/java/com/android/server/location/provider/StationaryThrottlingLocationProvider.java b/services/core/java/com/android/server/location/provider/StationaryThrottlingLocationProvider.java index cc51cea05160b..22a675ad39ab6 100644 --- a/services/core/java/com/android/server/location/provider/StationaryThrottlingLocationProvider.java +++ b/services/core/java/com/android/server/location/provider/StationaryThrottlingLocationProvider.java @@ -105,15 +105,20 @@ public final class StationaryThrottlingLocationProvider extends DelegateLocation synchronized (mLock) { mDeviceIdleHelper.addListener(this); - onDeviceIdleChanged(mDeviceIdleHelper.isDeviceIdle()); + mDeviceIdle = mDeviceIdleHelper.isDeviceIdle(); + mDeviceStationaryHelper.addListener(this); + mDeviceStationary = false; + mDeviceStationaryRealtimeMs = Long.MIN_VALUE; + + onThrottlingChangedLocked(false); } } @Override protected void onStop() { synchronized (mLock) { + mDeviceStationaryHelper.removeListener(this); mDeviceIdleHelper.removeListener(this); - onDeviceIdleChanged(false); mIncomingRequest = ProviderRequest.EMPTY_REQUEST; mOutgoingRequest = ProviderRequest.EMPTY_REQUEST; @@ -146,27 +151,13 @@ public final class StationaryThrottlingLocationProvider extends DelegateLocation } mDeviceIdle = deviceIdle; - - if (deviceIdle) { - // device stationary helper will deliver an immediate listener update - mDeviceStationaryHelper.addListener(this); - } else { - mDeviceStationaryHelper.removeListener(this); - mDeviceStationary = false; - mDeviceStationaryRealtimeMs = Long.MIN_VALUE; - onThrottlingChangedLocked(false); - } + onThrottlingChangedLocked(false); } } @Override public void onDeviceStationaryChanged(boolean deviceStationary) { synchronized (mLock) { - if (!mDeviceIdle) { - // stationary detection is only registered while idle - ignore late notifications - return; - } - if (mDeviceStationary == deviceStationary) { return; } From 8af9bbaeed2c30b466843d10bbdb5fb864ff04a9 Mon Sep 17 00:00:00 2001 From: Shuzhen Wang Date: Thu, 7 Oct 2021 19:39:57 -0700 Subject: [PATCH 2/3] Camera: Fix wrong wasImageCaptured flag CaptureFailure is created with a "dropped" parameter. But what's passed in is mayHaveBuffers, which is opposite of "dropped". Test: Camera CTS, GoogleCamera Bug: 201967385 Change-Id: I42eb2daa78f116502cb1c5943015ea3f4af3f1e3 --- core/java/android/hardware/camera2/CaptureFailure.java | 9 +++++---- .../android/hardware/camera2/impl/CameraDeviceImpl.java | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/core/java/android/hardware/camera2/CaptureFailure.java b/core/java/android/hardware/camera2/CaptureFailure.java index 20ca4a338f01b..032ed7e4db628 100644 --- a/core/java/android/hardware/camera2/CaptureFailure.java +++ b/core/java/android/hardware/camera2/CaptureFailure.java @@ -59,7 +59,7 @@ public class CaptureFailure { private final CaptureRequest mRequest; private final int mReason; - private final boolean mDropped; + private final boolean mWasImageCaptured; private final int mSequenceId; private final long mFrameNumber; private final String mErrorPhysicalCameraId; @@ -68,10 +68,11 @@ public class CaptureFailure { * @hide */ public CaptureFailure(CaptureRequest request, int reason, - boolean dropped, int sequenceId, long frameNumber, String errorPhysicalCameraId) { + boolean wasImageCaptured, int sequenceId, long frameNumber, + String errorPhysicalCameraId) { mRequest = request; mReason = reason; - mDropped = dropped; + mWasImageCaptured = wasImageCaptured; mSequenceId = sequenceId; mFrameNumber = frameNumber; mErrorPhysicalCameraId = errorPhysicalCameraId; @@ -141,7 +142,7 @@ public class CaptureFailure { * @return boolean True if the image was captured, false otherwise. */ public boolean wasImageCaptured() { - return !mDropped; + return mWasImageCaptured; } /** diff --git a/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java b/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java index 4708f3e0664fc..88649392c23cd 100644 --- a/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java +++ b/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java @@ -1867,7 +1867,7 @@ public class CameraDeviceImpl extends CameraDevice final CaptureFailure failure = new CaptureFailure( request, reason, - /*dropped*/ mayHaveBuffers, + mayHaveBuffers, requestId, frameNumber, errorPhysicalCameraId); From dd55b3dee558ff83f286d3abacfbd1a0c9d31278 Mon Sep 17 00:00:00 2001 From: Emilian Peev Date: Mon, 4 Oct 2021 18:27:14 -0700 Subject: [PATCH 3/3] Camera: Avoid possible deadlock when closing advanced extension session The advanced extension session request processor must not hold the 'mInterfaceLock' when calling the camera device implementation as it could result in a deadlock with the camera device close that can run at the same time and try to acquire the same locks in reverse order. The request processor will only read from 'mCameraConfigMap' which is initialized only once during session setup and will not change during the session lifetime. A sample stack trace will look like this: Service thread: 1) android.hardware.camera2.impl.CameraCaptureSessionImpl.stopRepeating 2) android.hardware.camera2.impl.CameraAdvancedExtensionSessionImpl$RequestProcessor.stopRepeating Client thread: 1) android.hardware.camera2.impl.CameraAdvancedExtensionSessionImpl.release 2) android.hardware.camera2.impl.CameraDeviceImpl.close Bug: 202074988 Test: Manual using application, Camera CTS Change-Id: If992667a7d7e54be5bea1fcc4b1d71ec1b895ca0 --- .../CameraAdvancedExtensionSessionImpl.java | 76 +++++++++---------- 1 file changed, 34 insertions(+), 42 deletions(-) diff --git a/core/java/android/hardware/camera2/impl/CameraAdvancedExtensionSessionImpl.java b/core/java/android/hardware/camera2/impl/CameraAdvancedExtensionSessionImpl.java index 8da6551f3d151..b8443fb6d14b9 100644 --- a/core/java/android/hardware/camera2/impl/CameraAdvancedExtensionSessionImpl.java +++ b/core/java/android/hardware/camera2/impl/CameraAdvancedExtensionSessionImpl.java @@ -873,21 +873,19 @@ public final class CameraAdvancedExtensionSessionImpl extends CameraExtensionSes @Override public int submitBurst(List requests, IRequestCallback callback) { int seqId = -1; - synchronized (mInterfaceLock) { - try { - CaptureCallbackHandler captureCallback = new CaptureCallbackHandler(callback); - ArrayList captureRequests = new ArrayList<>(); - for (Request request : requests) { - captureRequests.add(initializeCaptureRequest(mCameraDevice, request, - mCameraConfigMap)); - } - seqId = mCaptureSession.captureBurstRequests(captureRequests, - new CameraExtensionUtils.HandlerExecutor(mHandler), captureCallback); - } catch (CameraAccessException e) { - Log.e(TAG, "Failed to submit capture requests!"); - } catch (IllegalStateException e) { - Log.e(TAG, "Capture session closed!"); + try { + CaptureCallbackHandler captureCallback = new CaptureCallbackHandler(callback); + ArrayList captureRequests = new ArrayList<>(); + for (Request request : requests) { + captureRequests.add(initializeCaptureRequest(mCameraDevice, request, + mCameraConfigMap)); } + seqId = mCaptureSession.captureBurstRequests(captureRequests, + new CameraExtensionUtils.HandlerExecutor(mHandler), captureCallback); + } catch (CameraAccessException e) { + Log.e(TAG, "Failed to submit capture requests!"); + } catch (IllegalStateException e) { + Log.e(TAG, "Capture session closed!"); } return seqId; @@ -896,18 +894,16 @@ public final class CameraAdvancedExtensionSessionImpl extends CameraExtensionSes @Override public int setRepeating(Request request, IRequestCallback callback) { int seqId = -1; - synchronized (mInterfaceLock) { - try { - CaptureRequest repeatingRequest = initializeCaptureRequest(mCameraDevice, - request, mCameraConfigMap); - CaptureCallbackHandler captureCallback = new CaptureCallbackHandler(callback); - seqId = mCaptureSession.setSingleRepeatingRequest(repeatingRequest, - new CameraExtensionUtils.HandlerExecutor(mHandler), captureCallback); - } catch (CameraAccessException e) { - Log.e(TAG, "Failed to enable repeating request!"); - } catch (IllegalStateException e) { - Log.e(TAG, "Capture session closed!"); - } + try { + CaptureRequest repeatingRequest = initializeCaptureRequest(mCameraDevice, + request, mCameraConfigMap); + CaptureCallbackHandler captureCallback = new CaptureCallbackHandler(callback); + seqId = mCaptureSession.setSingleRepeatingRequest(repeatingRequest, + new CameraExtensionUtils.HandlerExecutor(mHandler), captureCallback); + } catch (CameraAccessException e) { + Log.e(TAG, "Failed to enable repeating request!"); + } catch (IllegalStateException e) { + Log.e(TAG, "Capture session closed!"); } return seqId; @@ -915,27 +911,23 @@ public final class CameraAdvancedExtensionSessionImpl extends CameraExtensionSes @Override public void abortCaptures() { - synchronized (mInterfaceLock) { - try { - mCaptureSession.abortCaptures(); - } catch (CameraAccessException e) { - Log.e(TAG, "Failed during capture abort!"); - } catch (IllegalStateException e) { - Log.e(TAG, "Capture session closed!"); - } + try { + mCaptureSession.abortCaptures(); + } catch (CameraAccessException e) { + Log.e(TAG, "Failed during capture abort!"); + } catch (IllegalStateException e) { + Log.e(TAG, "Capture session closed!"); } } @Override public void stopRepeating() { - synchronized (mInterfaceLock) { - try { - mCaptureSession.stopRepeating(); - } catch (CameraAccessException e) { - Log.e(TAG, "Failed during repeating capture stop!"); - } catch (IllegalStateException e) { - Log.e(TAG, "Capture session closed!"); - } + try { + mCaptureSession.stopRepeating(); + } catch (CameraAccessException e) { + Log.e(TAG, "Failed during repeating capture stop!"); + } catch (IllegalStateException e) { + Log.e(TAG, "Capture session closed!"); } } }