RFR: 8365937: post_method_exit might incorrectly set was_popped_by_exception and value in the middle of stack unwinding [v4]
Leonid Mesnik
lmesnik at openjdk.org
Tue Sep 2 01:16:55 UTC 2025
On Mon, 1 Sep 2025 05:19:51 GMT, David Holmes <dholmes at openjdk.org> wrote:
>> Sorry I still can't see this. Method A throws an exception. The callback for the method-exit event for method A then invokes Java method B. Method B also has a callback enabled and so we are processing the method-exit for B. Method B has completed normally, but the exception from method A is still listed as pending? I can't see how that would come about as we must (temporarily) clear the pending exception to do the upcall else the upcall method would immediately throw that pending exception. But if we have cleared it then the upcall method's method-exit event shouldn't be able to see it. Similarly we must save/restore the JvmtiThreadState else we would be processing the normal exit as if it were an exception one.
>
> 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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26886#discussion_r2314718031
More information about the hotspot-dev
mailing list