[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8302516: Do some cleanup of nsk/share/jdi/EventHandler.java
From:       Chris Plummer <cjplummer () openjdk ! org>
Date:       2023-02-27 21:15:02
Message-ID: z0SnRN-OM6fSAncj7u43SMBLuUVCisrI4RB6InaXnOE=.719d0111-dc42-4a58-b795-0e7410d9b3af () github ! com
[Download RAW message or body]

On Wed, 15 Feb 2023 00:13:22 GMT, Chris Plummer <cjplummer@openjdk.org> wrote:

> While working on [JDK-8289765](https://bugs.openjdk.org/browse/JDK-8289765), I \
> ended up doing a lot of cleanup of nsk/share/jdi/EventHandler.java, so much so that \
> the changes distract from the actual bug fix, so I decided it would be best to \
> first push them with a separate CR. Changes include: 
> - The main change is merging waitForRequestedEvent() and \
> waitForRequestedEventSet(). These methods are quite big and almost identical. I had \
> to add some more code to them for \
> [JDK-8289765](https://bugs.openjdk.org/browse/JDK-8289765), and decided it was best \
>                 to merge them first rather than making this code cloning situation \
>                 even worse.
> - Remove EventFilter.filtered() call when dealing with an uncaught exception event. \
>                 These events are never filtered.
> - Add complain() method. We already have a display() method so you can just call \
> display() instead of log.display(), and it also adds the "EventHandler> " prefix to \
> the output, so I though it would be good to do the same for log.complain() uses, \
>                 especially since some of the uses also were adding the same prefix.
> - Added a new EventListener that simply logs the event.
> 
> The remaining changes are just minor local edits whose purpose should be obvious.
> 
> If you need a bit of background on this code, read on. EventHandler has  run() \
> method that continually queries the event queue for more JDI events. When they come \
> in, the registered EventListeners all get various callbacks. First \
> eventSetReceived() is called for each. Then eventReceived() is called for each \
> listener for each Event in the EventSet.  If any eventReceived() returns true, that \
> means the event was handled an no more listeners should be called. Finally, \
> eventSetComplete() is called for each listener. 
> There are a number of default listeners registered that are always in place. See \
> createDefaultListeners(). They aren't really of concern with this PR. The \
> waitForRequestedEvent() and waitForRequestedEventSet() methods (now combined into \
> waitForRequestedEventSetCommon()) also register listeners, with the main one being \
> used to check for the specific event that has been requested. These listeners are \
> pre-pended to the default list. Listeners are always called in reverse of the order \
> added. 
> After setting up the listeners, waitForRequestedEventSetCommon() will block until a \
> matching event has arrived. This is detected by the setting of en.set (and \
> en.event). The listener will set them when the event matches, and this is done \
> async from the thread that is running the run() method (remember, the run() method \
> calls listener.eventSetRecieved()).

Thanks Alex!

Can I get one more review please?

-------------

PR: https://git.openjdk.org/jdk/pull/12568


[prev in list] [next in list] [prev in thread] [next in thread]