Closed lichtin closed 10 years ago
Can you provide a bit more information?
The problem itself has most likely something to do with private/public access problems or something like this.
2.23.0 works, it fails with 2.23.1 and 2.24.0. I'll look into private/public issues.
Another problem could be related to StdInstantiatorStrategy and DefaultInstantiatorStrategy (the way how Kryo uses them was changed recently). It could be that your nested class does not have no-arg constructor or something like this. Try to play with these two strategies and see what happens.
Thanks for the hints, the fix was to add no-arg ctors to the inner class.
You are welcome. There could be another fix for your problem. It would be nice, if you could test it:
((DefaultInstantiatorStrategy)kryo.getInstantiatorStrategy()).setFallbackInstantiatorStrategy(new StdInstantiatorStrategy());
The idea here is that if it is not possible to find a no-arg constructor using a default strategy, then it will use ASM-based StdInstantiatorStrategy, which does not require a class to have no-arg constructor. It creates class instances without invoking any constructor at all. This way there is no need for you to add no-arg constructors, which is sometimes just not possible.
Please let me know, if it works for you.
Unfortunately, and assuming I tested this correctly, setting that fallback strategy did not help..
I'm trying to reproduce your problem to check something. Could you provide me with the structure of your MyClazz and MyInnerClazz? I tried to mimic it, but I cannot get the same exception as you get. I always get other exceptions...
The structure looks like:
public interface FactI {
}
public abstract class FactAbst implements FactI {
}
public class FactStd extends FactAbst {
private Fact fact;
public FactStd() {
this(new Fact());
}
public FactStd(Fact fact) {
this.fact = fact;
}
protected static class Fact {
}
}
where MyClazz is FactStd and MyInnerClazz is Fact.
Thanks for providing the structure of your classes. I'm not able to reproduce your exception yet. I'm trying with this test:
@Test
public void testNestedClass1() {
Kryo kryo = new Kryo();
Output output = new Output(1024);
FactStd obj = new FactStd();
kryo.copy(obj);
kryo.writeClassAndObject(output, obj);
output.close();
Input input = new Input(output.toBytes());
FactStd obj1 = (FactStd)kryo.readClassAndObject(input);
}
But it works just fine for me using the latest Kryo version. Am I missing something?
I see the same. It works in a simple test environment. Need to figure out what the difference is.
I see the same. It works in a simple test environment. Need to figure out what the difference is.
Just a guess, but usually something like this may be related to class-loader issues.
Is your problem solved now?
Sorry I ran out of time to debug this. It works ok as long as I have a no-arg ctor to the inner class.
I have the same problem. My software is running in an OSGi container. I try to create an example to post but I'm not yet able. However by my test I see that the problem is on upgrading from reflectasm 1.05 to 1.07. The new version generate an IllegalAccessError that is not catched by following code in DefaultInstantiatorStrategy
try { final ConstructorAccess access = ConstructorAccess.get(type); return new ObjectInstantiator() { public Object newInstance () { try { return access.newInstance(); } catch (Exception ex) { throw new KryoException("Error constructing instance of class: " + className(type), ex); } } }; } catch (Exception ignored) { }
because the catch is for exception and the error is a Throwable. I think that is enough to change Exception in Throwable in the catch.
Regards Giuseppe
@ggerla Have you tried to apply your proposed change (Exception->Throwable) and see what happens? I guess IllegalAccessError is caught now, but your object is not constructed anyway, or? You just get now a KryoException. Or do I misunderstand what would happen?
I try to create an example to post but I'm not yet able.
Yes, an example would help a lot.
I didn't try yet. Now I'm traveling. I try it in the afternoon. However You have to catch throwable in the second one where You ignore the exception. I don't know if i'm able to prepar an example. It seems that a classic java application doesn't have this problem.
I don't know if i'm able to prepare an example. It seems that a classic java application doesn't have this problem.
I could change Exception to Throwable in Kryo. It is not a problem. But I want to be sure that it fixes the problem and objects can be created by Kryo. If this change just converts the caught Throwabe into KryoException, but does not help to create objects, then it does not solve the actual problem. This is why would be nice if you could either apply the change and check locally or provide a test, which I could use on my side.
What I see now is very interesting. To apply the change I checkout the kryo source at version 3a2eb7b3f3f04652e2dc40764c963f2bc99a92f5. I use mvn install to generate the jar and using it as dependency I can replicate the problem. To debug the change on the catch in eclipse I change the source and the target of maven-compiler-plugin in the pom to 1.7. In this case I have no problem. So I try with Java 1.6 and I have the same problem. So it seem that issue is due to java version. A possible way can be update the java version but I don't know if for you is the right way.
About the catch modification, you are right, it doesn't solve the problem. I try to investigate better, but my problem is to debug reflectasm part or give you an exemple to reproduce the problem. I'm working on it.
regards
Indeed, it sounds pretty interesting. It is interesting that it is dependent on the Java version.
Switching to 1.7 could be an option, but we would like to avoid it without a really big need for it. If it is possible to solve it differently, it would be better.
About the catch modification, you are right, it doesn't solve the problem.
That's a pity. I suspected that unfortunately it wouldn't help.
I try to investigate better, but my problem is to debug reflectasm part or give you an exemple to reproduce the problem. I'm working on it.
Thank's for trying to debug it and to provide an example.
BTW, debugging reflectasm should be also possible, I guess. After all JARs with sources should be also available and Eclipse should be able to fetch it. There was even a plugin that would fetch source JARs for you automatically.
@NathanSweet Do you have any ideas which of the changes in ReflectASM could result in such effects?
I'm not able to replicate the problem with a java application. Debugging kryo and reflectasm I see that the exception is throwed by MyCLassConstrucorAcces.newInstance() If I understand the code this class is retrieved using AccessClassLoader using super.loadClass(name, resolve); at row 76. In my case super has a parent named BundleClassLoaderJava5, so it could be that this class loader has a different behaviour respect BundleClassLoaderJava7.
I can't do more than this, but please ask me if you have some doubt
Great. I found the problem in reflectasm. Try the code below. I try that code with reflectasm 1.05 and 1.09. In the first case ConstructorAccess.get method thrw an exception, in the second no. This is because the new version check if the constructor is private and only in this case throw the exception. But my constructor is protected so not private. From my point of view the get method should check also protected constructor.
package kryo.issue.serializer;
import com.esotericsoftware.reflectasm.ConstructorAccess;
import kryo.test.serializable.ClassNotSerializable;
public class KryoIssue217 {
public static void main(String[] args) { try { final ConstructorAccess access = ConstructorAccess.get(ClassNotSerializable.class); ClassNotSerializable tp = (ClassNotSerializable) access.newInstance(); } catch (Exception ignored) { int i = 0; } }
package kryo.test.serializable;
import java.util.ArrayList; import java.util.List;
public class ClassNotSerializable {
private List
protected ClassNotSerializable() {
setList(new ArrayList
public ClassNotSerializable(String cts) {
setList(new ArrayList
public List
public void setList(List
Cool!
@NathanSweet Nate, now that the problem seems to be pinned down, could you check if the proposed solution (check for both private and protected) makes sense and can be incorporated into ReflectASM?
Hi all I apply my suggestion to reflectasm code, compile reflectasm and kryo and use new code in my software to do an integration test. I can confirm you that this solve the problem. If Nathan agree, I can commit my changes on github. I can fork the project and then make a poll request. Let me know....
Thanks Regards Giuseppe
Hi all,
I'm not able to reproduce it. The self-contained test avobe (and others) work ok for me on JDK 1.6, 1.7 and 1.8.
The must be any other reason or a combined reason. Seems like for any reason, in your environmnet/OSGi scenario, the classloader used to inject the Accessor is not the same as the one of the accessed class. So packages are different at runtime even if they have the same name.
So the Exception would happen to you even if you change the constructor scope to package-protected, right?
I suspect the problem can be in the assumption made in AccessClassLoader about if type.getClassLoader()==null, then use the SystemClassLoader (which is ok as per the JDK javadocs) but maybe not inside OSGi os whatever.
No time to test it, but I think this is the point...
Hi what I see in my debug scenario is that in the method getParentClassLoader after the call ClassLoader parent = type.getClassLoader(); parent is set to BundleClassLoaderJava5 so I never use the SystemClassLoader.
What I didn't understand is the different behaviour if Kryo is compiled for Java 1.7.
However why reflectasm check only private constructor and not protected?
Not much time right now, but I believe ReflectASM tries to create the accessor class in the same package, so it has access to protected members.
Good answer :-)
Ok, I prepare two osgi bundle project to reproduce the problem. I try two scenario: 1) I deploy two bundles in karaf with Kryo 2.23.0 -> All works fine 2) I deploy two bundles in karaf with Kryo 2.24.0 -> I have the IllegalAccessError
Let me know how can I send you my code and if you need support to use it :-)
Let me add that ConstructorAccess.java in reflectasm 1.07 at row 52 throw an exception with protected method. The equivalent of reflectasm 1.09 should be ConstructorAccess.java at row 53-54 but in this case the exception is throwed only for private.
@ggerla Can you see if you can reproduce the issue using https://github.com/magro/kryo-osgi-samples?
Ok, I have been able to reproduce: created two OSGi bundles, deployed it on Apache Felix, and got the Error when accessing one from the other.
I added this inside the "catch (Exception ignored)" block on AccessClassLoader.java line 87: System.err.println("Exception defining class: " + name); ignored.printStackTrace();
And I get the following err output:
Exception defining class: kryo.test.serializable.ClassNotSerializableConstructorAccess java.lang.reflect.InvocationTargetException at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:606) at com.esotericsoftware.reflectasm.AccessClassLoader.defineClass(AccessClassLoader.java:85) at com.esotericsoftware.reflectasm.ConstructorAccess.get(ConstructorAccess.java:85) at com.mycompany.osgi2.KryoIssue2171.main(KryoIssue2171.java:10) at com.mycompany.osgi2.Activator.start(Activator.java:10) at org.apache.felix.framework.util.SecureAction.startActivator(SecureAction.java:645) at org.apache.felix.framework.Felix.activateBundle(Felix.java:2154) at org.apache.felix.framework.Felix.startBundle(Felix.java:2072) at org.apache.felix.framework.Felix.setActiveStartLevel(Felix.java:1299) at org.apache.felix.framework.FrameworkStartLevelImpl.run(FrameworkStartLevelImpl.java:304) at java.lang.Thread.run(Thread.java:745) Caused by: java.lang.NoClassDefFoundError: com/esotericsoftware/reflectasm/ConstructorAccess at java.lang.ClassLoader.defineClass1(Native Method) at java.lang.ClassLoader.defineClass(ClassLoader.java:800) ... 14 more Caused by: java.lang.ClassNotFoundException: com.esotericsoftware.reflectasm.ConstructorAccess not found by com.mycompany.osgi1 [5] at org.apache.felix.framework.BundleWiringImpl.findClassOrResourceByDelegation(BundleWiringImpl.java:1556) at org.apache.felix.framework.BundleWiringImpl.access$400(BundleWiringImpl.java:77) at org.apache.felix.framework.BundleWiringImpl$BundleClassLoader.loadClass(BundleWiringImpl.java:1993) at java.lang.ClassLoader.loadClass(ClassLoader.java:358) ... 16 more
And then:
java.lang.IllegalAccessError: tried to access method kryo.test.serializable.ClassNotSerializable.
That is: as the accessed bundle doesn't have the dependence to ReflectASM, it can't be defined in its ClassLoader the accesor that extends ConstructorAccess (class not defined there).
Then goes to the default code that defines the class in the current ClassLoader, and that's why the newInstance throws the Error because istrying to access a protected method in a different package (same name but in different ClassLoaders).
This is the symptom, tomorrow let's think in how to catch/handle with it .... Now I'm going to bed :-)
Great!!! You are right!
I check that the same problem can be reproduced also with only one bundle...
Basing on your analisys let me suggest a solution: in ConstructorAccess.get method replace lines from 49 to 74 with
boolean isNonPublic = false;
if (!isNonStaticMemberClass) {
enclosingClassNameInternal = null;
try {
Constructor
In this way you check private constructor for standard Java application and also protected constructor for that application (like OSGi) in wich classloaders are different between reflectasm and type to instance.
Can be a good idea?
Umm, not exactly, in other scenarios class loaders can be hierarchically related (grandson, etc) and the access be legal.
De: Giuseppe Gerla [mailto:notifications@github.com] Enviado el: jueves, 17 de julio de 2014 11:08 Para: EsotericSoftware/kryo CC: Tumi Asunto: Re: [kryo] Seeing java.lang.IllegalAccessError when upgrading to 2.24.0 (#217)
Great!!! You are right!
I check that the same problem can be reproduced also with only one bundle...
Basing on your analisys let me suggest a solution: in ConstructorAccess.get method replace lines from 49 to 74 with
boolean isNonPublic = false; if (!isNonStaticMemberClass) { enclosingClassNameInternal = null; try { Constructor constructor = type.getDeclaredConstructor((Class[])null); isNonPublic = Modifier.isPrivate(constructor.getModifiers()) || (type.getClassLoader() != ConstructorAccess.class.getClassLoader() && Modifier.isProtected(constructor.getModifiers())); } catch (Exception ex) { throw new RuntimeException("Class cannot be created (missing no-arg constructor): " + type.getName(), ex); } if (isNonPublic) { throw new RuntimeException("Class cannot be created (the no-arg constructor is private): " + type.getName()); } } else { enclosingClassNameInternal = enclosingType.getName().replace('.', '/'); try { Constructor constructor = type.getDeclaredConstructor(enclosingType); // Inner classes should have this. isNonPublic = Modifier.isPrivate(constructor.getModifiers()) || (type.getClassLoader() != ConstructorAccess.class.getClassLoader() && Modifier.isProtected(constructor.getModifiers())); } catch (Exception ex) { throw new RuntimeException("Non-static member class cannot be created (missing enclosing class constructor): "
In this way you check private constructor for standard Java application and also protected constructor for that application (like OSGi) in wich classloaders are different between reflectasm and type to instance.
Can be a good idea?
— Reply to this email directly or view it on GitHub https://github.com/EsotericSoftware/kryo/issues/217#issuecomment-49278180 . https://github.com/notifications/beacon/6210680__eyJzY29wZSI6Ik5ld3NpZXM6QmVhY29uIiwiZXhwaXJlcyI6MTcyMTIwNzI5MywiZGF0YSI6eyJpZCI6MzE2MTM0NDZ9fQ==--0fcc02bb0117b657af289c9cdce51cb5923e8392.gif
Let me see it deeper later because I think my last response was not right, from the JLS:
At run time, a class or interface is determined not by its name alone, but by a pair: its fully qualified name and its defining class loader. Each such class or interface belongs to a single runtime package. The runtime package of a class or interface is determined by the package name and defining class loader of the class or interface.
http://docs.oracle.com/javase/specs/jvms/se5.0/html/ConstantPool.doc.html#72007
and later in 5.4.4 Access Control:
A class or interface C is accessible to a class or interface D if and only if either of the following conditions are true:
· C is public.
· C and D are members of the same runtime package http://docs.oracle.com/javase/specs/jvms/se5.0/html/ConstantPool.doc.html#72007 (§5.3).
A field or method R is accessible to a class or interface D if and only if any of the following conditions is true:
· R is public.
· R is protected and is declared in a class C, and D is either a subclass of C or C itself.
· R is either protected or package private (that is, neither public nor protected nor private), and is declared by a class in the same runtime package as D.
· R is private and is declared in D.
So the check could be between the loaders of type and accessClass…
In a few hours I will come back.
Best,
--Tumi
De: Jesús Viñuales [mailto:serverperformance@gmail.com] Enviado el: jueves, 17 de julio de 2014 12:41 Para: 'EsotericSoftware/kryo'; 'EsotericSoftware/kryo' Asunto: RE: [kryo] Seeing java.lang.IllegalAccessError when upgrading to 2.24.0 (#217)
Umm, not exactly, in other scenarios class loaders can be hierarchically related (grandson, etc) and the access be legal.
De: Giuseppe Gerla [mailto:notifications@github.com] Enviado el: jueves, 17 de julio de 2014 11:08 Para: EsotericSoftware/kryo CC: Tumi Asunto: Re: [kryo] Seeing java.lang.IllegalAccessError when upgrading to 2.24.0 (#217)
Great!!! You are right!
I check that the same problem can be reproduced also with only one bundle...
Basing on your analisys let me suggest a solution: in ConstructorAccess.get method replace lines from 49 to 74 with
boolean isNonPublic = false; if (!isNonStaticMemberClass) { enclosingClassNameInternal = null; try { Constructor constructor = type.getDeclaredConstructor((Class[])null); isNonPublic = Modifier.isPrivate(constructor.getModifiers()) || (type.getClassLoader() != ConstructorAccess.class.getClassLoader() && Modifier.isProtected(constructor.getModifiers())); } catch (Exception ex) { throw new RuntimeException("Class cannot be created (missing no-arg constructor): " + type.getName(), ex); } if (isNonPublic) { throw new RuntimeException("Class cannot be created (the no-arg constructor is private): " + type.getName()); } } else { enclosingClassNameInternal = enclosingType.getName().replace('.', '/'); try { Constructor constructor = type.getDeclaredConstructor(enclosingType); // Inner classes should have this. isNonPublic = Modifier.isPrivate(constructor.getModifiers()) || (type.getClassLoader() != ConstructorAccess.class.getClassLoader() && Modifier.isProtected(constructor.getModifiers())); } catch (Exception ex) { throw new RuntimeException("Non-static member class cannot be created (missing enclosing class constructor): "
In this way you check private constructor for standard Java application and also protected constructor for that application (like OSGi) in wich classloaders are different between reflectasm and type to instance.
Can be a good idea?
— Reply to this email directly or view it on GitHub https://github.com/EsotericSoftware/kryo/issues/217#issuecomment-49278180 . https://github.com/notifications/beacon/6210680__eyJzY29wZSI6Ik5ld3NpZXM6QmVhY29uIiwiZXhwaXJlcyI6MTcyMTIwNzI5MywiZGF0YSI6eyJpZCI6MzE2MTM0NDZ9fQ==--0fcc02bb0117b657af289c9cdce51cb5923e8392.gif
mmmm.... I don't think that you can compare that loaders because they are always different.
I don't understand well what you do in reflectasm, because you used reflection concepts that I don't know. So it is possible that following sentences are totally wrong. What I understand is that you create a new class using ClassWriter utility. This new class has a constructor created with this call
cw.visitMethod(ACC_PUBLIC, "
I suppose that when from Kryo you call newInstance you pass from this constructor. I tryed to do a simple test just to understand better and change ACC_PUBLIC in ACC_PROTECTED. A kind of magic... now all work fine.
This can help you?
No, in fact with that change ConstructorAccess.get wouldalways throw an Exception (because would try to invoke a protected constructor in the subclass).
Please download my ReflectASM fork, https://github.com/serverperformance/reflectasm, I think this change solves the issue.
Please test not only scenarios with protected constructors, but also ok scenarios. If you confirm, I will make a pull request, and in turn should be canged in Kryo the version dependency
Good. Now it works. I tryed it with a standard java application, osgi bundles (create to reproduce the problem) and my real software. All applications tested create classes with public construcotr and classes with protected constructor.
Nate, I just made a pull request in ReflectASM to solve this issue (https://github.com/EsotericSoftware/reflectasm/pull/29). Please review it and merge+release if it is ok for you (and then Kyo itself should be updated with dependence to the new ReflectASM version).
Regards,
--Tumi
De: Giuseppe Gerla [mailto:notifications@github.com] Enviado el: jueves, 17 de julio de 2014 17:34 Para: EsotericSoftware/kryo CC: Tumi Asunto: Re: [kryo] Seeing java.lang.IllegalAccessError when upgrading to 2.24.0 (#217)
Good. Now it works. I tryed it with a standard java application, osgi bundles (create to reproduce the problem) and my real software. All applications tested create classes with public construcotr and classes with protected constructor.
— Reply to this email directly or view it on GitHub https://github.com/EsotericSoftware/kryo/issues/217#issuecomment-49323289 . https://github.com/notifications/beacon/6210680__eyJzY29wZSI6Ik5ld3NpZXM6QmVhY29uIiwiZXhwaXJlcyI6MTcyMTIzMDQxMSwiZGF0YSI6eyJpZCI6MzE2MTM0NDZ9fQ==--88eac7818cd30726fbeb6a3d41d49414544bc1e6.gif
@serverperformance @ggerla Could you check the latest SNAPSHOT? It should use the fixed version of ReflectASM and should produce no errors. It would be nice if you could confirm it.
@serverperformance @ggerla Please note that the groupId is now just com.esotericsoftware
and the version 2.25.0-SNAPSHOT
.
Hi all I confirm you that kryo 2.25.0-SNAPSHOT solve this issue.
P.S. I see that there are some changes on dependencies. Now to deploy kryo in OSGi environment I need:
Thanks Regards
@ggerla Thanks for confirmation!
Regarding the changes on dependencies. Kryo artifacts are now published in two forms: "kryo" does not shade/inline any dependencies. "kryo-shaded" shade/inline some of the dependencies as before. Pick the one which is more suitable for your environment.
Great!
-----Original Message----- From: Giuseppe Gerla notifications@github.com Date: Tue, 29 Jul 2014 00:30:44 To: EsotericSoftware/kryokryo@noreply.github.com Reply-To: EsotericSoftware/kryo reply@reply.github.com Cc: Tumiserverperformance@gmail.com Subject: Re: [kryo] Seeing java.lang.IllegalAccessError when upgrading to 2.24.0 (#217)
Hi all I confirm you that kryo 2.25.0-SNAPSHOT solve this issue.
P.S. I see that there are some changes on dependencies. Now to deploy kryo in OSGi environment I need:
Thanks Regards
Reply to this email directly or view it on GitHub: https://github.com/EsotericSoftware/kryo/issues/217#issuecomment-50444264
Should we consider this issue as fixed? Can we close it?
@romix I don't understand if you ask me... For me, you can close the issue.
Wen trying to upgrade from 2.23.0 to 2.24.0, I am seeing following issue:
java.lang.IllegalAccessError: tried to access method mypkg.MyClazz$MyInnerClazz.()V from class mypkg.MyClazz$MyInnerClazzConstructorAccess
at mypkg.MyClazz$MyInnerClazzConstructorAccess.newInstance(Unknown Source)
at com.esotericsoftware.kryo.Kryo$DefaultInstantiatorStrategy$1.newInstance(Kryo.java:1193)
at com.esotericsoftware.kryo.Kryo.newInstance(Kryo.java:1061)
at com.esotericsoftware.kryo.serializers.FieldSerializer.createCopy(FieldSerializer.java:620)
at com.esotericsoftware.kryo.serializers.FieldSerializer.copy(FieldSerializer.java:624)
at com.esotericsoftware.kryo.Kryo.copy(Kryo.java:862)
at com.esotericsoftware.kryo.serializers.MapSerializer.copy(MapSerializer.java:157)
at com.esotericsoftware.kryo.serializers.MapSerializer.copy(MapSerializer.java:21)
at com.esotericsoftware.kryo.Kryo.copy(Kryo.java:862)
at com.esotericsoftware.kryo.serializers.UnsafeCacheFields$UnsafeObjectField.copy(UnsafeCacheFields.java:297)
at com.esotericsoftware.kryo.serializers.FieldSerializer.copy(FieldSerializer.java:634)
at com.esotericsoftware.kryo.Kryo.copy(Kryo.java:862)
Anyone know what the problem could be?