-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
[GR-46526] Preserve LinkageError
s and re-throw them at run-time.
#6753
Conversation
In case `--link-at-build-time` is used, `LinkageError`s are used as additional cause.
@zakkak could you maybe run some Quarkus samples and report how many |
@@ -845,6 +845,10 @@ private void handleClassFileName(URI container, Module module, String className) | |||
clazz = imageClassLoader.forName(className, module); | |||
} catch (AssertionError error) { | |||
VMError.shouldNotReachHere(error); | |||
} catch (LinkageError le) { | |||
synchronized (linkageErrors) { | |||
linkageErrors.put(className, le); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should make sure that the LinkageError
does not reference another Throwable
to prevent leakage?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't that risk losing the root cause?
To my understanding LinkageError
may only reference another Throwable
through the cause
field.
Sure, I will try to do some tests tomorrow, otherwise it will have to wait ~2 weeks. |
I am appending the results from a simple Java helloworld and some "beefy" scenarios from https://github.com/quarkus-qe/beefy-scenarios.
|
Hi @fniephaus , are there any updates on this? |
No, not at this point. We have an idea that no longer requires a static map, but I haven't had enough time to look into this yet. |
Hi @fniephaus, we hit that issue again in quarkusio/quarkus#41256 (comment) Are there any plans on merging a fix for it? |
Hi @zakkak, the PR is unfortunately not in the best state and more changes are needed. I wish I had more time for this but I'm afraid it's currently not a priority. Closing the PR for now |
@fniephaus could you share more info on this so that I could evaluate whether it's something I could contribute? |
Run-time example:
Build-time example: