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

Absolute classpath gets recorded in ClassPathSystemPropBuildStep #39387

Open
rsvoboda opened this issue Mar 13, 2024 · 17 comments
Open

Absolute classpath gets recorded in ClassPathSystemPropBuildStep #39387

rsvoboda opened this issue Mar 13, 2024 · 17 comments
Labels
area/core kind/bug Something isn't working

Comments

@rsvoboda
Copy link
Member

Describe the bug

Absolute classpath gets recorded in ClassPathSystemPropBuildStep, used for
"Make Truffle from GraalVM 23.1 work in all Quarkus modes" fix

This was noticed by @geoand as part of #39350 discussions

https://github.com/quarkusio/quarkus/blob/main/core/deployment/src/main/java/io/quarkus/deployment/steps/ClassPathSystemPropBuildStep.java#L49

When graal-sdk 23.1 is on the classpath, one can see error like this (please check Caused by part):

[ERROR] Errors:
[ERROR]   AlmostAllQuarkusExtensionsTest.testQuarkusEndpointWithManyExtensions » Runtime java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.deployment.steps.MainClassBuildStep#build threw an exception: java.lang.RuntimeException: Failed to record call to method public void io.quarkus.runtime.ClassPathSystemPropertyRecorder.set(java.lang.String)
	at io.quarkus.deployment.recording.BytecodeRecorderImpl.writeBytecode(BytecodeRecorderImpl.java:480)
	at io.quarkus.deployment.steps.MainClassBuildStep.writeRecordedBytecode(MainClassBuildStep.java:501)
	at io.quarkus.deployment.steps.MainClassBuildStep.build(MainClassBuildStep.java:201)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at io.quarkus.deployment.ExtensionLoader$3.execute(ExtensionLoader.java:849)
	at io.quarkus.builder.BuildContext.run(BuildContext.java:256)
	at org.jboss.threads.ContextHandler$1.runWith(ContextHandler.java:18)
	at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2513)
	at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1538)
	at java.base/java.lang.Thread.run(Thread.java:833)
	at org.jboss.threads.JBossThread.run(JBossThread.java:501)
Caused by: java.lang.RuntimeException: String too large to record: /Users/rsvoboda/.m2/repository/io/quarkus/quarkus-bootstrap-core/999-SNAPSHOT/quarkus-bootstrap-core-999-SNAPSHOT.jar:/Users/rsvoboda/...
....

Expected behavior

According to #39350 (comment) absolute paths should not be recorded

Actual behavior

Absolute CP is passed

How to Reproduce?

Alternatively you can use reproducer from #39350 but make sure to use Quarkus main revision before merging the fix, but with graal-sdk version set to 23.1.

Output of uname -a or ver

macOS

Output of java -version

Java 21

Quarkus version or git rev

Quarkus main 4ca065f

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

Copy link

quarkus-bot bot commented Mar 13, 2024

/cc @Sanne (core), @aloubyansky (core), @gsmet (core), @radcortez (core), @stuartwdouglas (core)

@geoand
Copy link
Contributor

geoand commented Mar 14, 2024

@zakkak @jerboaa @rsvoboda which application should I be using to ensure any potential fixes keep the Truffle functionality working?

@jerboaa
Copy link
Contributor

jerboaa commented Mar 14, 2024

I have no idea, sorry.

@zakkak
Copy link
Contributor

zakkak commented Mar 14, 2024

Me neither. The support was added on @chumer's request in #36242

Hopefully Christian can help. I would start by using the reproducer from #36242.

@geoand
Copy link
Contributor

geoand commented Mar 14, 2024

Hopefully Christian can help. I would start by using the reproducer from #36242.

Can this be used as is with Quarkus 3.8? I am asking because of the Truffle dependencies. Nothing should change from our perspective?

@zakkak
Copy link
Contributor

zakkak commented Mar 14, 2024

Can this be used as is with Quarkus 3.8?

I gave it a go (by updating the POM file to use Quarkus 3.8.0 and graal-sdk 23.1.2) and I am getting:

