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

Commit 64fb7cc4 authored by Utkarsh Nigam's avatar Utkarsh Nigam
Browse files

Catch RejectedExecutionException instead of checking is executor shutdown. 

This to avoid race conditions as executor is not guarded by a lock.

Bug: 409170985
Flag: EXEMPT minor bugfix
Test: Existing CTS and Unit tests
Change-Id: I343e363ce9a9bffa4db95bb80028fabfa618ca29
parent 5ab5185c

services/appfunctions/java/com/android/server/appfunctions/FutureAppSearchSessionImpl.java

+3 −4
Original line number Diff line number Diff line
@@ -30,15 +30,12 @@ import android.app.appsearch.GetByDocumentIdRequest; 
import android.app.appsearch.GetSchemaResponse;

import android.app.appsearch.PutDocumentsRequest;

import android.app.appsearch.RemoveByDocumentIdRequest;

import android.app.appsearch.SearchResult;

import android.app.appsearch.SearchResults;

import android.app.appsearch.SearchSpec;

import android.app.appsearch.SetSchemaRequest;

import android.app.appsearch.SetSchemaResponse;



import com.android.internal.infra.AndroidFuture;



import java.util.List;

import java.util.Objects;

import java.util.concurrent.Executor;
@@ -132,7 +129,9 @@ public class FutureAppSearchSessionImpl implements FutureAppSearchSession { 
                                    new AndroidFuture<>();



                            session.put(

                                    putDocumentsRequest, mExecutor, batchResultFuture::complete);

                                    putDocumentsRequest,

                                    mExecutor,

                                    new BatchResultCallbackAdapter<>(batchResultFuture));

                            return batchResultFuture;

                        });

    }

services/appfunctions/java/com/android/server/appfunctions/MetadataSyncAdapter.java

+39 −20
Original line number Diff line number Diff line
@@ -57,6 +57,7 @@ import java.util.concurrent.ExecutionException; 
import java.util.concurrent.ExecutorService;

import java.util.concurrent.Executors;

import java.util.concurrent.Future;

import java.util.concurrent.RejectedExecutionException;



/**

 * This class implements helper methods for synchronously interacting with AppSearch while
@@ -140,8 +141,10 @@ public class MetadataSyncAdapter { 
                mCurrentSyncTask = null;

            }



            if (!mExecutor.isShutdown()) {

            try {

                mCurrentSyncTask = mExecutor.submit(runnable);

            } catch (RejectedExecutionException ex) {

                Slog.w(TAG, "Failed to submit sync request due to executor shutdown.", ex);

            }

        }
@@ -407,7 +410,6 @@ public class MetadataSyncAdapter { 
        return diffMap;

    }





    /**

     * This method returns a map of package names to a set of function ids from the AppFunction

     * metadata.
@@ -425,28 +427,43 @@ public class MetadataSyncAdapter { 
            @NonNull FutureAppSearchSession searchSession,

            @NonNull String schemaType,

            @NonNull String propertyFunctionId,

            @NonNull String propertyPackageName) throws ExecutionException, InterruptedException {

        ArrayMap<String, ArraySet<String>> packageToFunctionIdMap = getPackageToFunctionIdMap(

                searchSession, schemaType, propertyFunctionId,

                propertyPackageName, DEFAULT_RESULT_COUNT_PER_PAGE);

            @NonNull String propertyPackageName)

            throws ExecutionException, InterruptedException {

        ArrayMap<String, ArraySet<String>> packageToFunctionIdMap =

                getPackageToFunctionIdMap(

                        searchSession,

                        schemaType,

                        propertyFunctionId,

                        propertyPackageName,

                        DEFAULT_RESULT_COUNT_PER_PAGE);

        int functionIdCount = countTotalStringsInValueSets(packageToFunctionIdMap);

        if (functionIdCount == DEFAULT_RESULT_COUNT_PER_PAGE) {

            // We might run into b/400670498 where only the first page is returned while there

            // are more. This could be a false positive if we happen to have

            // DEFAULT_RESULT_COUNT_PER_PAGE AppFunctions. Retry with a higher page count.

            Slog.d(TAG,

            Slog.d(

                    TAG,

                    "b/400587895: getPackageToFunctionIdMapWithRetry is retrying for schemaType = "

                            + schemaType);

            packageToFunctionIdMap = getPackageToFunctionIdMap(

                    searchSession, schemaType, propertyFunctionId,

                    propertyPackageName, RETRY_RESULT_COUNT_PER_PAGE);

            packageToFunctionIdMap =

                    getPackageToFunctionIdMap(

                            searchSession,

                            schemaType,

                            propertyFunctionId,

                            propertyPackageName,

                            RETRY_RESULT_COUNT_PER_PAGE);

            int retryFunctionIdCount = countTotalStringsInValueSets(packageToFunctionIdMap);

            if (retryFunctionIdCount != DEFAULT_RESULT_COUNT_PER_PAGE) {

                // This is likely we did hit the bug. But if the diff is small, it could be

                // just there were indeed changes in # of app functions during the two searches.

                Slog.d(TAG, "b/400587895: First search yields " + functionIdCount

                Slog.d(

                        TAG,

                        "b/400587895: First search yields "

                                + functionIdCount

                                + " results, but the second one with higher page size yields "

                        + retryFunctionIdCount + " results. schemaType = " + schemaType);

                                + retryFunctionIdCount

                                + " results. schemaType = "

                                + schemaType);

            }

        }

        return packageToFunctionIdMap;
@@ -482,7 +499,9 @@ public class MetadataSyncAdapter { 
                        .search(

                                "",

                                buildMetadataSearchSpec(

                                             schemaType, propertyFunctionId, propertyPackageName,

                                        schemaType,

                                        propertyFunctionId,

                                        propertyPackageName,

                                        resultCountPerPage))

                        .get(); ) {

            List<SearchResult> searchResultsList = futureSearchResults.getNextPage().get();