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

Commit 8cb5546c authored by Tony Mak's avatar Tony Mak
Browse files

Limit the maximal number of pending requests in TCMS 

Introduce FixedSizeQueue, a simple wrapper of ArrayDeque that only has
a limited size. This avoids the situation that TCMS caches a lot of
pending request in the queue while it is not going to bind to a TCS
anytime soon. When the request is removed from the queue, we notify
the client that the request is failed.

Also, we now limit the max amount of sessions in the cache.
We already have two measures to avoid the leak of sessions, i.e. Cleaner
and linkToDeath, but just in case.


Also, fixed an issue that onDestoryTcSession() is called on the
wrong TC if the TCS is not bound yet. This is an example of the
problematic running sequence:
1. TCS is not bound
2. onCreateTextClassificationSession is called
3. onDestory() is called. TCMS finds out which TC to call by looking
   at session cache.
   However, sessionCache does not contains the record that we need
   because we do not update it until the service is bound.
4. TCS is now bound.
5. onCreateTextClassificationSession is forwared to the right TC.
6. onDestory is forwared to the default TC, which is the default
   if we don't know which TC to call on.
The solution is just to update sessionCache as soon as
onCreateTextClassificationSession is invoked.


BUG: 156683847

Test: atest FixedSizeQueueTest
Test: Revert ag/11734845. Send a lot of messages that won't trigger
      suggestConversationActions. Check the output of
      "dumpsys textclassification", make sure the pending queue size
       is always <= the limit.
Test: m mts && mts-tradefed run mts-extservices

Change-Id: Ib2e3e0d553e703ea759144bc9b38fec0d87de719
parent a38a553c

services/core/java/com/android/server/textclassifier/FixedSizeQueue.java

0 → 100644
+103 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2020 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



package com.android.server.textclassifier;



import android.annotation.NonNull;

import android.annotation.Nullable;



import com.android.internal.annotations.VisibleForTesting;

import com.android.internal.util.Preconditions;



import java.util.ArrayDeque;

import java.util.Objects;

import java.util.Queue;



/**

 * A fixed-size queue which automatically evicts the oldest element from the queue when it is full.

 *

 * <p>This class does not accept null element.

 *

 * @param <E> the type of elements held in this queue

 */

@VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)

public final class FixedSizeQueue<E> {



    private final Queue<E> mDelegate;



    @Nullable

    private final OnEntryEvictedListener<E> mOnEntryEvictedListener;



    private final int mMaxSize;



    public FixedSizeQueue(int maxSize, @Nullable OnEntryEvictedListener<E> onEntryEvictedListener) {

        Preconditions.checkArgument(maxSize > 0, "maxSize (%s) must > 0", maxSize);

        mDelegate = new ArrayDeque<>(maxSize);

        mMaxSize = maxSize;

        mOnEntryEvictedListener = onEntryEvictedListener;

    }



    /** Returns the number of items in the queue. */

    public int size() {

        return mDelegate.size();

    }



    /** Adds an element to the queue, evicts the oldest element if it reaches its max capacity. */

    public boolean add(@NonNull E element) {

        Objects.requireNonNull(element);

        if (size() == mMaxSize) {

            E removed = mDelegate.remove();

            if (mOnEntryEvictedListener != null) {

                mOnEntryEvictedListener.onEntryEvicted(removed);

            }

        }

        mDelegate.add(element);

        return true;

    }



    /**

     * Returns and removes the head of the queue, or returns null if this queue is empty.

     */

    @Nullable

    public E poll() {

        return mDelegate.poll();

    }



    /**

     * Removes an element from the queue, returns a boolean to indicate if an element is removed.

     */

    public boolean remove(@NonNull E element) {

        Objects.requireNonNull(element);

        return mDelegate.remove(element);

    }



    /** Returns whether the queue is empty. */

    public boolean isEmpty() {

        return mDelegate.isEmpty();

    }



    /**

     * A listener to get notified when an element is evicted.

     *

     * @param <E> the type of element

     */

    public interface OnEntryEvictedListener<E> {

        /**

         * Notifies that an element is evicted because the queue is reaching its max capacity.

         */

        void onEntryEvicted(@NonNull E element);

    }

}

services/core/java/com/android/server/textclassifier/TextClassificationManagerService.java

+20 −7
Original line number Diff line number Diff line
@@ -41,6 +41,7 @@ import android.service.textclassifier.TextClassifierService; 
import android.service.textclassifier.TextClassifierService.ConnectionState;

import android.text.TextUtils;

import android.util.ArrayMap;

import android.util.LruCache;

import android.util.Slog;

import android.util.SparseArray;

import android.view.textclassifier.ConversationAction;
@@ -68,12 +69,10 @@ import com.android.server.SystemService; 


import java.io.FileDescriptor;

import java.io.PrintWriter;

import java.util.ArrayDeque;

import java.util.ArrayList;

import java.util.List;

import java.util.Map;

import java.util.Objects;

import java.util.Queue;



/**

 * A manager for TextClassifier services.
@@ -308,13 +307,15 @@ public final class TextClassificationManagerService extends ITextClassifierServi 
        Objects.requireNonNull(classificationContext);

        Objects.requireNonNull(classificationContext.getSystemTextClassifierMetadata());



        synchronized (mLock) {

            mSessionCache.put(sessionId, classificationContext);

        }

        handleRequest(

                classificationContext.getSystemTextClassifierMetadata(),

                /* verifyCallingPackage= */ true,

                /* attemptToBind= */ false,

                service -> {

                    service.onCreateTextClassificationSession(classificationContext, sessionId);

                    mSessionCache.put(sessionId, classificationContext);

                },

                "onCreateTextClassificationSession",

                NO_OP_CALLBACK);
