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
8048190: NoClassDefFoundError omits original ExceptionInInitializerError #4996
Conversation
|
Webrevs
|
Hi Coleen,
This was not quite what I had envisaged but initially I thought this looked really neat. However on further thought I think there are a number of potential problems with trying to re-create the original type of exception. The "exception" type may not exist or is not known to the current loader, or the "exception" type does not follow convention and have a string-taking constructor. But also for user-defined exception types you have no idea what execution context might be assumed by the exception constructor logic, or what that logic may do. It may be completely wrong/bad to try and create an exception of that type from the current thread in the current context.
Further comments/suggestions below.
Thanks,
David
test/hotspot/jtreg/runtime/ClassInitErrors/InitExceptionTest.java
Outdated
Show resolved
Hide resolved
test/hotspot/jtreg/runtime/ClassInitErrors/InitExceptionTest.java
Outdated
Show resolved
Hide resolved
test/hotspot/jtreg/runtime/ClassInitErrors/InitExceptionTest.java
Outdated
Show resolved
Hide resolved
test/hotspot/jtreg/runtime/ClassInitErrors/InitExceptionTest.java
Outdated
Show resolved
Hide resolved
…ve an oop. (Could potentially move to Doug's secret Class location. If the exception is not bootstrap exception, throw EIIE instead with message of original exception.
Hi Coleen,
Still thinking about the details of this. One concern below, and a few minor comments.
Thanks,
David
test/hotspot/jtreg/runtime/ClassInitErrors/InitExceptionTest.java
Outdated
Show resolved
Hide resolved
test/hotspot/jtreg/runtime/ClassInitErrors/InitExceptionUnloadTest.java
Outdated
Show resolved
Hide resolved
test/hotspot/jtreg/runtime/ClassInitErrors/InitExceptionUnloadTest.java
Outdated
Show resolved
Hide resolved
@dholmes-ora Thanks for looking at this in detail. It does a lot during initialization error so useful to be cautious. I was thinking of maybe having a diagnostic option to turn it off in case some application down the line finds it's doing too much or one that has a lot of initialization errors that blow up the table. |
…Much safer. Also remove the redundant test and not try to get OOM because that would make the test unreliable.
Hi Coleen,
Functional changes look good!
A couple of nits with the test.
Thanks,
David
test/hotspot/jtreg/runtime/ClassInitErrors/InitExceptionUnloadTest.java
Outdated
Show resolved
Hide resolved
test/hotspot/jtreg/runtime/ClassInitErrors/InitExceptionUnloadTest.java
Outdated
Show resolved
Hide resolved
test/hotspot/jtreg/runtime/ClassInitErrors/InitExceptionUnloadTest.java
Outdated
Show resolved
Hide resolved
@coleenp This change now passes all automated pre-integration checks. After integration, the commit message for the final commit will be:
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 31 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.
|
Thanks for the review and additional suggestions.
test/hotspot/jtreg/runtime/ClassInitErrors/InitExceptionUnloadTest.java
Outdated
Show resolved
Hide resolved
test/hotspot/jtreg/runtime/ClassInitErrors/InitExceptionUnloadTest.java
Outdated
Show resolved
Hide resolved
Thank you David and Ioi for your reviews and comments! |
Going to push as commit 464e874.
Your commit was automatically rebased without conflicts. |
This is a change to save the class initialization error stack trace in a hashtable and return it as the cause when NoClassDefFoundError is thrown. The first commit is a more limited version of this that just changes the message, by adding to the message string. The second commit is getting and saving the stack trace for the original exception, and using the thread in the message. See CR for more details about how the message looks.
Tested with tier1-3 tests on 3 platforms. Tier 4-6 in progress (all but two done and passed).
Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/4996/head:pull/4996
$ git checkout pull/4996
Update a local copy of the PR:
$ git checkout pull/4996
$ git pull https://git.openjdk.java.net/jdk pull/4996/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 4996
View PR using the GUI difftool:
$ git pr show -t 4996
Using diff file
Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/4996.diff