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

Commit 5af1c38c authored by Nikolas Havrikov's avatar Nikolas Havrikov
Browse files

Avoid accumulation of user state persist requests 

This CL tackles the problem which arises that arises from frequent
association changes for a given user.
Until now, every association change allocates a map of previously
used ids, which it holds until the current user state is written to
storage.
Unfortunately, frequent association changes can lead to a significant
pileup of persist requests, leading to out-of-memory errors and an
unresponsive system.

It does not help that the requests to write out the user state are
handled by the singleton background thread, which is shared across
all system services. This CL does not solve this particular issue,
and so it remains a future work item.

This CL introduces a dedicated PersistUserStateHandler, which
differs from the current handler in two ways:
1. It only ever accepts one persist request per user at a time.
   This prevents the aforementioned request pileup and OOM errors.
2. It reads the previously used ids and the changed associations
   at the time of writing out the state instead of the time of
   request arrival. This does not change the intended behavior,
   and comes with the advantage of not having to allocate Runnables.

Test: atest CtsCompanionDeviceManagerCoreTestCases
Test: atest CtsCompanionDeviceManagerUiAutomationTestCases
Change-Id: I9aaad5a25fc4c09fcd9808a84d8ba2ce7962bfc5
parent ce65b04d

services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java

+36 −3
Original line number Diff line number Diff line
@@ -82,6 +82,7 @@ import android.net.NetworkPolicyManager; 
import android.os.Binder;

import android.os.Environment;

import android.os.Handler;

import android.os.Message;

import android.os.Parcel;

import android.os.PowerWhitelistManager;

import android.os.RemoteCallbackList;
@@ -185,6 +186,7 @@ public class CompanionDeviceManagerService extends SystemService 
    private final CompanionDeviceManagerServiceInternal mLocalService = new LocalService(this);



    final Handler mMainHandler = Handler.getMain();

    private final PersistUserStateHandler mUserPersistenceHandler = new PersistUserStateHandler();

    private CompanionDevicePresenceController mCompanionDevicePresenceController;



    /**
@@ -366,9 +368,8 @@ public class CompanionDeviceManagerService extends SystemService 


        final List<AssociationInfo> updatedAssociations =

                mAssociationStore.getAssociationsForUser(userId);

        final Map<String, Set<Integer>> usedIdsForUser = getPreviouslyUsedIdsForUser(userId);

        BackgroundThread.getHandler().post(() ->

                mPersistentStore.persistStateForUser(userId, updatedAssociations, usedIdsForUser));



        mUserPersistenceHandler.postPersistUserState(userId);



        // Notify listeners if ADDED, REMOVED or UPDATED_ADDRESS_CHANGED.

        // Do NOT notify when UPDATED_ADDRESS_UNCHANGED, which means a minor tweak in association's
@@ -381,6 +382,13 @@ public class CompanionDeviceManagerService extends SystemService 
        restartBleScan();

    }



    private void persistStateForUser(@UserIdInt int userId) {

        final List<AssociationInfo> updatedAssociations =

                mAssociationStore.getAssociationsForUser(userId);

        final Map<String, Set<Integer>> usedIdsForUser = getPreviouslyUsedIdsForUser(userId);

        mPersistentStore.persistStateForUser(userId, updatedAssociations, usedIdsForUser);

    }



    private void notifyListeners(

            @UserIdInt int userId, @NonNull List<AssociationInfo> associations) {

        mListeners.broadcast((listener, callbackUserId) -> {
@@ -1349,4 +1357,29 @@ public class CompanionDeviceManagerService extends SystemService 
            mService.associationCleanUp(profile);

        }

    }



    /**

     * This class is dedicated to handling requests to persist user state.

     */

    @SuppressLint("HandlerLeak")

    private class PersistUserStateHandler extends Handler {

        PersistUserStateHandler() {

            super(BackgroundThread.get().getLooper());

        }



        /**

         * Persists user state unless there is already an outstanding request for the given user.

         */

        synchronized void postPersistUserState(@UserIdInt int userId) {

            if (!hasMessages(userId)) {

                sendMessage(obtainMessage(userId));

            }

        }



        @Override

        public void handleMessage(@NonNull Message msg) {

            final int userId = msg.what;

            persistStateForUser(userId);

        }

    }

}