RFR: 8345911: Enhance error message when IncompatibleClassChangeError is thrown for sealed class loading failures [v4]

David Holmes dholmes at openjdk.org
Fri Dec 13 04:22:20 UTC 2024


> When a sealed superclass and its permitted subclass get loaded by different classloaders they end up in different modules (the unnamed module of each loader) and cause an `IncompatibleClassChangeError` to be thrown when loading the subclass. The existing error message is not very useful in recognizing this situation:
> 
> java.lang.IncompatibleClassChangeError: class SealedSubClass cannot inherit from sealed class SealedClass
> 
> as inspection of the class sources will show the expected `permits` and `extends` clauses.
> 
> This change augments the `InstanceKlass::has_as_permitted_subclass` method to provide a means to return a meaningful error message to the `ClassFileParser` callers for use in any ICCE that is to be thrown. That same message is shared between unified logging within the method, and the throwing of the ICCE by the caller. We introduce a new helper to throw the ICCE in this case.
> 
> The changes also adds a little helper method to `ModuleEntry` to make it easier to gets its name.
> 
> The existing tests are updated to reflect the updated error messages, and a new test for the missing "different module" situation is added.
> 
> Note: as with the existing logging code the message always refers to class/subclass when in some cases it is really interface and/or subinterface. This can be changed to be more accurate but greatly complicates the formulation of the messages. It is not uncommon to use "(sub)class" when we mean "(sub)class or (sub)interface".
> 
> The presented implementation exposes the error message by having the caller pass in a `stringStream` which is then used to store the message. This has the downside of always requiring a `stringStream` in the caller, even if there is no problem to report. Alternatively, we could create the `stringStream` in the callee and then "return" its `as_string()` result to the caller via an out parameter (e.g. `char** error_msg`). However because of the lifetime of the `stringStream` we would need to `os::strdup` the `as_string()` value before returning it, and then free it in the caller. A third alternative would be to push the throwing of the ICCE down into the `InstanceKlass` method so there is no need to pass the message around - though that blurs the line of "division of responsibility" between checking the condition and deciding it should result in ICCE being thrown (the normal pattern is to have the `ClassFileParser` code make that decision, while `InstanceKlass` just provides the quer
 ies - but we could do it for conveni...

David Holmes has updated the pull request incrementally with one additional commit since the last revision:

  Update messages per feedback from Alan

-------------

Changes:
  - all: https://git.openjdk.org/jdk/pull/22703/files
  - new: https://git.openjdk.org/jdk/pull/22703/files/87c21f5e..9f3c618d

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=22703&range=03
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=22703&range=02-03

  Stats: 27 lines in 6 files changed: 0 ins; 0 del; 27 mod
  Patch: https://git.openjdk.org/jdk/pull/22703.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/22703/head:pull/22703

PR: https://git.openjdk.org/jdk/pull/22703


More information about the hotspot-dev mailing list