Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Android N: NoSuchMethodException Crash, caused by OpenJDK? #121

Closed
Wavesonics opened this issue Mar 10, 2016 · 37 comments
Closed

Android N: NoSuchMethodException Crash, caused by OpenJDK? #121

Wavesonics opened this issue Mar 10, 2016 · 37 comments

Comments

@Wavesonics
Copy link
Contributor

I'm not sure this is due to OpenJDK, but here's what I've found.

The Android N preview now uses OpenJDK, and when running my app using FST on the emulator using the latest N preview system image, I get the following crash:

Caused by: java.lang.NoSuchMethodException: newInstance [class java.lang.Class, long]
  at java.lang.Class.getMethod(Class.java:2016)
  at java.lang.Class.getDeclaredMethod(Class.java:1995)
  at org.objenesis.instantiator.android.Android18Instantiator.getNewInstanceMethod(Android18Instantiator.java:53)
  at org.objenesis.instantiator.android.Android18Instantiator.<init>(Android18Instantiator.java:38) 
  at org.objenesis.strategy.StdInstantiatorStrategy.newInstantiatorOf(StdInstantiatorStrategy.java:86) 
  at org.objenesis.ObjenesisBase.getInstantiatorOf(ObjenesisBase.java:91) 
  at org.nustaq.serialization.FSTObjenesisInstantiator.<init>(FSTObjenesisInstantiator.java:38) 
  at org.nustaq.serialization.FSTConfiguration$4.getInstantiator(FSTConfiguration.java:372) 
  at org.nustaq.serialization.FSTClazzInfo.<init>(FSTClazzInfo.java:129) 
  at org.nustaq.serialization.FSTClazzInfoRegistry.getCLInfo(FSTClazzInfoRegistry.java:130) 
  at org.nustaq.serialization.FSTClazzNameRegistry.addClassMapping(FSTClazzNameRegistry.java:98) 
  at org.nustaq.serialization.FSTClazzNameRegistry.registerClassNoLookup(FSTClazzNameRegistry.java:85) 
  at org.nustaq.serialization.FSTClazzNameRegistry.registerClass(FSTClazzNameRegistry.java:81) 
  at org.nustaq.serialization.FSTConfiguration.addDefaultClazzes(FSTConfiguration.java:773) 
  at org.nustaq.serialization.FSTConfiguration.initDefaultFstConfigurationInternal(FSTConfiguration.java:445) 
  at org.nustaq.serialization.FSTConfiguration.createAndroidDefaultConfiguration(FSTConfiguration.java:375) 
  at org.nustaq.serialization.FSTConfiguration.createDefaultConfiguration(FSTConfiguration.java:438) 
  at org.nustaq.serialization.FSTConfiguration.createDefaultConfiguration(FSTConfiguration.java:433) 
  at org.nustaq.serialization.FSTConfiguration.getDefaultConfiguration(FSTConfiguration.java:185) 
  at org.nustaq.serialization.FSTObjectOutput.<init>(FSTObjectOutput.java:74) 
  at ...
@Wavesonics Wavesonics changed the title Possible crash with OpenJDK NoSuchMethodException Crash, caused by OpenJDK? Mar 10, 2016
@RuedigerMoeller
Copy link
Owner

fst detects android and uses objenesis library to instantiate classes then. Once android uses openjdk, this is not required anymore and one could use the default instantiator.

to fix this, openjdk on android must be detected and the default jdk instantiator should get used. I'd say this should be fixed in objenesis, however it could be workarounded in fst as well.

@Wavesonics
Copy link
Contributor Author

Looks like they may have fixed this in the latest version if objenesis: https://github.com/easymock/objenesis/releases/tag/2.2

New SearchWorkingInstantiator allowing to identify which instantiator can work on a platform

So we should just need to update to 2.2?

@RuedigerMoeller
Copy link
Owner

try it, I don't use fst myself on android :). If it works I'll change dependencies and release 2.46

@Wavesonics
Copy link
Contributor Author

Hhmmm so I tried this:

    compile('de.ruedigermoeller:fst:2.45') {
        exclude group: 'org.objenesis', module: 'objenesis'
    }
    compile 'org.objenesis:objenesis:2.2'

