Throw correct exception when initialization already failed - #14352
Conversation
EricYangIBM
changed the title
| if (err instanceof Error) { | ||
| throw (Error)err; | ||
| } | ||
| throw new ExceptionInInitializerError(err); |
There was a problem hiding this comment.
@babsingh Perhaps this is still necessary if recordInitializationFailure skips the if (initialized) block
There was a problem hiding this comment.
yes, it's possible. let's keep it.
There was a problem hiding this comment.
Feedback and comments in #14291 still apply to this PR. The old PR (#14291) was created using a branch name which included *Error. This caused test failures. So, a new PR with a different branch name was created.
These changes LGTM since the JVM spec (Section 5.5 Initialization) is correctly satisfied:
If the class of E is not Error or one of its subclasses, then create a new instance of the class ExceptionInInitializerError with E as the argument, and use this object in place of E in the following step.
Approving. @EricYangIBM These changes only modify the error handling. So, the segfaults in your personal builds are not possibly due to these changes. You can squash the three commits.
@tajila Request for review, PR builds and merge.
|
Ran sanity.openjdk on temporary branch (that this branch now matches) and the previously numerous segfaults are now (mostly) gone: https://hyc-runtimes-jenkins.swg-devops.com/job/Test_openjdk18_j9_sanity.openjdk_x86-64_linux_Personal/19/ |
|
I believe the sanity.openjdk failures are related to AOT code and intermittent. Sometimes they happen and sometimes not, and they can affect a different number of tests in a run. Agree they aren't related to this change. |
|
jenkins compile alinux64 jdk18 |
|
Actually the commits are not squashed. Once that is done I would merge. |
|
We'd better run sanity and extended just in case there is some functionality test that expects the old behaviour. |
When initialization has already failed for a class before, throw `NoClassDefFoundError` with `ExceptionInInitializerError` as its cause instead of the `Throwable` that caused the first initialization failure. Fixes: eclipse-openj9#14080 Signed-off-by: Eric Yang <eric.yang@ibm.com>
055b4ef to
6cd666f
Compare
|
Commits are squashed. I have only run and verified sanity.functional and sanity.openjdk for xlinux jdk11, 17, and 18. Perhaps I could also run some internal builds. |
|
zOS and Windows internal builds passed: https://hyc-runtimes-jenkins.swg-devops.com/job/jvm.29.personal/30428/ |
|
jenkins test sanity,extended alinux64 jdk18 |
|
@EricYangIBM pls create a PR to un-exclude the test. |
Original issue: eclipse-openj9/openj9#14080 Fixed by: eclipse-openj9/openj9#14352 Signed-off-by: Eric Yang <eric.yang@ibm.com>
Original issue: eclipse-openj9/openj9#14080 Fixed by: eclipse-openj9/openj9#14352 Signed-off-by: Eric Yang <eric.yang@ibm.com>
3 participants
Throw correct exception when initialization already failed
When initialization has already failed for a class before, throw
NoClassDefFoundErrorwithExceptionInInitializerErroras its causeinstead of the
Throwablethat caused the first initialization failure.Supersedes #14291 (renamed branch)
Fixes: #14080
Signed-off-by: Eric Yang eric.yang@ibm.com