twilio / voice-quickstart-android

Quickstart app for the Voice Android SDK
https://www.twilio.com/docs/api/voice-sdk/android/getting-started
MIT License
184 stars 140 forks source link

[Resolved] `Parcel.readParcelable` returns null when reading a parcelled `CallInvite` (from FCM) #561

Closed cybex-dev closed 1 year ago

cybex-dev commented 1 year ago

From an adapted quickstart example, I added the following snippet (see // SNIPPET tags below).

You will notice the CallInvite read from the Parcel is null, this serves to emulate the functionality provided by TelecomManager.addNewIncomingCall(PhoneAccountHandle, Bundle) by providing the CallInvite as an extra in the bundle.

Code snippet with Parcel

public class VoiceFirebaseMessagingService extends FirebaseMessagingService {

    @Override
    public void onMessageReceived(final RemoteMessage remoteMessage) {
        // ... valid message
        handleInvite(callInvite, notificationId);
        //...
    }

    private void handleInvite(CallInvite callInvite, int notificationId) {
        // SNIPPET START
        Parcel p = Parcel.obtain();
        p.writeParcelable(callInvite, 0);
        ClassLoader classLoader = getApplicationContext().getClassLoader();
        CallInvite ci = p.readParcelable(classLoader); // <----------------- ci is always null
        if(ci != null){
            Log.d(TAG, "handleInvite: " + ci.getCallSid());
        } else {
            Log.d(TAG, "handleInvite: null");
        }
        // SNIPPET END
    }

}

The same occurs on with Kotlin, though I get a ClassNotFoundException for CallInvite.

So far, I've traced the issue to Parcel.readParcelableCreator(@Nullable ClassLoader) (for Android 11, API 30)

public final Parcelable.Creator<?> readParcelableCreator(@Nullable ClassLoader loader) {
        String name = readString();
        if (name == null) {
            return null;
        }
        Parcelable.Creator<?> creator;

name is always null, thus returns null when calling Parcel.readParcelable.

This occurs on both Android 11 & 12 (API 30, 31)

Tested on:

cybex-dev commented 1 year ago

Possibly related #345

cybex-dev commented 1 year ago

Possibly related #370

cybex-dev commented 1 year ago

@afalls-twilio if you have any questions or would like me to test something, feel free to post as I'm actively working on this ConnectionService migration and will keep a close eye on this issue.

afalls-twilio commented 1 year ago

@cybex-dev Thank you for the report, this is probably because handleInvite(..) wasn't being called on the same thread as jni_onload, and as such, the java classloader is only associated with the system class loader. Something related to this, (https://developer.android.com/training/articles/perf-jni#faq_FindClass) maybe?

This usually does what you want. You can get into trouble if you create a thread yourself (perhaps by calling pthread_create and then attaching it with AttachCurrentThread). Now there are no stack frames from your application. If you call FindClass from this thread, the JavaVM will start in the "system" class loader instead of the one associated with your application, so attempts to find app-specific classes will fail.

One idea could be to capture the class loader from the main activity's onCreate, where the Voice SDK is first invoked. Then see if this class loader still fails to find objects beyond the system classes? Also, maybe another hack/test is to hard code a class and see if it can find it, such as "com.twilio.voice.CallInvite"...

cybex-dev commented 1 year ago

@cybex-dev Thank you for the report, this is probably because handleInvite(..) wasn't being called on the same thread as jni_onload, and as such, the java classloader is only associated with the system class loader. Something related to this, (https://developer.android.com/training/articles/perf-jni#faq_FindClass) maybe?

This usually does what you want. You can get into trouble if you create a thread yourself (perhaps by calling pthread_create and then attaching it with AttachCurrentThread). Now there are no stack frames from your application. If you call FindClass from this thread, the JavaVM will start in the "system" class loader instead of the one associated with your application, so attempts to find app-specific classes will fail.

One idea could be to capture the class loader from the main activity's onCreate, where the Voice SDK is first invoked. Then see if this class loader still fails to find objects beyond the system classes? Also, maybe another hack/test is to hard code a class and see if it can find it, such as "com.twilio.voice.CallInvite"...

Awesome - thanks for the quick response.

Fwiw I tried:

  1. a custom TwilioClassLoader extends ClassLoader (with all super.* calls)
public class TwilioClassLoader extends ClassLoader {
    protected TwilioClassLoader(ClassLoader parent) {
        super(parent);
    }

    protected TwilioClassLoader() {
        super();
    }

    //...

    @Override
    public Class<?> loadClass(String name) throws ClassNotFoundException {
        return super.loadClass(name);
    }

    //...

    @Override
    protected Class<?> findClass(String name) throws ClassNotFoundException {
        return super.findClass(name);
    }

    //...
}

and added in handleInvite with:

//...
p.writeParcelable(callInvite, 0);
TwilioClassLoader classLoader = new TwilioClassLoader();
CallInvite ci = p.readParcelable(classLoader);
//...

but as expected, ci remained null.

  1. Grabbed currentThread's ClassLoader, as expected ci is null as you suggested:
p.writeParcelable(callInvite, 0);
ClassLoader classLoader = Thread.currentThread().getClass().getClassLoader();
CallInvite ci = p.readParcelable(classLoader);
  1. Finally, I tried CallInvite's ClassLoader (not sure if this is the intended way), ci is null as expected.
p.writeParcelable(callInvite, 0);
ClassLoader callInviteClassLoader = CallInvite.class.getClassLoader();
CallInvite ci = p.readParcelable(callInviteClassLoader);

Had a BP on CallInvite.CREATOR, noticed it unparcels as expected in IncomingCallNotificationService's onStartCommand as expected.

cybex-dev commented 1 year ago

It would seem the current thread "can" find the class com.twilio.voice.CallInvite with the current snippet inside of handleInvite:

Parcel p = Parcel.obtain();
p.writeParcelable(callInvite, 0);
try {
    Class<?> aClass = Class.forName("com.twilio.voice.CallInvite");
    Log.i(TAG, "CallInvite class found");
} catch (ClassNotFoundException e) {
    Log.d(TAG, "onStartCommand: ClassNotFound");
}
ClassLoader callInviteClassLoader = CallInvite.class.getClassLoader();
CallInvite ci = p.readParcelable(callInviteClassLoader);

LogCat includes:

2023-08-12 00:14:15.364 26945-26945 FlutterFcmService       com.twilio.twilio_voice_example      I  CallInvite class found
cybex-dev commented 1 year ago

Tried using the CallInvite and Thread.currentThread() classloaders to find the com.twilio.voice.CallInvite class with the code below:


// ...
p.writeParcelable(callInvite, 0);

// This does not work, ClassNotFoundException with `loadClass(String)`
try {
    ClassLoader currentThreadClassLoader = Thread.currentThread().getClass().getClassLoader();
    if(currentThreadClassLoader != null) {
        Class<?> aClass = currentThreadClassLoader.loadClass("com.twilio.voice.CallInvite");
        if (aClass != null) {
            Log.d(TAG, "handleInvite: " + aClass.getName());
        } else {
            Log.d(TAG, "handleInvite: aClass is null");
        }
    }

    Class<?> aClass = Class.forName("com.twilio.voice.CallInvite");
    Log.i(TAG, "CallInvite class found");
} catch (ClassNotFoundException e) {
    // Exception caught here
    Log.d(TAG, "onStartCommand: ClassNotFound");
}

// This finds the class
try {
    ClassLoader callInviteClassLoader = callInvite.getClass().getClassLoader();
    if(callInviteClassLoader != null) {
        Class<?> aClass1 = callInviteClassLoader.loadClass("com.twilio.voice.CallInvite");
        if (aClass1 != null) {
            Log.d(TAG, "handleInvite: " + aClass1.getName());
        } else {
            Log.d(TAG, "handleInvite: aClass1 is null");
        }
    }

    Class<?> aClass = Class.forName("com.twilio.voice.CallInvite");
    Log.i(TAG, "CallInvite class found");
} catch (ClassNotFoundException e) {
    Log.d(TAG, "onStartCommand: ClassNotFound");
}

CallInvite ci = p.readParcelable();
if(ci != null){
    Log.d(TAG, "handleInvite: " + ci.getCallSid());
} else {
    Log.d(TAG, "handleInvite: null");
}

Logcat Entries:

FlutterFcmService       com.twilio.twilio_voice_example      D  handleInvite: com.twilio.voice.CallInvite
FlutterFcmService       com.twilio.twilio_voice_example      D  handleInvite: [Thread.currentThread] ClassNotFound
FlutterFcmService       com.twilio.twilio_voice_example      D  handleInvite: [callInvite.getClass().getClassLoader()] com.twilio.voice.CallInvite
FlutterFcmService       com.twilio.twilio_voice_example      I  handleInvite: [Class.forName] CallInvite class found
FlutterFcmService       com.twilio.twilio_voice_example      D  handleInvite: null
cybex-dev commented 1 year ago

Something I'd like to point out, the Intent's getParcelableExtra (deprecated, since Android 33) and Parcel readParcelableExtra has 2 different 'starting' callstacks but both coalesce higher up to call native functions.

Intent's Callstack

This callstack is observed from the IncomingCallNotificationService's onStartCommand

from here on, the call stack is the same.

enter image description here


Parcel Callstack

This callstack originates from following the following code (shown in detail in original post)

p.writeParcelable(callInvite, 0);
ClassLoader classLoader = getApplicationContext().getClassLoader();
CallInvite ci = p.readParcelable(classLoader);

Note however, the ClassLoader has no effect on the outcome, as shown below by the callstack and discussion.

Callstack discussion

in the Parcel.readParcelable function, we have

public final <T extends Parcelable> T readParcelable(@Nullable ClassLoader loader) {
    Parcelable.Creator<?> creator = readParcelableCreator(loader);                                // Line #L3272
    ...

calling Parcel.readParcelableCreator, with the snippet:

public final Parcelable.Creator<?> readParcelableCreator(@Nullable ClassLoader loader) {
    String name = readString();                                                                                          // Line #L3312
    if (name == null) {
        return null;
    }
    Parcelable.Creator<?> creator;
    //...

Note, the loader at this point is never used.

This follows on to Parcel.readString():

public final String readString() {
    return readString16();
}

onwards...

The big difference between Intent.getParcelableExtra and Parcel.readParcelable (where the problem lies) is in what name resolves as, since Parcel.readParcelable immediately returns null (which is assigned to ci).

From the above examples:

It seems, the ReadWriteHelper in Parcel isn't finding the class name, i.e.

File: Parcel.java
public final @Nullable String readString16() {
    return mReadWriteHelper.readString16(this);
}
cybex-dev commented 1 year ago

Since Parcel writes the class name before the object's fields, etc, I looked into this as the readString() from Parcel.readParcelable was always null, resulting in no class being identified and the CallInvite wasn't unparcelled.

Following the callstack using the snippet:

Parcel p = Parcel.obtain();
p.writeParcelable(callInvite, 0);

Callstack

Thus, so far it seems either writeString(name) isn't successful or String name = readString() is failing.

cybex-dev commented 1 year ago

So, this is slightly embarrassing...

I noticed something in the Parcel.readString() documentation:

Read a string value from the parcel at the current dataPosition()

I modified the snippet with Parcel.setDataPosition():

p.writeParcelable(callInvite, 0);
p.setDataPosition(0);                                             // Added this
CallInvite ci = p.readParcelable(getApplicationContext().getClassLoader());

CallInvite is read as expected.

cybex-dev commented 1 year ago

On a last note, the original issue well-documented here had me simplifying the issue (in Java) on this Github issue. However, since I resolved setDataPosition issue for reading the Parcelable CallInvite, the overarching issue has not yet been resolved. That is, when passing a CallInvite into a Bundle and attempting to read it in the ConnectionService, I get an exception;

Class not found when unmarshalling: com.twilio.voice.CallInvite
java.lang.ClassNotFoundException: com.twilio.voice.CallInvite
    at java.lang.Class.classForName(Native Method)
    at java.lang.Class.forName(Class.java:454)
    at android.os.Parcel.readParcelableCreator(Parcel.java:3403)
    //...

Some of these functions may look familiar going by my study of Parcel.java in the above issue

Original Problem

My Kotlin code (testing out the ConnectionService) works with:

private fun handleFCMCallInvite(callInvite: CallInvite, notificationId: Int) {
    // Get telecom manager
    val telecomManager = getSystemService(TELECOM_SERVICE) as TelecomManager

    // Get PhoneAccountHandle
    val caller = callInvite.from!!.toString()
    val componentName = ComponentName(applicationContext.packageName, TwilioVoiceConnectionService::class.java.name)
    val phoneAccountHandle = PhoneAccountHandle(componentName, caller)

    // Create my Bundle containing information e.g. notificationId and callInvite
    val myBundle = Bundle()
    myBundle.putInt(Constants.INCOMING_CALL_NOTIFICATION_ID, notificationId)
    myBundle.putParcelable(Constants.INCOMING_CALL_INVITE, callInvite)

    // Add new incoming call to the telecom manager
    telecomManager.addNewIncomingCall(phoneAccountHandle, Bundle().apply {
        putBundle(EXTRA_INCOMING_CALL_EXTRAS, myBundle)
        putParcelable(EXTRA_PHONE_ACCOUNT_HANDLE, phoneAccountHandle)
    })
}

Followed by reading the extras from the Bundle in the TwilioVoiceConnectionService.kt as follows:

override fun onCreateIncomingConnection(connectionManagerPhoneAccount: PhoneAccountHandle?, request: ConnectionRequest?): Connection {
    super.onCreateIncomingConnection(connectionManagerPhoneAccount, request)
    Log.d(TAG, "onCreateIncomingConnection")
    val connection: Connection = VoipConnection(applicationContext)
    connection.extras = request?.extras

    var ci: CallInvite? = null
    val myBundle: Bundle? = connection.extras.getBundle(TelecomManager.EXTRA_INCOMING_CALL_EXTRAS);
    if(myBundle != null) {
        Log.d(TAG, "onCreateIncomingConnection: myBundle is not null")

        /// --------------------------------------------------------
        /// This next line throws the ClassNotFoundException occurs
        /// --------------------------------------------------------
        if (myBundle.containsKey(Constants.INCOMING_CALL_INVITE) ) { 
            Log.d(TAG, "onCreateIncomingConnection: myBundle contains INCOMING_CALL_INVITE")
            ci = myBundle.getParcelable(Constants.INCOMING_CALL_INVITE)
        } else {
            Log.d(TAG, "onCreateIncomingConnection: myBundle does not contain INCOMING_CALL_INVITE")
        }
    } else {
        Log.d(TAG, "onCreateIncomingConnection: myBundle is null")
    }

    ...
}

In a manner of speaking @afalls-twilio, you were correct in saying the ClassLoader of the current thread doesn't seem to be setup correctly. This was confirmed and discussed in some detail by David Wasser at SO.

TL;DR Solution

A simple line addition when reading the bundle adds the class to be found i.e.

val myBundle: Bundle? = connection.extras.getBundle(TelecomManager.EXTRA_INCOMING_CALL_EXTRAS);
myBundle.classLoader = CallInvite::class.java.classLoader
// read CallInvite from Bundle as required

I suspect, this is a significant issue with Samsung devices by browsing through previous github repo issues including the SO thread mentioned earlier.

afalls-twilio commented 1 year ago

@cybex-dev Interesting... After doing some more digging, I discovered that when trying to unmarshal a Bundle object, one must set the ClassLoader if marshaling classes not provided by Android.

https://developer.android.com/reference/android/os/Bundle#getParcelable(java.lang.String)

For example...

Bundle requestExtras = request.getExtras().getParcelable(TelecomManager.EXTRA_INCOMING_CALL_EXTRAS);
requestExtras.setClassLoader(CallInvite.class.getClassLoader());
CallInvite invite = requestExtras.getParcelable(Constants.INCOMING_CALL_INVITE);
tunglthtv commented 10 months ago

Hi @cybex-dev please can you update this code for ver 0.0.9? We have got this issues on this.

cybex-dev commented 10 months ago

Hi @cybex-dev please can you update this code for ver 0.0.9? We have got this issues on this.

0.0.9?

tunglthtv commented 10 months ago

@cybex-dev sorry my mistake. I think that i am in twilio-voice sdk.