And still get the same crash, so looks like maybe it is not fixed up stream.

@Wavesonics
Copy link
Contributor Author

Actually looking deeper at the FST gradle file, there are a lot of explicit references to 2.1, so my hack may not have worked.

@RuedigerMoeller
Copy link
Owner

I am using maven for the standard build. Can you give me a snipped of code which reliably detects OpenJDK running on Android ?
Currently there is an Android check only, but for the future I need to 1st detect Android in order to use an Android-Instantiation Strategy + Configuration, then I need to detect wether its the OpenJDK Android internally moving to native FST Object Instantiation strategy.

@Wavesonics
Copy link
Contributor Author

Android API 24 and above will be based on OpenJDK. However if you need a way to detect it without compiling against Android symbols, I'm not sure.

It did look like Objenesis atleast tried to solve this:
easymock/objenesis#34

@RuedigerMoeller
Copy link
Owner

should be possible using system properties like "os.name" etc. could you dump and post system properties with openjdk/android n ?

@Wavesonics
Copy link
Contributor Author

Ok so os.name isn't helpful:

os.name: 'Linux'

Here are some values that you could use to detect Android in general:

java.vm.name: 'Dalvik'
java.vm.specification.name: 'Dalvik Virtual Machine Specification'
java.vendor: 'The Android Project'
java.runtime.name: 'Android Runtime'

Even ART runtimes report as Dalvik. So no help there.

The VM Version reports as the same value between 6.0 (JDK) and 6.1 (OpenJDK)

java.vm.version: '2.1.0'

Even the library reports the same

java.specification.name: 'Dalvik Core Library'

So tl;dr: I haven't found anything you can switch on yet.

However, could you use reflection to see if the OpenJDK instantiate method exists?

@Wavesonics
Copy link
Contributor Author

Ok idk how I feel about this, but I dumped all the system properties and think I may have found one to switch on:
java.boot.class.path contains a new jar file which is the custom OpenJDK that they compiled: core-oj.jar or more completely /system/framework/core-oj.jar

I confirmed this does not exist on previous versions of Android, and even found the check-in where this was added which confirms it is what contains the OpenJDK classes.

The full value of java.boot.class.path is as such:
/system/framework/core-oj.jar:/system/framework/core-libart.jar:/system/framework/conscrypt.jar:/system/framework/okhttp.jar:/system/framework/core-junit.jar:/system/framework/bouncycastle.jar:/system/framework/ext.jar:/system/framework/framework.jar:/system/framework/telephony-common.jar:/system/framework/voip-common.jar:/system/framework/ims-common.jar:/system/framework/apache-xml.jar:/system/framework/org.apache.http.legacy.boot.jar

If you do switch based on this I recommend just using the jar file name and not it's full path as OEMs have been known to mess with things like that.

Edit: Since this is an Android specific way of detecting the OpenJDK, I would probably pair a check for that jar along with a check of java.runtime.name == 'Android Runtime' or something...

@yuriymyronovych
Copy link

Hi guys, can you help me out on this as from this thread I am still not clear how would you fix this one. I assume I have to do hacks myself?

Thanks,
Y

@yuriymyronovych
Copy link

Assuming I have to do fix myself, Wavesonics, any chance you might share code you used to fix this one?

@yuriymyronovych
Copy link

RuedigerMoeller, in your replies you kind of implied you are planning to fix it in 2.46, is this still the plan?

@RuedigerMoeller
Copy link
Owner

With Android N, fst actually does not need to make use of Objenesis.

in FSTConfiguration.java