java.lang.ClassNotFoundException: org.graalvm.polyglot.impl.AbstractPolyglotImpl
	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:641)
	at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:526)
	at java.base/java.lang.ClassLoader.defineClass1(Native Method)
	at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1027)
	at java.base/java.security.SecureClassLoader.defineClass(SecureClassLoader.java:150)
	at java.base/jdk.internal.loader.BuiltinClassLoader.defineClass(BuiltinClassLoader.java:862)
	at java.base/jdk.internal.loader.BuiltinClassLoader.findClassOnClassPathOrNull(BuiltinClassLoader.java:760)
	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(BuiltinClassLoader.java:681)
	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:639)
	at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:526)
	at io.quarkus.bootstrap.classloading.QuarkusClassLoader.loadClass(QuarkusClassLoader.java:496)
	at io.quarkus.bootstrap.classloading.QuarkusClassLoader.loadClass(QuarkusClassLoader.java:468)
	at java.base/java.lang.Class.forName0(Native Method)
	at java.base/java.lang.Class.forName(Class.java:534)
	at java.base/java.lang.Class.forName(Class.java:513)
	at java.base/java.util.ServiceLoader$LazyClassPathLookupIterator.nextProviderClass(ServiceLoader.java:1217)
	at java.base/java.util.ServiceLoader$LazyClassPathLookupIterator.hasNextService(ServiceLoader.java:1228)
	at java.base/java.util.ServiceLoader$LazyClassPathLookupIterator.hasNext(ServiceLoader.java:1273)
	at java.base/java.util.ServiceLoader$2.hasNext(ServiceLoader.java:1309)
	at java.base/java.util.ServiceLoader$3.hasNext(ServiceLoader.java:1393)
	at org.graalvm.polyglot.Engine$1.searchServiceLoader(Engine.java:1735)
	at org.graalvm.polyglot.Engine$1.run(Engine.java:1712)
	at org.graalvm.polyglot.Engine$1.run(Engine.java:1707)
	at java.base/java.security.AccessController.doPrivileged(AccessController.java:319)
	at org.graalvm.polyglot.Engine.initEngineImpl(Engine.java:1707)
	at org.graalvm.polyglot.Engine$ImplHolder.<clinit>(Engine.java:190)
	at org.graalvm.polyglot.Engine.getImpl(Engine.java:442)
	at org.graalvm.polyglot.Engine$Builder.build(Engine.java:740)
	at org.graalvm.polyglot.Context$Builder.build(Context.java:1925)
	at org.graalvm.polyglot.Context.create(Context.java:979)
	at org.quarkus.GreetingResource.hello(GreetingResource.java:15)
	at org.quarkus.GreetingResource$quarkusrestinvoker$hello_e747664148511e1e5212d3e0f4b40d45c56ab8a1.invoke(Unknown Source)
	at org.jboss.resteasy.reactive.server.handlers.InvocationHandler.handle(InvocationHandler.java:29)
	at io.quarkus.resteasy.reactive.server.runtime.QuarkusResteasyReactiveRequestContext.invokeHandler(QuarkusResteasyReactiveRequestContext.java:141)
	at org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext.run(AbstractResteasyReactiveContext.java:147)
	at io.quarkus.vertx.core.runtime.VertxCoreRecorder$14.runWith(VertxCoreRecorder.java:582)
	at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2513)
	at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1538)
	at org.jboss.threads.DelegatingRunnable.run(DelegatingRunnable.java:29)
	at org.jboss.threads.ThreadLocalResettingRunnable.run(ThreadLocalResettingRunnable.java:29)
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
	at java.base/java.lang.Thread.run(Thread.java:1583)
Resulted in: java.lang.NoClassDefFoundError: org/graalvm/polyglot/impl/AbstractPolyglotImpl
	... 41 more

So not sure...

@geoand
Copy link
Contributor

geoand commented Mar 14, 2024

Yeah...

@rodion-lezhnyuk-tealium
Copy link

rodion-lezhnyuk-tealium commented May 7, 2024

Hello @geoand, could you please provide some updates, when can we expect fixes for that and may be there is workaround this String too large to record issue without awaiting of your fix? We need org.graalvm.polyglot dependency in our project and we wanted to upgrade to LTS 3.8.3.

Thanks!

@geoand
Copy link
Contributor

geoand commented May 8, 2024

@rodion-lezhnyuk-tealium do you have a sample application I can try?

