Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f9835510 authored by charleschen's avatar charleschen
Browse files

Fix race condition for VQDS functionalities

Lock are missing for thread safety on major VQDS functions.
These methods can be invoked consecutively and race condition can
happen. We need to add a lock to prevent it from happening.

Bug: 295390470
Test: atest CtsVoiceInteractionTestCases
Flag: N/A
Change-Id: I99c43deb024f2d5fee5c1ac12e549966f949dc1e
parent 6fafa0de
Loading
Loading
Loading
Loading
+64 −30
Original line number Diff line number Diff line
@@ -94,8 +94,10 @@ public class VisualQueryDetector {
     */
    public void updateState(@Nullable PersistableBundle options,
            @Nullable SharedMemory sharedMemory) {
        synchronized (mInitializationDelegate.getLock()) {
            mInitializationDelegate.updateState(options, sharedMemory);
        }
    }


    /**
@@ -116,11 +118,13 @@ public class VisualQueryDetector {
        if (DEBUG) {
            Slog.i(TAG, "#startRecognition");
        }
        synchronized (mInitializationDelegate.getLock()) {
            // check if the detector is active with the initialization delegate
            mInitializationDelegate.startRecognition();

            try {
            mManagerService.startPerceiving(new BinderCallback(mExecutor, mCallback));
                mManagerService.startPerceiving(new BinderCallback(
                        mExecutor, mCallback, mInitializationDelegate.getLock()));
            } catch (SecurityException e) {
                Slog.e(TAG, "startRecognition failed: " + e);
                return false;
@@ -129,6 +133,7 @@ public class VisualQueryDetector {
            }
            return true;
        }
    }

    /**
     * Stops visual query detection recognition.
@@ -140,8 +145,9 @@ public class VisualQueryDetector {
        if (DEBUG) {
            Slog.i(TAG, "#stopRecognition");
        }
        synchronized (mInitializationDelegate.getLock()) {
            // check if the detector is active with the initialization delegate
        mInitializationDelegate.startRecognition();
            mInitializationDelegate.stopRecognition();

            try {
                mManagerService.stopPerceiving();
@@ -150,6 +156,7 @@ public class VisualQueryDetector {
            }
            return true;
        }
    }

    /**
     * Destroy the current detector.
@@ -160,12 +167,16 @@ public class VisualQueryDetector {
        if (DEBUG) {
            Slog.i(TAG, "#destroy");
        }
        synchronized (mInitializationDelegate.getLock()) {
            mInitializationDelegate.destroy();
        }
    }

    /** @hide */
    public void dump(String prefix, PrintWriter pw) {
        // TODO: implement this
        synchronized (mInitializationDelegate.getLock()) {
            mInitializationDelegate.dump(prefix, pw);
        }
    }

    /** @hide */
@@ -175,8 +186,10 @@ public class VisualQueryDetector {

    /** @hide */
    void registerOnDestroyListener(Consumer<AbstractDetector> onDestroyListener) {
        synchronized (mInitializationDelegate.getLock()) {
            mInitializationDelegate.registerOnDestroyListener(onDestroyListener);
        }
    }

    /**
     * A class that lets a VoiceInteractionService implementation interact with
@@ -282,6 +295,15 @@ public class VisualQueryDetector {
        public boolean isUsingSandboxedDetectionService() {
            return true;
        }

        @Override
        public void dump(String prefix, PrintWriter pw) {
            // No-op
        }

        private Object getLock() {
            return mLock;
        }
    }

    private static class BinderCallback
@@ -289,31 +311,43 @@ public class VisualQueryDetector {
        private final Executor mExecutor;
        private final VisualQueryDetector.Callback mCallback;

        BinderCallback(Executor executor, VisualQueryDetector.Callback callback) {
        private final Object mLock;

        BinderCallback(Executor executor, VisualQueryDetector.Callback callback, Object lock) {
            this.mExecutor = executor;
            this.mCallback = callback;
            this.mLock = lock;
        }

        /** Called when the detected result is valid. */
        @Override
        public void onQueryDetected(@NonNull String partialQuery) {
            Slog.v(TAG, "BinderCallback#onQueryDetected");
            Binder.withCleanCallingIdentity(() -> mExecutor.execute(
                    () -> mCallback.onQueryDetected(partialQuery)));
            Binder.withCleanCallingIdentity(() -> {
                synchronized (mLock) {
                    mCallback.onQueryDetected(partialQuery);
                }
            });
        }

        @Override
        public void onQueryFinished() {
            Slog.v(TAG, "BinderCallback#onQueryFinished");
            Binder.withCleanCallingIdentity(() -> mExecutor.execute(
                    () -> mCallback.onQueryFinished()));
            Binder.withCleanCallingIdentity(() -> {
                synchronized (mLock) {
                    mCallback.onQueryFinished();
                }
            });
        }

        @Override
        public void onQueryRejected() {
            Slog.v(TAG, "BinderCallback#onQueryRejected");
            Binder.withCleanCallingIdentity(() -> mExecutor.execute(
                    () -> mCallback.onQueryRejected()));
            Binder.withCleanCallingIdentity(() -> {
                synchronized (mLock) {
                    mCallback.onQueryRejected();
                }
            });
        }

        /** Called when the detection fails due to an error. */
+69 −61
Original line number Diff line number Diff line
@@ -106,6 +106,7 @@ final class VisualQueryDetectorSession extends DetectorSession {
            @Override
            public void onAttentionGained() {
                Slog.v(TAG, "BinderCallback#onAttentionGained");
                synchronized (mLock) {
                    mEgressingData = true;
                    if (mAttentionListener == null) {
                        return;
@@ -118,17 +119,18 @@ final class VisualQueryDetectorSession extends DetectorSession {
                            callback.onVisualQueryDetectionServiceFailure(
                                    new VisualQueryDetectionServiceFailure(
                                            ERROR_CODE_ILLEGAL_ATTENTION_STATE,
                                        "Attention listener failed to switch to GAINED state."));
                                            "Attention listener fails to switch to GAINED state."));
                        } catch (RemoteException ex) {
                            Slog.v(TAG, "Fail to call onVisualQueryDetectionServiceFailure");
                        }
                    return;
                    }
                }
            }

            @Override
            public void onAttentionLost() {
                Slog.v(TAG, "BinderCallback#onAttentionLost");
                synchronized (mLock) {
                    mEgressingData = false;
                    if (mAttentionListener == null) {
                        return;
@@ -141,18 +143,19 @@ final class VisualQueryDetectorSession extends DetectorSession {
                            callback.onVisualQueryDetectionServiceFailure(
                                    new VisualQueryDetectionServiceFailure(
                                            ERROR_CODE_ILLEGAL_ATTENTION_STATE,
                                        "Attention listener failed to switch to LOST state."));
                                            "Attention listener fails to switch to LOST state."));
                        } catch (RemoteException ex) {
                            Slog.v(TAG, "Fail to call onVisualQueryDetectionServiceFailure");
                        }
                    return;
                    }
                }
            }

            @Override
            public void onQueryDetected(@NonNull String partialQuery) throws RemoteException {
                Objects.requireNonNull(partialQuery);
                Slog.v(TAG, "BinderCallback#onQueryDetected");
                synchronized (mLock) {
                    Objects.requireNonNull(partialQuery);
                    if (!mEgressingData) {
                        Slog.v(TAG, "Query should not be egressed within the unattention state.");
                        callback.onVisualQueryDetectionServiceFailure(
@@ -165,10 +168,12 @@ final class VisualQueryDetectorSession extends DetectorSession {
                    callback.onQueryDetected(partialQuery);
                    Slog.i(TAG, "Egressed from visual query detection process.");
                }
            }

            @Override
            public void onQueryFinished() throws RemoteException {
                Slog.v(TAG, "BinderCallback#onQueryFinished");
                synchronized (mLock) {
                    if (!mQueryStreaming) {
                        Slog.v(TAG, "Query streaming state signal FINISHED is block since there is"
                                + " no active query being streamed.");
@@ -181,10 +186,12 @@ final class VisualQueryDetectorSession extends DetectorSession {
                    callback.onQueryFinished();
                    mQueryStreaming = false;
                }
            }

            @Override
            public void onQueryRejected() throws RemoteException {
                Slog.v(TAG, "BinderCallback#onQueryRejected");
                synchronized (mLock) {
                    if (!mQueryStreaming) {
                        Slog.v(TAG, "Query streaming state signal REJECTED is block since there is"
                                + " no active query being streamed.");
@@ -197,6 +204,7 @@ final class VisualQueryDetectorSession extends DetectorSession {
                    callback.onQueryRejected();
                    mQueryStreaming = false;
                }
            }
        };
        return mRemoteDetectionService.run(
                service -> service.detectWithVisualSignals(internalCallback));