RFR: 8302516: Do some cleanup of nsk/share/jdi/EventHandler.java

Leonid Mesnik lmesnik at openjdk.org
Wed Mar 1 16:07:17 UTC 2023


On Wed, 15 Feb 2023 00:13:22 GMT, Chris Plummer <cjplummer at 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()).

Seems that the braces style is incorrect in some places.

test/hotspot/jtreg/vmTestbase/nsk/share/jdi/EventHandler.java line 336:

> 334:                     defaultExceptionRequest != null &&
> 335:                     defaultExceptionRequest.equals(event.request()))
> 336:                 {

wrong indentation style

test/hotspot/jtreg/vmTestbase/nsk/share/jdi/EventHandler.java line 372:

> 370: 
> 371:     private class EventNotification {
> 372:         volatile Event event = null;

assign to null is not needed.

test/hotspot/jtreg/vmTestbase/nsk/share/jdi/EventHandler.java line 382:

> 380:                                                           long timeout,
> 381:                                                           boolean shouldRemoveListeners)
> 382:     {

wrong brace style

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

Changes requested by lmesnik (Reviewer).

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


More information about the serviceability-dev mailing list