JDK-8294567: IGV: IllegalStateException in search

[tobiasholenstein]

When searching for a node, IGV first looks in the current open graph. If it can find the node here everything works fine.

Problem

If if cannot find the node if uses the searchForward and searchBackward in the EditorInputGraphProvider to search in the other graphs. It crashes here because editor.isOpened() can only be called from the EventDispatchThread

Solution

The calls to editor.isOpened() are not needed because editor != null already means that it is open. So just remove all 4 calls to editor.isOpened().

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8294567: IGV: IllegalStateException in search

Reviewers

• Roberto Castañeda Lozano (@robcasloz - Reviewer)
• Christian Hagedorn (@chhagedorn - Reviewer)
• Vladimir Kozlov (@vnkozlov - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/10483/head:pull/10483
$ git checkout pull/10483

Update a local copy of the PR:
$ git checkout pull/10483
$ git pull https://git.openjdk.org/jdk pull/10483/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 10483

View PR using the GUI difftool:
$ git pr show -t 10483

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/10483.diff

[bridgekeeper]

👋 Welcome back tholenstein! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@tobiasholenstein The following label will be automatically applied to this pull request:

• hotspot-compiler

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 00: Full (756fe562)

[robcasloz]

Looks good!

[openjdk]

@tobiasholenstein This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8294567: IGV: IllegalStateException in search

Reviewed-by: rcastanedalo, chagedorn, kvn

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 24 new commits pushed to the master branch:

• 48674d4: 8291428: JFR: 'jfr print' displays incorrect timestamps during DST
• 3b1bc21: 8294307: ISO 4217 Amendment 173 Update
• b8f9a91: 8293940: Some tests for virtual threads take too long
• 1d26c4b: 8291022: JFR: Reduce logging in ChunkHeader constructor
• 3efbd5f: 8294626: Improve URL protocol lower casing
• 052a924: 8294293: Remove unused _width and _newlines field in outputStream
• c2ce43c: 8291021: JFR: Only one finished state in ChunkHeader class
• f015733: 8294357: (tz) Update Timezone Data to 2022d
• 64da862: 8287597: List all preview features on the javadoc PREVIEW page
• b4e1e41: 8293000: Review running times of jshell regression tests
• ... and 14 more: https://git.openjdk.org/jdk/compare/1decdcee71eae3344aaf2c0223d40a2b57a044b9...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[chhagedorn]

Looks good!

[vnkozlov]

I have general question (disclaimer: I did not use IGV for long time).
Is it possible to specify input data file when you launch IGV, run IGV in verification mode (load data, process/verify it, do some basic search and other manipulations) and exit?
We can then check result and catch cases like this bug.
Since we support this tool we should do some basic automatic testing for it.

[tobiasholenstein]

I have general question (disclaimer: I did not use IGV for long time).
Is it possible to specify input data file when you launch IGV, run IGV in verification mode (load data, process/verify it, do some basic search and other manipulations) and exit?
We can then check result and catch cases like this bug.
Since we support this tool we should do some basic automatic testing for it.

Hi Vladimir,
We have jUnit tests that are run every time we build IGV. But they are very basic at the moment and have poor coverage. I agree that the automated tests should be improved. Maybe we could start writing more jUnit tests. Ideally, we should also cover some more complex examples with the jUnit tests - like some kind of integration testing. For end-to-end testing (system testing), which is probably the scenario you describe, we do that by hand at the moment. The last time we did that was at #10195 when we moved to Netbeans Platform 15. Automating this would be great - maybe there are frameworks that help with GUI testing, or we implement a "testing mode". For search, I am not sure yet how best to do this, except with jUnit tests for searchForward() and searchBackward(). They both are called in EditorInputGraphProvider.java, which is an @ServiceProvider in the Netbeans platform. NodeQuickSearch.java then uses the InputGraphProvider service. But neither EditorInputGraphProvider.java nor NodeQuickSearch.java is instantiated directly by us. Instead, we tell the Netbeans Platform in view/layer.xml that we want to use it for the search: <file name="com-sun-hotspot-igv-view-NodeQuickSearch.instance"/>.

[vnkozlov]

Thank you for your response.

[tobiasholenstein]

Thanks you @robcasloz , @chhagedorn and @vnkozlov for the reviews!

/integrate

[openjdk]

Going to push as commit 6e8f038.
Since your change was applied there have been 33 commits pushed to the master branch:

• bc668b9: 8293099: JFR: Typo in TestRemoteDump.java
• 03f25a9: 8293562: blocked threads with KeepAliveCache.get
• a69ee85: 8292336: JFR: Warn users if -XX:StartFlightRecording:disk=false is specified with maxage or maxsize
• b8b9b97: 8294676: [JVMCI] InstalledCode.deoptimize(false) should not touch address field
• fd59430: 8294610: java/net/vthread/HttpALot.java is slow on Linux
• c7ab1ca: 8294609: C2: Improve inlining of methods with unloaded signature classes
• 375f02f: 8294608: Remove redundant unchecked suppression in FileDescriptor
• d207da8: 8294533: Documentation mistake in Process::getErrorStream and getInputStream
• da4e96d: 8276545: Fix handling of trap count overflow in Parse::Parse()
• 48674d4: 8291428: JFR: 'jfr print' displays incorrect timestamps during DST
• ... and 23 more: https://git.openjdk.org/jdk/compare/1decdcee71eae3344aaf2c0223d40a2b57a044b9...master

Your commit was automatically rebased without conflicts.

[openjdk]

@tobiasholenstein Pushed as commit 6e8f038.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.