Substitute BootLoader.hasClassPath() to work around a JDK 11 NPE

[gwenneg]

Fixes #6089

[gwenneg]

This fixes the NullPointerException and allows the integration tests to end successfully.

@stuartwdouglas Does this look like a reasonable fix? I'm not really in my field of expertise here so I might be doing something totally wrong.

[stuartwdouglas]

This looks reasonable but you should file a GraalVM issue so it can be fixed upstream.

[gwenneg]

Sure, I'll do it. Thanks for having a look!

[gwenneg]

GraalVM issue created: oracle/graal#1966

[gwenneg]

@gsmet This might be the solution to all the JDK 11 native integration tests that failed to start recently. It had definitely nothing to do with the time the native image needed to start :)

[jaikiran]

Hello @gwenneg, I just commented on that graal issue you opened. The graal team has fixed it in master branch some days back as part of oracle/graal@68027de.

I think we will need this workaround in Quarkus until 19.3.x is released with that fix. Their commit is different from what is being proposed in this PR - they return true instead of false for hasClassPath. I think we should just copy over their change in that graal commit into Quarkus as a substitution to be consistent with their semantics.

[gwenneg]

I don't think we can copy the entire Target_jdk_internal_loader_BootLoader class as we would have to import the ClassForNameSupport class in Quarkus and it is an internal class which is not exported in the svm-19.3.0.jar module-info. But I agree we should at least return the same true value than the GraalVM substitution from oracle/graal@68027de.

[gwenneg]

Actually, changing the returned value from false to true in our substitution could also cause the other BootLoader methods to be called (the ones that were substituted in oracle/graal@68027de). I'll need to run an entire Quarkus native build to check if everything's OK with such a change.

[jaikiran]

I don't think we can copy the entire Target_jdk_internal_loader_BootLoader class as we would have to import the ClassForNameSupport class in Quarkus and it is an internal class which is not exported in the svm-19.3.0.jar module-info

Hello @gwenneg, sorry, I wasn't clear. I actually meant, we should just copy over those 5 new Substitute methods (which don't require any internal class usage) to the BootLoader that were added as part of that commit in graal.

[gwenneg]

Oh, I misunderstood you first comment (which was clear enough), sorry :) That will work and I'll do it right now.

[gwenneg]

This PR needs to be tested with a full JDK 11 native build.

PLEASE DON'T MERGE before I say it's OK.

[gwenneg]

Thanks for your help with this @jaikiran!

[gwenneg]

I've successfully tested the latest version of this PR with many JDK 11 native integration tests. I think it can be safely merged into master after being reviewed again.

[gwenneg]

I just saw a similar NPE that may be related, I'm putting the PR back to WIP to investigate what's going on.

[gwenneg]

I created #6127 to fix the similar NPE mentioned above. Both share a common root cause but require different and independent substitutions to be fixed. I think merging this PR ASAP is important because it might help us detect new issues during native tests execution, that's why I didn't treat #6127 in the same PR.

@gsmet: We're ready for a final review and merge, for real this time.

[gsmet]

Nice work, @gwenneg .