On Mon, 26 Jan 2026 12:01:03 GMT, Joel Sikström <[email protected]> wrote:
>> src/hotspot/share/classfile/classFileParser.cpp line 6298:
>>
>>> 6296: if (klass != nullptr) {
>>> 6297: if (klass->is_inline_klass()) {
>>> 6298: set_inline_layout_info_klass(fieldinfo.index(), klass,
>>> CHECK);
>>
>> I see that this can throw a Metaspace OOME (hence the CHECK) and that we
>> later have this comment:
>>
>>
>> // Loads triggered by the LoadableDescriptors attribute are
>> speculative, failures must not impact loading of current class
>> if (HAS_PENDING_EXCEPTION) {
>> CLEAR_PENDING_EXCEPTION;
>> }
>>
>> Can you verify that using CHECK here is really appropriate? The
>> `resolve_super_or_fail` uses THREAD, and I think that's what the comment
>> refer to. But with the current patch there's a "returning" call in-between
>> those two parts of the function.
>
> No, I didn't consider any pending exceptions when I added the CHECK. I see
> that we have an `assert(false, ...)` down in the Metaspace allocation if
> there's a pending exception, so we really shouldn't try to allocate the array
> of InlineLayoutInfo if there's a pending exception, which makes sense.
>
> Looking at the code, I think we should re-arrange it so that we clear any
> pending exception before calling `set_inline_layout_info_klass`, while
> keeping the `CHECK` so that we exit from
> `ClassFileParser::post_process_parsed_stream` if we fail to allocate in
> Metaspace. This seems to me to be consistent with the old behavior, but it
> would be nice for someone to fill in here.
After looking at this for a bit longer I argue that we should never get a
resolved klass (non-nullptr) with a pending exception, so the code can be
simplified a bit further.
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1966#discussion_r2731177921
