From b8bf78a7d1417f9c10576cfa8348d247d1e64623 Mon Sep 17 00:00:00 2001 From: Chris Ye Date: Fri, 21 Feb 2020 14:49:01 -0800 Subject: [PATCH] Remove InputChannel.finalize() by using NativeAllocationRegistry Remove InputChannel.finalize() by using NativeAllocationRegistry, for native object destruction. Refactor native input channel API to align the native input channel object with java input channel object. Bug: 113624847 Change-Id: Iabd2709885816aeb921f82d1905f93112fefb12d --- core/java/android/view/InputChannel.java | 79 ++++++++---- core/jni/android_view_InputChannel.cpp | 146 ++++++++--------------- 2 files changed, 105 insertions(+), 120 deletions(-) diff --git a/core/java/android/view/InputChannel.java b/core/java/android/view/InputChannel.java index 5a64b131faeae..72cc10a0aad18 100644 --- a/core/java/android/view/InputChannel.java +++ b/core/java/android/view/InputChannel.java @@ -22,6 +22,8 @@ import android.os.Parcel; import android.os.Parcelable; import android.util.Slog; +import libcore.util.NativeAllocationRegistry; + /** * An input channel specifies the file descriptors used to send input events to * a window in another process. It is Parcelable so that it can be sent @@ -33,6 +35,10 @@ public final class InputChannel implements Parcelable { private static final String TAG = "InputChannel"; private static final boolean DEBUG = false; + private static final NativeAllocationRegistry sRegistry = + NativeAllocationRegistry.createMalloced( + InputChannel.class.getClassLoader(), + nativeGetFinalizer()); @UnsupportedAppUsage public static final @android.annotation.NonNull Parcelable.Creator CREATOR @@ -51,18 +57,18 @@ public final class InputChannel implements Parcelable { @SuppressWarnings("unused") @UnsupportedAppUsage private long mPtr; // used by native code + private Runnable mCleaner; - private static native InputChannel[] nativeOpenInputChannelPair(String name); + private static native long[] nativeOpenInputChannelPair(String name); - private native void nativeDispose(boolean finalized); - private native void nativeRelease(); - private native void nativeTransferTo(InputChannel other); - private native void nativeReadFromParcel(Parcel parcel); - private native void nativeWriteToParcel(Parcel parcel); - private native void nativeDup(InputChannel target); - private native IBinder nativeGetToken(); + private static native long nativeGetFinalizer(); + private native void nativeDispose(long channel); + private native long nativeReadFromParcel(Parcel parcel); + private native void nativeWriteToParcel(Parcel parcel, long channel); + private native long nativeDup(long channel); + private native IBinder nativeGetToken(long channel); - private native String nativeGetName(); + private native String nativeGetName(long channel); /** * Creates an uninitialized input channel. @@ -73,13 +79,25 @@ public final class InputChannel implements Parcelable { public InputChannel() { } - @Override - protected void finalize() throws Throwable { - try { - nativeDispose(true); - } finally { - super.finalize(); + /** + * Set Native input channel object from native space. + * @param nativeChannel the native channel object. + * + * @hide + */ + @UnsupportedAppUsage + public void SetNativeInputChannel(long nativeChannel) { + if (nativeChannel == 0) { + throw new IllegalArgumentException("Set native input channel to null."); } + if (mPtr != 0) { + throw new IllegalArgumentException("Already has native input channel."); + } + if (DEBUG) { + Slog.d(TAG, "SetNativeInputChannel : " + String.format("%x", nativeChannel)); + } + mCleaner = sRegistry.registerNativeAllocation(this, nativeChannel); + mPtr = nativeChannel; } /** @@ -98,7 +116,13 @@ public final class InputChannel implements Parcelable { if (DEBUG) { Slog.d(TAG, "Opening input channel pair '" + name + "'"); } - return nativeOpenInputChannelPair(name); + InputChannel channels[] = new InputChannel[2]; + long[] nativeChannels = nativeOpenInputChannelPair(name); + for (int i = 0; i< 2; i++) { + channels[i] = new InputChannel(); + channels[i].SetNativeInputChannel(nativeChannels[i]); + } + return channels; } /** @@ -106,7 +130,7 @@ public final class InputChannel implements Parcelable { * @return The input channel name. */ public String getName() { - String name = nativeGetName(); + String name = nativeGetName(mPtr); return name != null ? name : "uninitialized"; } @@ -116,7 +140,7 @@ public final class InputChannel implements Parcelable { * When all references are released, the input channel will be closed. */ public void dispose() { - nativeDispose(false); + nativeDispose(mPtr); } /** @@ -124,7 +148,6 @@ public final class InputChannel implements Parcelable { * still exist in native-land, then the channel may continue to exist. */ public void release() { - nativeRelease(); } /** @@ -137,8 +160,10 @@ public final class InputChannel implements Parcelable { if (outParameter == null) { throw new IllegalArgumentException("outParameter must not be null"); } - - nativeTransferTo(outParameter); + if (outParameter.mPtr != 0) { + throw new IllegalArgumentException("Other object already has a native input channel."); + } + outParameter.SetNativeInputChannel(nativeDup(mPtr)); } /** @@ -146,7 +171,7 @@ public final class InputChannel implements Parcelable { */ public InputChannel dup() { InputChannel target = new InputChannel(); - nativeDup(target); + target.SetNativeInputChannel(nativeDup(mPtr)); return target; } @@ -159,8 +184,10 @@ public final class InputChannel implements Parcelable { if (in == null) { throw new IllegalArgumentException("in must not be null"); } - - nativeReadFromParcel(in); + long nativeIn = nativeReadFromParcel(in); + if (nativeIn > 0) { + SetNativeInputChannel(nativeIn); + } } @Override @@ -169,7 +196,7 @@ public final class InputChannel implements Parcelable { throw new IllegalArgumentException("out must not be null"); } - nativeWriteToParcel(out); + nativeWriteToParcel(out, mPtr); if ((flags & PARCELABLE_WRITE_RETURN_VALUE) != 0) { dispose(); @@ -182,6 +209,6 @@ public final class InputChannel implements Parcelable { } public IBinder getToken() { - return nativeGetToken(); + return nativeGetToken(mPtr); } } diff --git a/core/jni/android_view_InputChannel.cpp b/core/jni/android_view_InputChannel.cpp index 891520a34ce10..cc182b64f251b 100644 --- a/core/jni/android_view_InputChannel.cpp +++ b/core/jni/android_view_InputChannel.cpp @@ -37,7 +37,6 @@ static struct { jclass clazz; jfieldID mPtr; // native object attached to the DVM InputChannel - jmethodID ctor; } gInputChannelClassInfo; // ---------------------------------------------------------------------------- @@ -88,12 +87,6 @@ static NativeInputChannel* android_view_InputChannel_getNativeInputChannel(JNIEn return reinterpret_cast(longPtr); } -static void android_view_InputChannel_setNativeInputChannel(JNIEnv* env, jobject inputChannelObj, - NativeInputChannel* nativeInputChannel) { - env->SetLongField(inputChannelObj, gInputChannelClassInfo.mPtr, - reinterpret_cast(nativeInputChannel)); -} - sp android_view_InputChannel_getInputChannel(JNIEnv* env, jobject inputChannelObj) { NativeInputChannel* nativeInputChannel = android_view_InputChannel_getNativeInputChannel(env, inputChannelObj); @@ -111,20 +104,15 @@ void android_view_InputChannel_setDisposeCallback(JNIEnv* env, jobject inputChan } } -static jobject android_view_InputChannel_createInputChannel(JNIEnv* env, +static jlong android_view_InputChannel_createInputChannel(JNIEnv* env, sp inputChannel) { std::unique_ptr nativeInputChannel = std::make_unique(inputChannel); - jobject inputChannelObj = env->NewObject(gInputChannelClassInfo.clazz, - gInputChannelClassInfo.ctor); - if (inputChannelObj) { - android_view_InputChannel_setNativeInputChannel(env, inputChannelObj, - nativeInputChannel.release()); - } - return inputChannelObj; + + return reinterpret_cast(nativeInputChannel.release()); } -static jobjectArray android_view_InputChannel_nativeOpenInputChannelPair(JNIEnv* env, +static jlongArray android_view_InputChannel_nativeOpenInputChannelPair(JNIEnv* env, jclass clazz, jstring nameObj) { ScopedUtfChars nameChars(env, nameObj); std::string name = nameChars.c_str(); @@ -140,96 +128,71 @@ static jobjectArray android_view_InputChannel_nativeOpenInputChannelPair(JNIEnv* return nullptr; } - jobjectArray channelPair = env->NewObjectArray(2, gInputChannelClassInfo.clazz, nullptr); + jlongArray channelPair = env->NewLongArray(2); + if (channelPair == nullptr) { + return nullptr; + } + + jlong* outArray = env->GetLongArrayElements(channelPair, 0); + outArray[0] = android_view_InputChannel_createInputChannel(env, serverChannel); if (env->ExceptionCheck()) { return nullptr; } - jobject serverChannelObj = android_view_InputChannel_createInputChannel(env, serverChannel); + outArray[1] = android_view_InputChannel_createInputChannel(env, clientChannel); if (env->ExceptionCheck()) { return nullptr; } + env->ReleaseLongArrayElements(channelPair, outArray, 0); - jobject clientChannelObj = android_view_InputChannel_createInputChannel(env, clientChannel); - if (env->ExceptionCheck()) { - return nullptr; - } - - env->SetObjectArrayElement(channelPair, 0, serverChannelObj); - env->SetObjectArrayElement(channelPair, 1, clientChannelObj); return channelPair; } -static void android_view_InputChannel_nativeDispose(JNIEnv* env, jobject obj, jboolean finalized) { +static void InputChannel_nativeDestroy(void *rawInputChannel) { NativeInputChannel* nativeInputChannel = - android_view_InputChannel_getNativeInputChannel(env, obj); + reinterpret_cast(rawInputChannel); if (nativeInputChannel) { - if (finalized) { - ALOGW("Input channel object '%s' was finalized without being disposed!", - nativeInputChannel->getInputChannel()->getName().c_str()); - } + delete nativeInputChannel; + } +} +static jlong android_view_InputChannel_getNativeFinalizer(JNIEnv* env, jobject obj) { + return static_cast(reinterpret_cast(&InputChannel_nativeDestroy)); +} + +static void android_view_InputChannel_nativeDispose(JNIEnv* env, jobject obj, jlong channel) { + NativeInputChannel* nativeInputChannel = + reinterpret_cast(channel); + + if (nativeInputChannel) { nativeInputChannel->invokeAndRemoveDisposeCallback(env, obj); - - android_view_InputChannel_setNativeInputChannel(env, obj, nullptr); - delete nativeInputChannel; } } -static void android_view_InputChannel_nativeRelease(JNIEnv* env, jobject obj, jboolean finalized) { - NativeInputChannel* nativeInputChannel = - android_view_InputChannel_getNativeInputChannel(env, obj); - if (nativeInputChannel) { - android_view_InputChannel_setNativeInputChannel(env, obj, nullptr); - delete nativeInputChannel; - } -} - -static void android_view_InputChannel_nativeTransferTo(JNIEnv* env, jobject obj, - jobject otherObj) { - if (android_view_InputChannel_getNativeInputChannel(env, otherObj) != nullptr) { - jniThrowException(env, "java/lang/IllegalStateException", - "Other object already has a native input channel."); - return; - } - - NativeInputChannel* nativeInputChannel = - android_view_InputChannel_getNativeInputChannel(env, obj); - android_view_InputChannel_setNativeInputChannel(env, otherObj, nativeInputChannel); - android_view_InputChannel_setNativeInputChannel(env, obj, nullptr); -} - -static void android_view_InputChannel_nativeReadFromParcel(JNIEnv* env, jobject obj, +static jlong android_view_InputChannel_nativeReadFromParcel(JNIEnv* env, jobject obj, jobject parcelObj) { - if (android_view_InputChannel_getNativeInputChannel(env, obj) != nullptr) { - jniThrowException(env, "java/lang/IllegalStateException", - "This object already has a native input channel."); - return; - } - Parcel* parcel = parcelForJavaObject(env, parcelObj); if (parcel) { bool isInitialized = parcel->readInt32(); if (isInitialized) { sp inputChannel = InputChannel::read(*parcel); - NativeInputChannel* nativeInputChannel = new NativeInputChannel(inputChannel); - - android_view_InputChannel_setNativeInputChannel(env, obj, nativeInputChannel); + return reinterpret_cast(nativeInputChannel); } } + return 0; } static void android_view_InputChannel_nativeWriteToParcel(JNIEnv* env, jobject obj, - jobject parcelObj) { + jobject parcelObj, jlong channel) { Parcel* parcel = parcelForJavaObject(env, parcelObj); if (parcel == nullptr) { ALOGE("Could not obtain parcel for Java object"); return; } - NativeInputChannel* nativeInputChannel = - android_view_InputChannel_getNativeInputChannel(env, obj); + reinterpret_cast(channel); + if (!nativeInputChannel) { parcel->writeInt32(0); // not initialized return; @@ -238,9 +201,9 @@ static void android_view_InputChannel_nativeWriteToParcel(JNIEnv* env, jobject o nativeInputChannel->getInputChannel()->write(*parcel); } -static jstring android_view_InputChannel_nativeGetName(JNIEnv* env, jobject obj) { +static jstring android_view_InputChannel_nativeGetName(JNIEnv* env, jobject obj, jlong channel) { NativeInputChannel* nativeInputChannel = - android_view_InputChannel_getNativeInputChannel(env, obj); + reinterpret_cast(channel); if (! nativeInputChannel) { return nullptr; } @@ -249,18 +212,19 @@ static jstring android_view_InputChannel_nativeGetName(JNIEnv* env, jobject obj) return name; } -static void android_view_InputChannel_nativeDup(JNIEnv* env, jobject obj, jobject otherObj) { +static jlong android_view_InputChannel_nativeDup(JNIEnv* env, jobject obj, jlong channel) { NativeInputChannel* nativeInputChannel = - android_view_InputChannel_getNativeInputChannel(env, obj); + reinterpret_cast(channel); + if (nativeInputChannel == nullptr) { jniThrowRuntimeException(env, "InputChannel has no valid NativeInputChannel"); - return; + return 0; } sp inputChannel = nativeInputChannel->getInputChannel(); if (inputChannel == nullptr) { jniThrowRuntimeException(env, "NativeInputChannel has no corresponding InputChannel"); - return; + return 0; } sp dupInputChannel = inputChannel->dup(); if (dupInputChannel == nullptr) { @@ -268,13 +232,12 @@ static void android_view_InputChannel_nativeDup(JNIEnv* env, jobject obj, jobjec "Could not duplicate input channel %s", inputChannel->getName().c_str()); jniThrowRuntimeException(env, message.c_str()); } - android_view_InputChannel_setNativeInputChannel(env, otherObj, - new NativeInputChannel(dupInputChannel)); + return reinterpret_cast(new NativeInputChannel(dupInputChannel)); } -static jobject android_view_InputChannel_nativeGetToken(JNIEnv* env, jobject obj) { +static jobject android_view_InputChannel_nativeGetToken(JNIEnv* env, jobject obj, jlong channel) { NativeInputChannel* nativeInputChannel = - android_view_InputChannel_getNativeInputChannel(env, obj); + reinterpret_cast(channel); if (nativeInputChannel) { return javaObjectForIBinder(env, nativeInputChannel->getInputChannel()->getConnectionToken()); @@ -286,23 +249,21 @@ static jobject android_view_InputChannel_nativeGetToken(JNIEnv* env, jobject obj static const JNINativeMethod gInputChannelMethods[] = { /* name, signature, funcPtr */ - { "nativeOpenInputChannelPair", "(Ljava/lang/String;)[Landroid/view/InputChannel;", + { "nativeOpenInputChannelPair", "(Ljava/lang/String;)[J", (void*)android_view_InputChannel_nativeOpenInputChannelPair }, - { "nativeDispose", "(Z)V", + { "nativeGetFinalizer", "()J", + (void*)android_view_InputChannel_getNativeFinalizer }, + { "nativeDispose", "(J)V", (void*)android_view_InputChannel_nativeDispose }, - { "nativeRelease", "()V", - (void*)android_view_InputChannel_nativeRelease }, - { "nativeTransferTo", "(Landroid/view/InputChannel;)V", - (void*)android_view_InputChannel_nativeTransferTo }, - { "nativeReadFromParcel", "(Landroid/os/Parcel;)V", + { "nativeReadFromParcel", "(Landroid/os/Parcel;)J", (void*)android_view_InputChannel_nativeReadFromParcel }, - { "nativeWriteToParcel", "(Landroid/os/Parcel;)V", + { "nativeWriteToParcel", "(Landroid/os/Parcel;J)V", (void*)android_view_InputChannel_nativeWriteToParcel }, - { "nativeGetName", "()Ljava/lang/String;", + { "nativeGetName", "(J)Ljava/lang/String;", (void*)android_view_InputChannel_nativeGetName }, - { "nativeDup", "(Landroid/view/InputChannel;)V", + { "nativeDup", "(J)J", (void*)android_view_InputChannel_nativeDup }, - { "nativeGetToken", "()Landroid/os/IBinder;", + { "nativeGetToken", "(J)Landroid/os/IBinder;", (void*)android_view_InputChannel_nativeGetToken }, }; @@ -315,9 +276,6 @@ int register_android_view_InputChannel(JNIEnv* env) { gInputChannelClassInfo.mPtr = GetFieldIDOrDie(env, gInputChannelClassInfo.clazz, "mPtr", "J"); - gInputChannelClassInfo.ctor = GetMethodIDOrDie(env, gInputChannelClassInfo.clazz, "", - "()V"); - return res; }