protected static FSTConfiguration createAndroidDefaultConfiguration(ConcurrentHashMap<FieldKey,FSTClazzInfo.FSTFieldInfo> shared) {
        final Objenesis genesis = new ObjenesisStd();
        FSTConfiguration conf = new FSTConfiguration(shared) {
            @Override
            public FSTClassInstantiator getInstantiator(Class clazz) {
                return new FSTObjenesisInstantiator(genesis,clazz); // <= this is not required for ANDROID N
            }
        };
[...]

so in case Android N is detected (System props or so, see above) you can do
plain conf = new FSTConfiguration(shared);.

@Wavesonics
Copy link
Contributor Author

OK I spent about 30 seconds trying to do this in the library it's self, but apparently the version I forked doesn't contain the createAndroidDefaultConfiguration method for some reason? Not sure, but here is the Android N/OpenJDK detection code, I tested this on a few emulators and it works properly:

    public static boolean hasOpenJdk()
    {
        final String classPath = System.getProperty( "java.boot.class.path" );
        return classPath.toLowerCase().contains( "core-oj.jar" );
    }

In order to work around the fact that I didn't do this in the library it's self, I rolled that into a larger class as such:

package org.nustaq.serialization;

import org.nustaq.serialization.serializers.FSTMapSerializer;

import java.util.concurrent.ConcurrentHashMap;

public class AndroidFSTConfiguration extends FSTConfiguration
{
    protected AndroidFSTConfiguration( final ConcurrentHashMap<FieldKey, FSTClazzInfo.FSTFieldInfo> sharedFieldInfos )
    {
        super( sharedFieldInfos );
    }

    public static FSTConfiguration create( final ConcurrentHashMap<FieldKey, FSTClazzInfo.FSTFieldInfo> sharedFieldInfos )
    {
        final FSTConfiguration conf;
        if( hasOpenJdk() )
        {
            conf = new FSTConfiguration( sharedFieldInfos );
            initDefaultFstConfigurationInternal(conf);
            if ( isAndroid ) {
                try {
                    conf.registerSerializer( Class.forName("com.google.gson.internal.LinkedTreeMap"), new FSTMapSerializer(), true);
                } catch (ClassNotFoundException e) {
                    //silent
                }
                try {
                    conf.registerSerializer(Class.forName("com.google.gson.internal.LinkedHashTreeMap"), new FSTMapSerializer(), true);
                } catch (ClassNotFoundException e) {
                    //silent
                }
            }
        }
        else
        {
            conf = FSTConfiguration.createAndroidDefaultConfiguration( sharedFieldInfos );
        }

        return conf;
    }

    public static boolean hasOpenJdk()
    {
        final String classPath = System.getProperty( "java.boot.class.path" );
        return classPath.toLowerCase().contains( "core-oj.jar" );
    }
}

And I use that to create a config:

private static final FSTConfiguration s_fst = AndroidFSTConfiguration.create( null );

Which I in turn use create my FST Streams:

FSTObjectOutput oos = s_fst.getObjectOutput( outputStream );

And that all works, no more crashing on Android N.

@RuedigerMoeller
Copy link
Owner

Good!

However this won't work when using a fst server and you have mixed OpenJDK / Dalvik clients. but i will incorporate the "idea" of your fix into fst 2.46

@RuedigerMoeller RuedigerMoeller changed the title NoSuchMethodException Crash, caused by OpenJDK? NoSuchMethodException Crash, caused by OpenJDK? (Android N) Apr 13, 2016
@RuedigerMoeller RuedigerMoeller changed the title NoSuchMethodException Crash, caused by OpenJDK? (Android N) Android N: NoSuchMethodException Crash, caused by OpenJDK? Apr 13, 2016
@yuriymyronovych
Copy link

Thank you guys, that is very helpful.

@narayank
Copy link

Android Maintainer here : I was about to contact you folks about this.

(1) Please use sun.misc.Unsafe.allocateInstance on OpenJDK based Androids. That's what the Sun instantiator for objenesis will do (eventually) but not before trying many other options that aren't available on Android.

(2) I'd strongly recommend against looking at the boot classpath system property. The safest way to get information about Android devices is by using the android.os.Build class. usesOpenJdk = (android.os.Build.VERSION.SDK_INT > 23)

@RuedigerMoeller
Copy link
Owner

RuedigerMoeller commented Apr 15, 2016

"sun.misc.Unsafe.allocateInstance" does not run default construction for transient fields.
e.g.

class SerializableClass impls Serzbe{
   transient String foo = "BAR";
}

will have "foo" set to null after deserialization .. when allocated with Unsafe

@RuedigerMoeller
Copy link
Owner

RuedigerMoeller commented Apr 15, 2016

@narayank regarding os detection: Pls prefer a System property for version detection as referencing Android specific classes is a hassle when not running on Android ..

@Wavesonics
Copy link
Contributor Author

@narayank it would be great if Android set some more detail in the standard Java System properties.

Specifically, Why do ART run times report as Dalvik?

java.vm.name: 'Dalvik'

java.vm.specification.name and even java.vm.version all remain constant over quite a few versions of Android.

@narayank
Copy link

We don't plan on exposing any additional properties about system versions etc, especially since the equivalent constants exist in android.*. System properties are very brittle - they're mutable and can easily be ovewritten by code that's trying to be too "smart" (and yes - i've seen several examples of this). Using reflection to probe the existence of a certain class or method is a few more lines of code but is a bit more foolproof.