There is a super simple fix that I can make to get over the basic problem, but as I know there are other problems after that one is solved, I would like to be able to test things out on a sample.

@rodion-lezhnyuk-tealium
Copy link

Hi @geoand, thanks for the response. Sorry, I can't share the application as it's a private company repository. Could you please provide commit example on another repository so I can do same things but in my project?

@geoand
Copy link
Contributor

geoand commented May 8, 2024

Hi,

I don't need the actual application, just something that contains the proper dependencies and some sample code that causes the problem.

@rodion-lezhnyuk-tealium
Copy link

Hi @geoand, here is a repository with sample code which reproduces a bug. I put some descrption to readme.md, please check this out. Feel free to contact me in case of any questions :)

@geoand
Copy link
Contributor

geoand commented May 9, 2024

Thanks, I'll have a look tomorrow

geoand added a commit to geoand/quarkus that referenced this issue May 10, 2024
This is better than the current state, but it is not
yet the absolutely correct

Relates: quarkusio#39387
@geoand
Copy link
Contributor

geoand commented May 10, 2024

#40549 is the fix to the immediate issue but I still need a reproducer that uses Quarkus and Truffle in order to come up with a final / complete fix

geoand added a commit that referenced this issue May 10, 2024
Overcome 'String too large to record' issue with Truffle
gsmet pushed a commit to gsmet/quarkus that referenced this issue May 10, 2024
This is better than the current state, but it is not
yet the absolutely correct

Relates: quarkusio#39387
(cherry picked from commit 56bbb39)
@rodion-lezhnyuk-tealium

Hi @geoand, thanks a lot for the fix, appreciate it a lot. Got few questions regarding the fix:

  1. Are you planning to merge this fix into the 3.8.3 LTS version of the library, or it will be merged only to the latest version of quarkus?
  2. Could you provide an approximate estimate of when that fix will be available in one of artifacts?
  3. Is it possible to not await for artifact release and apply this fix in our project via some buildstep replacement mechanism (I'm not sure that quarkus supports that) or some other way?

@geoand
Copy link
Contributor

geoand commented May 13, 2024

Hi,

Are you planning to merge this fix into the 3.8.3 LTS version of the library, or it will be merged only to the latest version of quarkus?

Yes, that's what the triage backport-3.8 label is meant to signify.

Could you provide an approximate estimate of when that fix will be available in one of artifacts?

Unfortunately I cannot.

Is it possible to not await for artifact release and apply this fix in our project via some buildstep replacement mechanism (I'm not sure that quarkus supports that) or some other way?

If you have truffle on the classpath, there is no way around it

gsmet pushed a commit to gsmet/quarkus that referenced this issue May 22, 2024
This is better than the current state, but it is not
yet the absolutely correct

Relates: quarkusio#39387
(cherry picked from commit 56bbb39)
gsmet pushed a commit to gsmet/quarkus that referenced this issue May 22, 2024
This is better than the current state, but it is not
yet the absolutely correct

Relates: quarkusio#39387
(cherry picked from commit 56bbb39)
gsmet pushed a commit to gsmet/quarkus that referenced this issue May 23, 2024
This is better than the current state, but it is not
yet the absolutely correct

Relates: quarkusio#39387
(cherry picked from commit 56bbb39)
gsmet pushed a commit to gsmet/quarkus that referenced this issue Jun 4, 2024
This is better than the current state, but it is not
yet the absolutely correct

Relates: quarkusio#39387
(cherry picked from commit 56bbb39)
gsmet pushed a commit to gsmet/quarkus that referenced this issue Jun 4, 2024
This is better than the current state, but it is not
yet the absolutely correct

Relates: quarkusio#39387
(cherry picked from commit 56bbb39)
@rodion-lezhnyuk-tealium
Copy link

rodion-lezhnyuk-tealium commented Jun 19, 2024

@geoand thanks for the response, in case it would be helpful for someone, here is an example PR which fixes this issue on project side (not the framework side)
rodion-lezhnyuk-tealium/quarkus-absolute-classpath-sample#1

it simply replaces the classes with the fix that you provided above

holly-cummins pushed a commit to holly-cummins/quarkus that referenced this issue Jul 31, 2024
This is better than the current state, but it is not
yet the absolutely correct

Relates: quarkusio#39387
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/core kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants