From d049581fd05c05cbc46e38b62e2622ddd553b1e5 Mon Sep 17 00:00:00 2001 From: Milton Wu Date: Thu, 21 Apr 2022 19:03:13 +0800 Subject: [PATCH] Access mCurrentOperation in main handler BiometricScheduler::mCurrentOperation may be accessed on binder thread from FingerprintProvider, and it causes NullPointerException when mCurrentOperation was free on other thread. Use Consumer<> interface to return BaseClientMonitor asynchronously on BiometricScheduler's main handler. Bug: 228396526 Test: atest BiometricSchedulerTest FingerprintProviderTest \ Fingerprint21Test Test: Manually test UDFPS unlock on device Change-Id: Iea91f21879fc097932b9d2d08c5d60a99a4f6648 --- .../sensors/BiometricScheduler.java | 26 ++++++++---- .../fingerprint/aidl/FingerprintProvider.java | 42 +++++++++---------- .../fingerprint/hidl/Fingerprint21.java | 39 +++++++++-------- 3 files changed, 61 insertions(+), 46 deletions(-) diff --git a/services/core/java/com/android/server/biometrics/sensors/BiometricScheduler.java b/services/core/java/com/android/server/biometrics/sensors/BiometricScheduler.java index 19a93f30937fd..63609f77dc75b 100644 --- a/services/core/java/com/android/server/biometrics/sensors/BiometricScheduler.java +++ b/services/core/java/com/android/server/biometrics/sensors/BiometricScheduler.java @@ -46,6 +46,7 @@ import java.util.Date; import java.util.Deque; import java.util.List; import java.util.Locale; +import java.util.function.Consumer; /** * A scheduler for biometric HAL operations. Maintains a queue of {@link BaseClientMonitor} @@ -457,22 +458,33 @@ public class BiometricScheduler { } /** + * Get current operation BaseClientMonitor + * @deprecated TODO: b/229994966, encapsulate client monitors * @return the current operation */ + @Deprecated + @Nullable public BaseClientMonitor getCurrentClient() { return mCurrentOperation != null ? mCurrentOperation.getClientMonitor() : null; } - /** The current operation if the requestId is set and matches. */ + /** + * The current operation if the requestId is set and matches. + * @deprecated TODO: b/229994966, encapsulate client monitors + */ @Deprecated @Nullable - public BaseClientMonitor getCurrentClientIfMatches(long requestId) { - if (mCurrentOperation != null) { - if (mCurrentOperation.isMatchingRequestId(requestId)) { - return mCurrentOperation.getClientMonitor(); + public void getCurrentClientIfMatches(long requestId, + @NonNull Consumer clientMonitorConsumer) { + mHandler.post(() -> { + if (mCurrentOperation != null) { + if (mCurrentOperation.isMatchingRequestId(requestId)) { + clientMonitorConsumer.accept(mCurrentOperation.getClientMonitor()); + return; + } } - } - return null; + clientMonitorConsumer.accept(null); + }); } public int getCurrentPendingCount() { diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/FingerprintProvider.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/FingerprintProvider.java index 7d5b77c2d711d..998a8e1e9f901 100644 --- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/FingerprintProvider.java +++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/FingerprintProvider.java @@ -582,35 +582,35 @@ public class FingerprintProvider implements IBinder.DeathRecipient, ServiceProvi @Override public void onPointerDown(long requestId, int sensorId, int x, int y, float minor, float major) { - final BaseClientMonitor client = - mSensors.get(sensorId).getScheduler().getCurrentClientIfMatches(requestId); - if (!(client instanceof Udfps)) { - Slog.e(getTag(), "onPointerDown received during client: " + client); - return; - } - ((Udfps) client).onPointerDown(x, y, minor, major); + mSensors.get(sensorId).getScheduler().getCurrentClientIfMatches(requestId, (client) -> { + if (!(client instanceof Udfps)) { + Slog.e(getTag(), "onPointerDown received during client: " + client); + return; + } + ((Udfps) client).onPointerDown(x, y, minor, major); + }); } @Override public void onPointerUp(long requestId, int sensorId) { - final BaseClientMonitor client = - mSensors.get(sensorId).getScheduler().getCurrentClientIfMatches(requestId); - if (!(client instanceof Udfps)) { - Slog.e(getTag(), "onPointerUp received during client: " + client); - return; - } - ((Udfps) client).onPointerUp(); + mSensors.get(sensorId).getScheduler().getCurrentClientIfMatches(requestId, (client) -> { + if (!(client instanceof Udfps)) { + Slog.e(getTag(), "onPointerUp received during client: " + client); + return; + } + ((Udfps) client).onPointerUp(); + }); } @Override public void onUiReady(long requestId, int sensorId) { - final BaseClientMonitor client = - mSensors.get(sensorId).getScheduler().getCurrentClientIfMatches(requestId); - if (!(client instanceof Udfps)) { - Slog.e(getTag(), "onUiReady received during client: " + client); - return; - } - ((Udfps) client).onUiReady(); + mSensors.get(sensorId).getScheduler().getCurrentClientIfMatches(requestId, (client) -> { + if (!(client instanceof Udfps)) { + Slog.e(getTag(), "onUiReady received during client: " + client); + return; + } + ((Udfps) client).onUiReady(); + }); } @Override diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21.java index 52dbe2460e1cf..78a30e820c806 100644 --- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21.java +++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21.java @@ -794,32 +794,35 @@ public class Fingerprint21 implements IHwBinder.DeathRecipient, ServiceProvider @Override public void onPointerDown(long requestId, int sensorId, int x, int y, float minor, float major) { - final BaseClientMonitor client = mScheduler.getCurrentClientIfMatches(requestId); - if (!(client instanceof Udfps)) { - Slog.w(TAG, "onFingerDown received during client: " + client); - return; - } - ((Udfps) client).onPointerDown(x, y, minor, major); + mScheduler.getCurrentClientIfMatches(requestId, (client) -> { + if (!(client instanceof Udfps)) { + Slog.w(TAG, "onFingerDown received during client: " + client); + return; + } + ((Udfps) client).onPointerDown(x, y, minor, major); + }); } @Override public void onPointerUp(long requestId, int sensorId) { - final BaseClientMonitor client = mScheduler.getCurrentClientIfMatches(requestId); - if (!(client instanceof Udfps)) { - Slog.w(TAG, "onFingerDown received during client: " + client); - return; - } - ((Udfps) client).onPointerUp(); + mScheduler.getCurrentClientIfMatches(requestId, (client) -> { + if (!(client instanceof Udfps)) { + Slog.w(TAG, "onFingerDown received during client: " + client); + return; + } + ((Udfps) client).onPointerUp(); + }); } @Override public void onUiReady(long requestId, int sensorId) { - final BaseClientMonitor client = mScheduler.getCurrentClientIfMatches(requestId); - if (!(client instanceof Udfps)) { - Slog.w(TAG, "onUiReady received during client: " + client); - return; - } - ((Udfps) client).onUiReady(); + mScheduler.getCurrentClientIfMatches(requestId, (client) -> { + if (!(client instanceof Udfps)) { + Slog.w(TAG, "onUiReady received during client: " + client); + return; + } + ((Udfps) client).onUiReady(); + }); } @Override