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

Java 21-ea NPE issue #5689

Closed
beikov opened this issue Jun 9, 2023 · 17 comments · Fixed by #5690
Closed

Java 21-ea NPE issue #5689

beikov opened this issue Jun 9, 2023 · 17 comments · Fixed by #5690
Assignees

Comments

@beikov
Copy link

beikov commented Jun 9, 2023

It seems that the byte code parsing is affected by

I'm seeing the following error when running the Eclipse Transformer on Java 21:

java.lang.NullPointerException
	at java.base/java.util.Objects.requireNonNull(Objects.java:233)
	at aQute.bnd.classfile.ConstantPool.utf8Info(ConstantPool.java:710)
	at aQute.bnd.classfile.MethodParametersAttribute$MethodParameter.write(MethodParametersAttribute.java:78)
	at aQute.bnd.classfile.MethodParametersAttribute.write(MethodParametersAttribute.java:44)
	at aQute.bnd.classfile.Attribute.writeAttributes(Attribute.java:136)
	at aQute.bnd.classfile.MemberInfo.write(MemberInfo.java:55)
	at aQute.bnd.classfile.ClassFile.write(ClassFile.java:134)
	at org.eclipse.transformer.action.impl.ClassActionImpl.apply(ClassActionImpl.java:444)
	at org.eclipse.transformer.action.impl.ActionImpl.basicApply(ActionImpl.java:537)
	at org.eclipse.transformer.action.impl.ActionImpl.apply(ActionImpl.java:505)
	at org.eclipse.transformer.action.impl.ContainerActionImpl.apply(ContainerActionImpl.java:274)
	at org.eclipse.transformer.action.impl.ContainerActionImpl.apply(ContainerActionImpl.java:159)
	at org.eclipse.transformer.action.impl.ActionImpl.apply(ActionImpl.java:622)
	at org.eclipse.transformer.Transformer$TransformOptions.transform(Transformer.java:1781)
	at org.eclipse.transformer.Transformer.run(Transformer.java:1874)
	at org.eclipse.transformer.jakarta.JakartaTransformer.main(JakartaTransformer.java:30)
@beikov
Copy link
Author

beikov commented Jun 9, 2023

Seems 6.4.1 is not affected by this anymore. Sorry for the noise. Should have updated first.

@beikov beikov closed this as completed Jun 9, 2023
@beikov
Copy link
Author

beikov commented Jun 9, 2023

Seems like I spoke too fast. I was using an older JDK 21 locally which didn't have that fix yet, so the byte code reader is still running into that problem.

@beikov beikov reopened this Jun 9, 2023
@bjhargrave
Copy link
Member

Can you please provide a small github repo which demonstrates the problem? Then we can investigate. Thanks.

@beikov
Copy link
Author

beikov commented Jun 9, 2023

I don't know anything about your test infrastructure, but AFAIU the problem, you only need to compile a class like this:

public class Outer {
  public class Inner { }
}

and then try to read the class files. The problem is that javac for JDK 21-ea produces a MethodParametersAttribute for the implicit constructor in the inner class, which has no name. Prior JDKs did not produce that and you seem to expect that the parameter name is always given.

@bjhargrave bjhargrave self-assigned this Jun 9, 2023
bjhargrave added a commit to bjhargrave/bnd that referenced this issue Jun 9, 2023
A method parameter may not have a name.

Fixes bndtools#5689

Signed-off-by: BJ Hargrave <bj@hargrave.dev>
@beikov
Copy link
Author

beikov commented Jun 9, 2023

I know you just recently released 6.4.1, but it would be awesome if you could do another release soon @pkriens so I don't have to keep my JDK 21-ea enablement PR pending too long. Thanks!

@beikov
Copy link
Author

beikov commented Jun 15, 2023

Ping @pkriens @bjhargrave

@pkriens
Copy link
Member

pkriens commented Jun 15, 2023

Can't you go to the 7.0.0-SNAPSHOT?

We plan to release a 7.0.0 release soon but it will take a few weeks.

@beikov
Copy link
Author

beikov commented Jun 15, 2023

I don't know what changed in 7 or if it is backwards compatible with 6, but I'd prefer if you could release a 6.4.2.

@pkriens
Copy link
Member

pkriens commented Jun 15, 2023

Releasing is a lot of work. 6.4.1 was only released because of my customers in embedded. I regret not calling it differently. There is not going to be another release in this classic line unless there is a very severe reason, which I very much doubt will happen.

The current development branch is 7.0.0 and its planned to be release this summer. Although it is a breaking release this is mostly due to going to Java 17. It should be fully compatible for your purpose.

@beikov
Copy link
Author

beikov commented Jun 15, 2023

The fact that it requires Java 17 is already a road blocker for projects that still want to allow compiling with JDK 8 or 11.
I understand that releasing requires effort, but maybe you can reduce the effort by not publishing blog posts or anything for this one?

@bjhargrave
Copy link
Member

Have you confirmed that the fix in 7.0.0-SNAPSHOT does indeed fix your problem?

And projects can easily change to build with JDK 17 while targeting older JDK versions. Since this issue is around building to target JDK 21, you are going to need to use JDK 21 to even build class files for JDK 21. So it seems that you should not have a issue building with JDK 21.

@beikov
Copy link
Author

beikov commented Jun 15, 2023

Have you confirmed that the fix in 7.0.0-SNAPSHOT does indeed fix your problem?

I did not check if 7.0.0-SNAPSHOT fixes the issue yet, but I fear the version might have breaking changes that affect the Eclipse Transformer.

And projects can easily change to build with JDK 17 while targeting older JDK versions. Since this issue is around building to target JDK 21, you are going to need to use JDK 21 to even build class files for JDK 21. So it seems that you should not have a issue building with JDK 21.

I run the build with JDK 21 to ensure that it actually works for people that use that JDK, but updating the version of the bnd will inherently require that the build always requires at least JDK 17. I can of course parameterize the version and I will do that if you really don't want to do another release. Just wanted to try to convince you since requiring a user to update to a new major that doesn't even have a stable release yet for a bug fix is a bit much for my taste.

@bjhargrave
Copy link
Member

but I fear the version might have breaking changes that affect the Eclipse Transformer.

https://github.com/eclipse/transformer/actions/runs/5244172556/jobs/9469805018 Transformer regularly builds and tests using 7.0.0-SNAPSHOT.

but updating the version of the bnd will inherently require that the build always requires at least JDK 17

If you are talking about updating the version of Bnd used by transformer for JDK 21 support, then updating to 7.0.0 would require all transformer users to use JDK 17. But any such update would be after Bnd 7.0.0 was properly released.

@bjhargrave
Copy link
Member

I did not check if 7.0.0-SNAPSHOT fixes the issue yet

But if you want to press the case for a 6.4.2 release, then you must confirm the fix I made does indeed fix the problem. Otherwise a 6.4.2 release would be of no value.

@beikov
Copy link
Author

beikov commented Jun 15, 2023

I verified that 7.0.0-SNAPSHOT works

@beikov
Copy link
Author

beikov commented Sep 23, 2023

It's 3 months now that I reported this problem, yet there is no 6.4.2 or 7.0.0 release yet. Java 21 was released a few days ago and the lack of a release is blocking my testing to compile with Java 21. I would really appreciate if you could do a release soon so that I can upgrade. I'm sure that there will be more users running into this issue soon now that Java 21 is GA and more people start testing that version.

@pkriens
Copy link
Member

pkriens commented Sep 26, 2023

There will be a release at the end of this week.

I published a RC2 last friday

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 a pull request may close this issue.

3 participants