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

Commit b7dfb9b8 authored by Yohei Yukawa's avatar Yohei Yukawa
Browse files

Break a cross process ref chain in spell checker 

This is a follow up CL to my CL [1], which unintentionally introduced
a cross process reference chain in callback objects among the
following three processes:
 * spell checker client process X
 * system server process Y
 * spell checker service process Z

This is a common problem when implementing cross process callbacks
because we need to garbage collect processes Z, Y, X, in that order to
fully clean up objects that are directly or indirectly referenced from
callback objects in process X and Y.

Fortunately our spell checker API design allows us to mitigate this
problem by internally breaking references from those callback objects
to other heavyweight objects, like we often do so almost everywhere in
the system, because the system knows the entire lifecycle of callback
objects around spell checker.

android.view.textservice.SpellCheckerSession$InternalListener in the
process X already does this so we don't need to worry about reference
chains beyond that objects.

TextServicesManagerService.ISpellCheckerServiceCallbackBinder, which
was introduced by my previous CL [1], is however problematic because
it (indirectly) holds strong references to other objects including
binder proxies to the process X.  This CL addresses this part by
breaking references from ISpellCheckerServiceCallbackBinder to other
objects in TSMS with WeakReference, like we usually do so in the
system.

 [1]: Iee04f4c0e2d248041d01c528344b191b9875d122
      6008106a

Bug: 109701487
Test: Manually verified with hprof heap dump that
      ISpellCheckerServiceCallbackBinder no longer has strong
      reference chains to other heavy weight objects.
Change-Id: Icdf11e5be07abbe71bdf8af537959957d760b80d
parent 381f6287

services/core/java/com/android/server/TextServicesManagerService.java

+36 −8
Original line number Diff line number Diff line
@@ -63,6 +63,7 @@ import android.view.textservice.SpellCheckerSubtype; 
import java.io.FileDescriptor;

import java.io.IOException;

import java.io.PrintWriter;

import java.lang.ref.WeakReference;

import java.util.Arrays;

import java.util.ArrayList;

import java.util.HashMap;
@@ -1024,19 +1025,46 @@ public class TextServicesManagerService extends ITextServicesManager.Stub { 
    private static final class ISpellCheckerServiceCallbackBinder

            extends ISpellCheckerServiceCallback.Stub {

        @NonNull

        private final SpellCheckerBindGroup mBindGroup;

        @NonNull

        private final SessionRequest mRequest;

        private final Object mCallbackLock = new Object();



        ISpellCheckerServiceCallbackBinder(@NonNull final SpellCheckerBindGroup bindGroup,

                @NonNull final SessionRequest request) {

            mBindGroup = bindGroup;

            mRequest = request;

        @GuardedBy("mCallbackLock")

        @Nullable

        private WeakReference<SpellCheckerBindGroup> mBindGroup;



        /**

         * Original {@link SessionRequest} that is associated with this callback.

         *

         * <p>Note that {@link SpellCheckerBindGroup#mOnGoingSessionRequests} guarantees that this

         * {@link SessionRequest} object is kept alive until the request is canceled.</p>

         */

        @GuardedBy("mCallbackLock")

        @Nullable

        private WeakReference<SessionRequest> mRequest;



        ISpellCheckerServiceCallbackBinder(@NonNull SpellCheckerBindGroup bindGroup,

                @NonNull SessionRequest request) {

            synchronized (mCallbackLock) {

                mBindGroup = new WeakReference<>(bindGroup);

                mRequest = new WeakReference<>(request);

            }

        }



        @Override

        public void onSessionCreated(@Nullable ISpellCheckerSession newSession) {

            mBindGroup.onSessionCreated(newSession, mRequest);

            final SpellCheckerBindGroup group;

            final SessionRequest request;

            synchronized (mCallbackLock) {

                if (mBindGroup == null || mRequest == null) {

                    return;

                }

                group = mBindGroup.get();

                request = mRequest.get();

                mBindGroup = null;

                mRequest = null;

            }

            if (group != null && request != null) {

                group.onSessionCreated(newSession, request);

            }

        }

    }

}