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

Commit ec1b1649 authored by Abodunrinwa Toki's avatar Abodunrinwa Toki
Browse files

RESTRICT AUTOMERGE Close TextClassifier native resources. 

Bug: 122904322
Test: atest android.view.textclassifier.TextClassificationManagerTest
Test: (MANUAL)
      1. Start an app with a TextView
      2. Select the text to trigger the text selection toolbar and TextClassifier
      3. Close the app's activity by repeatedly clicking back on the nav bar
      4. Start a Profiler for the app and observe for memory growth
      5. Repeat 1 - 3 several times and observe.
         If the bug still exists, memory should grow at about 4 - 5MB.
	 If the bug is fixed, Memory should remain about the same across activity restarts.

Change-Id: Id9a60dea85cf3949de030042537287ddb29842a2
parent 64ca10b7

core/java/android/view/textclassifier/TextClassifierImpl.java

+24 −10
Original line number Diff line number Diff line
@@ -64,6 +64,9 @@ import java.util.concurrent.TimeUnit; 
import java.util.regex.Matcher;

import java.util.regex.Pattern;



//TODO: use java.lang.ref.Cleaner once Android supports Java 9

import sun.misc.Cleaner;



/**

 * Default implementation of the {@link TextClassifier} interface.

 *
@@ -92,6 +95,8 @@ public final class TextClassifierImpl implements TextClassifier { 
    private ModelFile mModel;

    @GuardedBy("mLock") // Do not access outside this lock.

    private TextClassifierImplNative mNative;

    @GuardedBy("mLock")

    private Cleaner mNativeCleaner;



    private final Object mLoggerLock = new Object();

    @GuardedBy("mLoggerLock") // Do not access outside this lock.
@@ -304,12 +309,12 @@ public final class TextClassifierImpl implements TextClassifier { 
            if (bestModel == null) {

                throw new FileNotFoundException("No model for " + localeList.toLanguageTags());

            }

            if (mNative == null || !Objects.equals(mModel, bestModel)) {

            if (mNative == null || mNative.isClosed() || !Objects.equals(mModel, bestModel)) {

                Log.d(DEFAULT_LOG_TAG, "Loading " + bestModel);

                destroyNativeIfExistsLocked();

                final ParcelFileDescriptor fd = ParcelFileDescriptor.open(

                        new File(bestModel.getPath()), ParcelFileDescriptor.MODE_READ_ONLY);

                mNative = new TextClassifierImplNative(fd.getFd());

                mNativeCleaner = Cleaner.create(this, new NativeCloser(mNative));

                closeAndLogError(fd);

                mModel = bestModel;

            }
@@ -324,14 +329,6 @@ public final class TextClassifierImpl implements TextClassifier { 
        }

    }



    @GuardedBy("mLock") // Do not call outside this lock.

    private void destroyNativeIfExistsLocked() {

        if (mNative != null) {

            mNative.close();

            mNative = null;

        }

    }



    private static String concatenateLocales(@Nullable LocaleList locales) {

        return (locales == null) ? "" : locales.toLanguageTags();

    }
@@ -822,4 +819,21 @@ public final class TextClassifierImpl implements TextClassifier { 
                    parsedTime.hashCode());

        }

    }



    /**

     * Code to close a TextClassifierImplNative. Must not reference the TextClassifierImpl.

     */

    private static final class NativeCloser implements Runnable {



        private final TextClassifierImplNative mNative;



        NativeCloser(TextClassifierImplNative nativeImpl) {

            mNative = Preconditions.checkNotNull(nativeImpl);

        }



        @Override

        public void run() {

            mNative.close();

        }

    }

}

core/java/android/view/textclassifier/TextClassifierImplNative.java

+15 −2
Original line number Diff line number Diff line
@@ -28,7 +28,8 @@ final class TextClassifierImplNative { 
        System.loadLibrary("textclassifier");

    }



    private final long mModelPtr;

    private final Object mCloseLock = new Object();

    private long mModelPtr;



    /**

     * Creates a new instance of TextClassifierImplNative, using the provided model image, given as
@@ -102,7 +103,19 @@ final class TextClassifierImplNative { 


    /** Frees up the allocated memory. */

    public void close() {

        synchronized (mCloseLock) {

            if (!isClosed()) {

                nativeClose(mModelPtr);

                mModelPtr = 0;

            }

        }

    }



    /**

     * Returns true if this object is closed, returns false otherwise.

     */

    public boolean isClosed() {

        return mModelPtr == 0L;

    }



    /** Returns a comma separated list of locales supported by the model as BCP 47 tags. */