Anyhow - re your comment above, are you saying sun.misc.Unsafe.allocateInstance isn't good enough for you ? Objenesis uses it on Sun VMs.

@henri-tremblay
Copy link

@narayank Do you know why sun.reflect.ReflectionFactory isn't in Android N since it is supposed to be an OpenJDK class? Also, Android N currently returns 23. Is that a bug that will be fixed?

@RuedigerMoeller
Copy link
Owner

RuedigerMoeller commented Apr 26, 2016

@narayank

"are you saying sun.misc.Unsafe.allocateInstance isn't good enough for you "

Its just wrong and so is Objenesis :), see my reasoning, that's not compatible to JDK serialization. That's why sun.reflect.ReflectionFactory is the best solution.

@henri-tremblay
Copy link

@RuedigerMoeller Objenesis should not be needed indeed but sadly is. Objenesis uses allocateInstance anyway in some cases. However, on OpenJDK it's not. It is using the ReflectionFactory because benchmarks proved it faster. And ObjectStreamClass for serialization.

In the case of Android N, it seems that ReflectionFactory isn't available. So I will use Unsafe and ObjectStreamClass.

But I was surprised to see ReflectionFactory disappear. Though my question to @narayank

@henri-tremblay
Copy link

To give you a heads up, right now I'm quite ready to deliver. I'm only still puzzled by the fact that the API level is still 23... So I'm currently using the bootclasspath trick.

@narayank
Copy link

The API level will be stuck at 23 (for complicated reasons I can't quite recall) until the Android N API is finalized.

I realize this is pretty awkward for folks doing things on the developer preview - you'll probably have to use "N".equals(VERSION.CODENAME) or your existing trick for the time being.

@henri-tremblay
Copy link

FWI, Objenesis 2.3 was released yesterday. It now support Android N

@Wavesonics
Copy link
Contributor Author

Sounds like just updating the dependency and doing a release might fix our issue without any of these work arounds.

@Wavesonics
Copy link
Contributor Author

The Android N public beta was released today making this a bit more of a pressing issue, @RuedigerMoeller would it be possible to upgrade to Objenesis 2.3 and do a release?

@GaelCougnaud
Copy link

Hi,

Our application use a third library, which has a dependency to fast-serialization.
With first Android N versions, this issue happens. But with the last release of Android N (Developper preview 3), this issue no longer exists.
Do you notice the same behavior ?

@RuedigerMoeller
Copy link
Owner

I'll update to objenesis 2.3 this week if you can confirm it fixes the issue. Still have some work/validation to do regarding unmodifiable related pull requests ..

@henri-tremblay
Copy link

And, BTW, objenesis 2.4 is now out.
Le 23 mai 2016 9:15 AM, "moru0011" notifications@github.com a écrit :

I'll update to objenesis 2.3 this week if you can confirm it fixes the
issue. Still have some work/validation to do regarding unmodifiable related
pull requests ..


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#121 (comment)

@RuedigerMoeller
Copy link
Owner

thanks :)

@RuedigerMoeller
Copy link
Owner

upgraded to objenesis 2.4 in fst 2.46

@Wavesonics
Copy link
Contributor Author

Sweet! Just confirmed this does indeed fix the crash on Android N!

@RuedigerMoeller
Copy link
Owner

:)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants