On Mon, 1 Sep 2025 05:19:51 GMT, David Holmes <dhol...@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

Reply via email to