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

Commit 0f551e00 authored by Simranjit Kohli's avatar Simranjit Kohli
Browse files

Prevent race condition 

Use proper synchronization to prevent server crash.

There were more cases where lokcing wasn't done as expected.
Those are addressed in this change.

Additional refactoring is also done to make code follow existing pattern.

Test: atest CtsAutoFillServiceTestCases
Bug: 339729009
Change-Id: I4c43bfcd8be93d1e3dd159fa243693cb0fc607bb
parent a1dcd5ae

services/autofill/java/com/android/server/autofill/AutofillManagerService.java

+1 −1
Original line number Diff line number Diff line
@@ -2003,7 +2003,7 @@ public final class AutofillManagerService 
                final AutofillManagerServiceImpl service =

                        peekServiceForUserWithLocalBinderIdentityLocked(userId);

                if (service != null) {

                    service.setViewAutofilled(sessionId, getCallingUid(), id);

                    service.setViewAutofilledLocked(sessionId, getCallingUid(), id);

                } else if (sVerbose) {

                    Slog.v(TAG, "setAutofillFailure(): no service for " + userId);

                }

services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java

+4 −5
Original line number Diff line number Diff line
@@ -478,7 +478,7 @@ final class AutofillManagerServiceImpl 
    }



    @GuardedBy("mLock")

    void setViewAutofilled(int sessionId, int uid, @NonNull AutofillId id) {

    void setViewAutofilledLocked(int sessionId, int uid, @NonNull AutofillId id) {

        if (!isEnabledLocked()) {

            Slog.wtf(TAG, "Service not enabled");

            return;
@@ -488,7 +488,7 @@ final class AutofillManagerServiceImpl 
            Slog.v(TAG, "setViewAutofilled(): no session for " + sessionId + "(" + uid + ")");

            return;

        }

        session.setViewAutofilled(id);

        session.setViewAutofilledLocked(id);

    }



    @GuardedBy("mLock")
@@ -792,11 +792,10 @@ final class AutofillManagerServiceImpl 
     * Initializes the last fill selection after an autofill service returned a new

     * {@link FillResponse}.

     */

    void setLastResponse(int sessionId, @NonNull FillResponse response) {

        synchronized (mLock) {

    @GuardedBy("mLock")

    void setLastResponseLocked(int sessionId, @NonNull FillResponse response) {

            mEventHistory = new FillEventHistory(sessionId, response.getClientState());

    }

    }



    void setLastAugmentedAutofillResponse(int sessionId) {

        synchronized (mLock) {

services/autofill/java/com/android/server/autofill/Session.java

+53 −49
Original line number Diff line number Diff line
@@ -1677,6 +1677,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 


        final LogMaker requestLog;



        synchronized (mLock) {

            // Start a new FillResponse logger for the success case.

            mFillResponseEventLogger.startLogForNewResponse();

            mFillResponseEventLogger.maybeSetRequestId(requestId);
@@ -1690,9 +1691,9 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 
                    (int) (fillRequestReceivedRelativeTimestamp));

            mFillResponseEventLogger.maybeSetLatencyFillResponseReceivedMillis(

                    (int) (fillRequestReceivedRelativeTimestamp));

        mFillResponseEventLogger.maybeSetDetectionPreference(getDetectionPreferenceForLogging());

            mFillResponseEventLogger.maybeSetDetectionPreference(

                    getDetectionPreferenceForLogging());



        synchronized (mLock) {

            if (mDestroyed) {

                Slog.w(TAG, "Call to Session#onFillRequestSuccess() rejected - session: "

                        + id + " destroyed");
@@ -1744,11 +1745,9 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 
                Slog.v(TAG, "Service requested to wait for delayed fill response.");

                registerDelayedFillBroadcastLocked();

            }

        }



        mService.setLastResponse(id, response);

            mService.setLastResponseLocked(id, response);



        synchronized (mLock) {

            if (mLogViewEntered) {

                mLogViewEntered = false;

                mService.logViewEntered(id, null);
@@ -1821,6 +1820,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 
        }



        int datasetCount = (datasetList == null) ? 0 : datasetList.size();

        synchronized (mLock) {

            mFillResponseEventLogger.maybeSetTotalDatasetsProvided(datasetCount);

            // It's possible that this maybe overwritten later on after PCC filtering.

            mFillResponseEventLogger.maybeSetAvailableCount(datasetCount);
@@ -1832,6 +1832,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 
            mFillResponseEventLogger.maybeSetLatencyResponseProcessingMillis();

            mFillResponseEventLogger.logAndEndEvent();

        }

    }





    @GuardedBy("mLock")
@@ -2381,6 +2382,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 
            @Nullable CharSequence message) {

        boolean showMessage = !TextUtils.isEmpty(message);



        synchronized (mLock) {

            // Start a new FillResponse logger for the failure or timeout case.

            mFillResponseEventLogger.startLogForNewResponse();

            mFillResponseEventLogger.maybeSetRequestId(requestId);
@@ -2389,13 +2391,13 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 
                    AVAILABLE_COUNT_WHEN_FILL_REQUEST_FAILED_OR_TIMEOUT);

            mFillResponseEventLogger.maybeSetTotalDatasetsProvided(

                    AVAILABLE_COUNT_WHEN_FILL_REQUEST_FAILED_OR_TIMEOUT);

        mFillResponseEventLogger.maybeSetDetectionPreference(getDetectionPreferenceForLogging());

            mFillResponseEventLogger.maybeSetDetectionPreference(

                    getDetectionPreferenceForLogging());

            final long fillRequestReceivedRelativeTimestamp =

                    SystemClock.elapsedRealtime() - mLatencyBaseTime;

            mFillResponseEventLogger.maybeSetLatencyFillResponseReceivedMillis(

                    (int) (fillRequestReceivedRelativeTimestamp));



        synchronized (mLock) {

            unregisterDelayedFillBroadcastLocked();

            if (mDestroyed) {

                Slog.w(TAG, "Call to Session#onFillRequestFailureOrTimeout(req=" + requestId
@@ -2635,8 +2637,8 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 
                        + id + " destroyed");

                return;

            }

        }

            mSaveEventLogger.maybeSetLatencySaveRequestMillis();

        }

        mHandler.sendMessage(obtainMessage(

                AutofillManagerServiceImpl::handleSessionSave,

                mService, this));
@@ -3227,7 +3229,9 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 
        mHandler.sendMessage(obtainMessage(Session::handleLogContextCommitted, this,

                Event.NO_SAVE_UI_REASON_NONE,

                COMMIT_REASON_UNKNOWN));

        logAllEvents(COMMIT_REASON_UNKNOWN);

        synchronized (mLock) {

            logAllEventsLocked(COMMIT_REASON_UNKNOWN);

        }

    }



    /**
@@ -4571,7 +4575,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 


                    FillResponse response = viewState.getSecondaryResponse();

                    if (response != null) {

                        logPresentationStatsOnViewEntered(response, isCredmanRequested);

                        logPresentationStatsOnViewEnteredLocked(response, isCredmanRequested);

                    }



                    // If the ViewState is ready to be displayed, onReady() will be called.
@@ -4662,7 +4666,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 


                FillResponse response = viewState.getResponse();

                if (response != null) {

                    logPresentationStatsOnViewEntered(response, isCredmanRequested);

                    logPresentationStatsOnViewEnteredLocked(response, isCredmanRequested);

                }



                if (isSameViewEntered) {
@@ -4703,7 +4707,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 
    }



    @GuardedBy("mLock")

    private void logPresentationStatsOnViewEntered(FillResponse response,

    private void logPresentationStatsOnViewEnteredLocked(FillResponse response,

            boolean isCredmanRequested) {

        mPresentationStatsEventLogger.maybeSetRequestId(response.getRequestId());

        mPresentationStatsEventLogger.maybeSetIsCredentialRequest(isCredmanRequested);
@@ -5061,7 +5065,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 


        getUiForShowing().showFillDialog(filledId, response, filterText,

                mService.getServicePackageName(), mComponentName, serviceIcon, this,

                id, mCompatMode, mPresentationStatsEventLogger);

                id, mCompatMode, mPresentationStatsEventLogger, mLock);

        return true;

    }
@@ -5420,7 +5424,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 
     * Sets the state of views that failed to autofill.

     */

    @GuardedBy("mLock")

    void setViewAutofilled(@NonNull AutofillId id) {

    void setViewAutofilledLocked(@NonNull AutofillId id) {

        if (sVerbose) {

            Slog.v(TAG, "View autofilled: " + id);

        }
@@ -6663,7 +6667,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 
    }



    @GuardedBy("mLock")

    private void logAllEvents(@AutofillCommitReason int val) {

    private void logAllEventsLocked(@AutofillCommitReason int val) {

        if (sVerbose) {

            Slog.v(TAG, "logAllEvents(" + id + "): commitReason: " + val);

        }
@@ -6695,7 +6699,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState 
        if (sVerbose) {

            Slog.v(TAG, "destroyLocked for session: " + id);

        }

        logAllEvents(COMMIT_REASON_SESSION_DESTROYED);

        logAllEventsLocked(COMMIT_REASON_SESSION_DESTROYED);



        if (mDestroyed) {

            return null;

services/autofill/java/com/android/server/autofill/ui/AutoFillUI.java

+16 −9
Original line number Diff line number Diff line
@@ -425,7 +425,8 @@ public final class AutoFillUI { 
            @Nullable String filterText, @Nullable String servicePackageName,

            @NonNull ComponentName componentName, @Nullable Drawable serviceIcon,

            @NonNull AutoFillUiCallback callback, int sessionId, boolean compatMode,

            @Nullable PresentationStatsEventLogger mPresentationStatsEventLogger) {

            @Nullable PresentationStatsEventLogger presentationStatsEventLogger,

            @NonNull Object sessionLock) {

        if (sVerbose) {

            Slog.v(TAG, "showFillDialog for "

                    + componentName.toShortString() + ": " + response);
@@ -467,10 +468,12 @@ public final class AutoFillUI { 
                        @Override

                        public void onDatasetPicked(Dataset dataset) {

                            log(MetricsEvent.TYPE_ACTION);

                            if (mPresentationStatsEventLogger != null) {

                                mPresentationStatsEventLogger.maybeSetPositiveCtaButtonClicked(

                            synchronized (sessionLock) {

                                if (presentationStatsEventLogger != null) {

                                    presentationStatsEventLogger.maybeSetPositiveCtaButtonClicked(

                                            true);

                                }

                            }

                            hideFillDialogUiThread(callback);

                            if (mCallback != null) {

                                final int datasetIndex = response.getDatasets().indexOf(dataset);
@@ -482,8 +485,10 @@ public final class AutoFillUI { 
                        @Override

                        public void onDismissed() {

                            log(MetricsEvent.TYPE_DISMISS);

                            if (mPresentationStatsEventLogger != null) {

                                mPresentationStatsEventLogger.maybeSetDialogDismissed(true);

                            synchronized (sessionLock) {

                                if (presentationStatsEventLogger != null) {

                                    presentationStatsEventLogger.maybeSetDialogDismissed(true);

                                }

                            }

                            hideFillDialogUiThread(callback);

                            callback.requestShowSoftInput(focusedId);
@@ -493,10 +498,12 @@ public final class AutoFillUI { 
                        @Override

                        public void onCanceled() {

                            log(MetricsEvent.TYPE_CLOSE);

                            if (mPresentationStatsEventLogger != null) {

                                mPresentationStatsEventLogger.maybeSetNegativeCtaButtonClicked(

                            synchronized (sessionLock) {

                                if (presentationStatsEventLogger != null) {

                                    presentationStatsEventLogger.maybeSetNegativeCtaButtonClicked(

                                            true);

                                }

                            }

                            hideFillDialogUiThread(callback);

                            callback.requestShowSoftInput(focusedId);

                            callback.requestFallbackFromFillDialog();