On Tue, 17 Oct 2023 20:41:10 GMT, Johannes Bechberger <jbechber...@openjdk.org> wrote:
>> It seems that ex.request()) is null for this event. The spec for >> Event.request() says null might happen for VMDeath events, but doesn't >> mention any other. In any case, I guess JDI does not filter out any events >> for which there is not a request, which is why the test is seeing it. This >> also means the event could be considered unexpected if the debugger did not >> explicitly add an ExceptionEventRequest, and it could in fact cause an NPE >> if the debugger decides to look at ex.request(). >> >> On the other hand, I think if the debug agent requires that a handler be in >> place (as a result of the debugger creating and enabling the EventRequest) >> in order for onthrow to send the event, then the handler creation might come >> too late, and the event will get filtered out. > > Yes, so the current way is probably ok? > > IDEs like IntelliJ properly handle the exception event that is sent and don't > report an error, so I don't see a problem. > On the other hand, I think if the debug agent requires that a handler be in > place (as a result of the debugger creating and enabling the EventRequest) in > order for onthrow to send the event, then the handler creation might come too > late, and the event will get filtered out. I was thinking that maybe suspend=y is the way around this for the debugger. This will cause the debug agent to do a SUSPEND_ALL before sending the ExceptionEvent. However, the event could still be received by the debugger before the debugger has setup the ExceptionRequest. The only thing suspend=y is really doing here is ensuring that all threads get suspended when the event is generated. In fact that makes it even more important that the debugger process the event. Otherwise it won't be doing eventSet.resume(), and execution will come to a stand still. I think we might just be stuck with what we have now. My main concern then is that it is neither spec'd nor documented well. I'll file a separate CR to cover that. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16145#discussion_r1362819327
