RFR: 8365937: post_method_exit might incorrectly set was_popped_by_exception and value in the middle of stack unwinding [v4]
David Holmes
dholmes at openjdk.org
Tue Sep 2 03:57:53 UTC 2025
On Tue, 2 Sep 2025 01:13:40 GMT, Leonid Mesnik <lmesnik at openjdk.org> wrote:
>> Or mabye I do get it. The actual pending exception (on the ThreadShadow) must be saved, cleared and later restored, else we can't execute the Java upcall. But because we have separate paths for normal and exceptional method-exit processing, maybe we don't have to save/restore the JvmtiThreadState exception fields? But in that case are those fields even useful? To be honest I can't see how the two fields interact: there is either no exception being thrown, or there is - so only one field needed.
>
> BTW, I forget to mention that the
> `state->is_exception_detected() && !state->is_exception_caught();`
> is just the same as
> `state->is_exception_detected()`
> Because exception state is enum an if it is `ES_DETECTED` then it can't be `ES_CAUGHT`.
>
> Am I understand correctly, that you propose to clear/restore `_exception_state` for any upcall since it is the separate execution path? In this case it should be done in every place where java is called from vm while exception is in detected state. It makes sense to keep this field consistent.
>
> I looked on the `_exception_state` in the `JvmtiThreadState` and is also used in the `notice_unwind_due_to_exception`. But it is unclear how it can be false in this method.
> Also, it is used in the `post_exception_throw` and it also unclear what is checked there. And seems it is not used in any other places.
>
> So, probably, this state is not useful if method exit/exception posting work without it. Or it is needed to find all places where it is should be saved/restored. It might includes methodExit events but there are other places that should be fixed. It is needed to change this line to assertion and see where it is hit.
> However, I think it could be done separately. It is too complicated for this fix.
I think this state, and its use, needs more examination, but yes that can be deferred to another issue. Thanks
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26886#discussion_r2314852578
More information about the hotspot-dev
mailing list