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

Commit 1e245673 authored by Ryan Mitchell's avatar Ryan Mitchell
Browse files

Migrate ApkAssets to use NativeAllocationRegistry 

This change ensures that mNativePtr can be deleted only when its
ApkAssets is really inaccessible and prevents finalization race
conditions.

Bug: 159041693
Test: boot and add logs to ensure destructor runs
Change-Id: Iff0612fe6862a17b8c889d9e7ba230094e3cb14a
parent 3f7cb9bc

core/java/android/content/res/ApkAssets.java

+30 −14
Original line number Diff line number Diff line
@@ -25,6 +25,8 @@ import android.content.res.loader.ResourcesProvider; 


import com.android.internal.annotations.GuardedBy;



import libcore.util.NativeAllocationRegistry;



import java.io.FileDescriptor;

import java.io.IOException;

import java.lang.annotation.Retention;
@@ -101,11 +103,11 @@ public final class ApkAssets { 
    public @interface FormatType {}



    @GuardedBy("this")

    private long mNativePtr;  // final, except cleared in finalizer.

    private final long mNativePtr;



    @Nullable

    @GuardedBy("this")

    private final StringBlock mStringBlock;  // null or closed if mNativePtr = 0.

    private final StringBlock mStringBlock;



    @PropertyFlags

    private final int mFlags;
@@ -113,6 +115,19 @@ public final class ApkAssets { 
    @Nullable

    private final AssetsProvider mAssets;



    @GuardedBy("this")

    @Nullable

    private final Runnable mRunNativeCleanup;



    // Use a Holder to allow static initialization of ApkAssets in the boot image, and

    // possibly to avoid some initialization ordering issues.

    private static class NoImagePreloadHolder {

        // TODO(175425996): Make size estimate more accurate

        public static final NativeAllocationRegistry REGISTRY =

                NativeAllocationRegistry.createMalloced(ApkAssets.class.getClassLoader(),

                                                        nativeGetFinalizer());

    }



    /**

     * Creates a new ApkAssets instance from the given path on disk.

     *
@@ -287,6 +302,8 @@ public final class ApkAssets { 
        mFlags = flags;

        mNativePtr = nativeLoad(format, path, flags, assets);

        mStringBlock = new StringBlock(nativeGetStringBlock(mNativePtr), true /*useSparse*/);

        mRunNativeCleanup = NoImagePreloadHolder.REGISTRY.registerNativeAllocation(

                this, mNativePtr);

        mAssets = assets;

    }
@@ -298,6 +315,8 @@ public final class ApkAssets { 
        mFlags = flags;

        mNativePtr = nativeLoadFd(format, fd, friendlyName, flags, assets);

        mStringBlock = new StringBlock(nativeGetStringBlock(mNativePtr), true /*useSparse*/);

        mRunNativeCleanup = NoImagePreloadHolder.REGISTRY.registerNativeAllocation(

                this, mNativePtr);

        mAssets = assets;

    }
@@ -309,6 +328,8 @@ public final class ApkAssets { 
        mFlags = flags;

        mNativePtr = nativeLoadFdOffsets(format, fd, friendlyName, offset, length, flags, assets);

        mStringBlock = new StringBlock(nativeGetStringBlock(mNativePtr), true /*useSparse*/);

        mRunNativeCleanup = NoImagePreloadHolder.REGISTRY.registerNativeAllocation(

                this, mNativePtr);

        mAssets = assets;

    }
@@ -316,6 +337,7 @@ public final class ApkAssets { 
        mFlags = flags;

        mNativePtr = nativeLoadEmpty(flags, assets);

        mStringBlock = null;

        mRunNativeCleanup = null;

        mAssets = assets;

    }
@@ -403,22 +425,16 @@ public final class ApkAssets { 
        return "ApkAssets{path=" + getAssetPath() + "}";

    }



    @Override

    protected void finalize() throws Throwable {

        close();

    }



    /**

     * Closes this class and the contained {@link #mStringBlock}.

     */

    public void close() {

        synchronized (this) {

            if (mNativePtr != 0) {

            if (mStringBlock != null) {

                mStringBlock.close();

            }

                nativeDestroy(mNativePtr);

                mNativePtr = 0;

            if (mRunNativeCleanup != null) {

                mRunNativeCleanup.run();

            }

        }

    }
@@ -433,7 +449,6 @@ public final class ApkAssets { 
    private static native long nativeLoadFdOffsets(@FormatType int format,

            @NonNull FileDescriptor fd, @NonNull String friendlyName, long offset, long length,

            @PropertyFlags int flags, @Nullable AssetsProvider asset) throws IOException;

    private static native void nativeDestroy(long ptr);

    private static native @NonNull String nativeGetAssetPath(long ptr);

    private static native long nativeGetStringBlock(long ptr);

    private static native boolean nativeIsUpToDate(long ptr);
@@ -441,4 +456,5 @@ public final class ApkAssets { 
    private static native @Nullable OverlayableInfo nativeGetOverlayableInfo(long ptr,

            String overlayableName) throws IOException;

    private static native boolean nativeDefinesOverlayable(long ptr) throws IOException;

    private static native final long nativeGetFinalizer();

}

core/jni/android_content_res_ApkAssets.cpp

+6 −2
Original line number Diff line number Diff line
@@ -315,10 +315,14 @@ static jlong NativeLoadEmpty(JNIEnv* env, jclass /*clazz*/, jint flags, jobject 
  return reinterpret_cast<jlong>(apk_assets.release());

}



static void NativeDestroy(JNIEnv* /*env*/, jclass /*clazz*/, jlong ptr) {

static void NativeDestroy(void* ptr) {

  delete reinterpret_cast<ApkAssets*>(ptr);

}



static jlong NativeGetFinalizer(JNIEnv* /*env*/, jclass /*clazz*/) {

  return reinterpret_cast<jlong>(&NativeDestroy);

}



static jstring NativeGetAssetPath(JNIEnv* env, jclass /*clazz*/, jlong ptr) {

  const ApkAssets* apk_assets = reinterpret_cast<const ApkAssets*>(ptr);

  return env->NewStringUTF(apk_assets->GetPath().c_str());
@@ -427,7 +431,7 @@ static const JNINativeMethod gApkAssetsMethods[] = { 
    {"nativeLoadFdOffsets",

     "(ILjava/io/FileDescriptor;Ljava/lang/String;JJILandroid/content/res/loader/AssetsProvider;)J",

     (void*)NativeLoadFromFdOffset},

    {"nativeDestroy", "(J)V", (void*)NativeDestroy},

    {"nativeGetFinalizer", "()J", (void*)NativeGetFinalizer},

    {"nativeGetAssetPath", "(J)Ljava/lang/String;", (void*)NativeGetAssetPath},

    {"nativeGetStringBlock", "(J)J", (void*)NativeGetStringBlock},

    {"nativeIsUpToDate", "(J)Z", (void*)NativeIsUpToDate},