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

Fix faulty class loader #43718

Merged
merged 1 commit into from
Oct 5, 2024
Merged

Conversation

dmlloyd
Copy link
Member

@dmlloyd dmlloyd commented Oct 5, 2024

The class loader as it is written will always delegate to the parent class loader first looking for the bytes of the class. This is very bad if the parent class loader happens to have the class, as is the case when a JBang library overlaps with a Quarkus one. This results in the RuntimeLauncherClassLoader defining the class as if it found it in its own set of resources, however the class bytes actually came from the JBang class loader (all of the class loaders involved are parent-first, so it goes right up to the top).

A class loader should never search for class resources outside of its own search path (particularly in parent class loaders). So, the fix is to look for the ide-launcher-res resource exclusively. If it's not found there, we can safely fail because the class loader is parent-first, so there'd be nothing left to search anyway.

Should fix #43648, which should be rebased after this. Fixes #43681.

@gastaldi
Copy link
Contributor

gastaldi commented Oct 5, 2024

Nice catch @dmlloyd !

Copy link

quarkus-bot bot commented Oct 5, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit aa6bf6c.

Failing Jobs

Status Name Step Failures Logs Raw logs Build scan
✔️ JVM Tests - JDK 17 Logs Raw logs 🔍
JVM Tests - JDK 21 Build Failures Logs Raw logs 🔍

Full information is available in the Build summary check run.
You can consult the Develocity build scans.

Failures

⚙️ JVM Tests - JDK 21 #

- Failing: extensions/reactive-mysql-client/deployment 
! Skipped: integration-tests/hibernate-reactive-mariadb integration-tests/hibernate-reactive-mysql integration-tests/hibernate-reactive-mysql-agroal-flyway and 1 more

📦 extensions/reactive-mysql-client/deployment

io.quarkus.reactive.mysql.client.MySQLPoolProducerTest. - History - More details - Source on GitHub

java.lang.RuntimeException: 
java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.datasource.deployment.devservices.DevServicesDatasourceProcessor#launchDatabases threw an exception: java.lang.RuntimeException: org.testcontainers.containers.ContainerLaunchException: Container startup failed for image docker.io/mysql:8.4
	at io.quarkus.datasource.deployment.devservices.DevServicesDatasourceProcessor.startDevDb(DevServicesDatasourceProcessor.java:367)
	at io.quarkus.datasource.deployment.devservices.DevServicesDatasourceProcessor.launchDatabases(DevServicesDatasourceProcessor.java:121)
	at java.base/java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:733)
	at io.quarkus.deployment.ExtensionLoader$3.execute(ExtensionLoader.java:856)
	at io.quarkus.builder.BuildContext.run(BuildContext.java:256)

@maxandersen
Copy link
Member

very nice find - any chance this could explain some of the classloader leaks we've been battling with ?

@gsmet
Copy link
Member

gsmet commented Oct 5, 2024

any chance this could explain some of the classloader leaks we've been battling with ?

I don't think so. IIRC, this class loader is only used for IDE launching and JBang.
I'm not exactly sure why we use it for running a JBang script outside of the IDE though.
I would have expected this integration to use the standard class loading.

@gastaldi gastaldi merged commit af53a4d into quarkusio:main Oct 5, 2024
52 of 53 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.16 - main milestone Oct 5, 2024
@dmlloyd dmlloyd deleted the fix-bad-classloader branch October 5, 2024 13:15
@gsmet gsmet modified the milestones: 3.16.0.CR1, 3.15.2 Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JBangIntegration needs some TLC ASAP
4 participants