@@ -588,12 +589,14 @@ public final class TextClassificationManagerService extends ITextClassifierServi 
     * are cleaned up automatically when the client process is dead.

     */

    static final class SessionCache {

        private static final int MAX_CACHE_SIZE = 100;



        @NonNull

        private final Object mLock;

        @NonNull

        @GuardedBy("mLock")

        private final Map<TextClassificationSessionId, StrippedTextClassificationContext> mCache =

                new ArrayMap<>();

        private final LruCache<TextClassificationSessionId, StrippedTextClassificationContext>

                mCache = new LruCache<>(MAX_CACHE_SIZE);

        @NonNull

        @GuardedBy("mLock")

        private final Map<TextClassificationSessionId, DeathRecipient> mDeathRecipients =
@@ -775,6 +778,8 @@ public final class TextClassificationManagerService extends ITextClassifierServi 
    }



    private final class ServiceState {

        private static final int MAX_PENDING_REQUESTS = 20;



        @UserIdInt

        final int mUserId;

        @NonNull
@@ -786,7 +791,15 @@ public final class TextClassificationManagerService extends ITextClassifierServi 
        final int mBindServiceFlags;

        @NonNull

        @GuardedBy("mLock")

        final Queue<PendingRequest> mPendingRequests = new ArrayDeque<>();

        final FixedSizeQueue<PendingRequest> mPendingRequests =

                new FixedSizeQueue<>(MAX_PENDING_REQUESTS,

                        request -> {

                            Slog.w(LOG_TAG,

                                    String.format("Pending request[%s] is dropped", request.mName));

                            if (request.mOnServiceFailure != null) {

                                request.mOnServiceFailure.run();

                            }

                        });

        @Nullable

        @GuardedBy("mLock")

        ITextClassifierService mService;
@@ -910,7 +923,7 @@ public final class TextClassificationManagerService extends ITextClassifierServi 
                pw.printPair("bindServiceFlags", mBindServiceFlags);

                pw.printPair("boundServiceUid", mBoundServiceUid);

                pw.printPair("binding", mBinding);

                pw.printPair("numberRequests", mPendingRequests.size());

                pw.printPair("numOfPendingRequests", mPendingRequests.size());

            }

        }

services/tests/servicestests/src/com/android/server/textclassifier/FixedSizeQueueTest.java

0 → 100644
+109 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2020 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



package com.android.server.textclassifier;



import static com.google.common.truth.Truth.assertThat;



import org.junit.Test;



import java.util.ArrayList;

import java.util.List;



public class FixedSizeQueueTest {



    @Test

    public void add_belowMaxCapacity() {

        FixedSizeQueue<Integer> queue = new FixedSizeQueue<>(1, /* onEntryEvictedListener= */ null);

        assertThat(queue.size()).isEqualTo(0);



        queue.add(1);



        assertThat(queue.size()).isEqualTo(1);

        assertThat(queue.poll()).isEqualTo(1);

    }



    @Test

    public void add_exceedMaxCapacity() {

        FixedSizeQueue<Integer> queue = new FixedSizeQueue<>(2, /* onEntryEvictedListener= */ null);



        queue.add(1);

        queue.add(2);

        queue.add(3);



        assertThat(queue.size()).isEqualTo(2);

        assertThat(queue.poll()).isEqualTo(2);

        assertThat(queue.poll()).isEqualTo(3);

    }



    @Test

    public void poll() {

        FixedSizeQueue<Integer> queue = new FixedSizeQueue<>(1, /* onEntryEvictedListener= */ null);



        queue.add(1);



        assertThat(queue.poll()).isEqualTo(1);

        assertThat(queue.poll()).isNull();

    }



    @Test

    public void remove() {

        FixedSizeQueue<Integer> queue = new FixedSizeQueue<>(1, /* onEntryEvictedListener= */ null);



        queue.add(1);



        assertThat(queue.remove(1)).isTrue();

        assertThat(queue.isEmpty()).isTrue();

    }



    @Test

    public void remove_noSuchElement() {

        FixedSizeQueue<Integer> queue = new FixedSizeQueue<>(1, /* onEntryEvictedListener= */ null);



        queue.add(1);



        assertThat(queue.remove(2)).isFalse();

    }



    @Test

    public void isEmpty_true() {

        FixedSizeQueue<Integer> queue = new FixedSizeQueue<>(1, /* onEntryEvictedListener= */ null);



        assertThat(queue.isEmpty()).isTrue();

    }



    @Test

    public void isEmpty_false() {

        FixedSizeQueue<Integer> queue = new FixedSizeQueue<>(1, /* onEntryEvictedListener= */ null);



        queue.add(1);



        assertThat(queue.isEmpty()).isFalse();

    }



    @Test

    public void onEntryEvicted() {

        List<Integer> onEntryEvictedElements = new ArrayList<>();

        FixedSizeQueue<Integer> queue =

                new FixedSizeQueue<>(1, onEntryEvictedElements::add);



        queue.add(1);

        queue.add(2);

        queue.add(3);



        assertThat(onEntryEvictedElements).containsExactly(1, 2).inOrder();

    }

}