Skip to content

Conversation

stuartwdouglas
Copy link
Collaborator

Fixes #5326

Thanks @jaikiran for your excellent analysis and test case

jaikiran and others added 2 commits November 11, 2019 20:36
if (Kind.CLASS.equals(type.kind())) {
return creator.loadClass(type.asClassType().name().toString());
String className = type.asClassType().name().toString();
return doLoadClass(creator, className);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, we use BytecodeCreator.loadClass() in many places. Why do we actually get an IAE when trying to use Hello.class in the MorningGreeting bean? I mean it works with javac so why is it a problem for gizmo?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The interface is not actually accessible from the bean, so LDC does not work. It's a bit of a corner case but it's just how the JVM works

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I see. I've verified this again and it's indeed not possible ;-). So the only thing I'm not sure about is that we do add the TCCL and Class.forName() for every bean type. Which means that the constructor will grow for a bean with large number of bean types.

Also I wonder if should verify all those places where we use BytecodeCreator.loadClass()...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The recorders already use the TCCL because this problem popped up a while ago. In general though this code should not be slower (even thought it is a few more bytecodes), and the number of types should never be large enough that the method size limit becomes a problem.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should never be large enough that the method size limit becomes a problem.

We do a lot more stuff in the constructor but you're right that we should not hit the limit for common use cases.

I'm going to approve this PR but create a follow-up issue to verify all other occurences of BytecodeCreator.loadClass() in arc.

@mkouba mkouba added this to the 1.1.0 milestone Nov 12, 2019
@mkouba mkouba merged commit eccdeb4 into quarkusio:master Nov 12, 2019
@gsmet gsmet removed the backport? label Nov 14, 2019
@gsmet gsmet modified the milestones: 1.1.0, 1.0.0.Final Nov 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

IllegalAccessError trying to run UberJar when public class C extends public class B extends package class A

4 participants