RFR: 8263322: Deadlock when calling Application.launch in the FXThread after Platform.startup [v4]

[Kevin Rushforth]

On Wed, 10 Mar 2021 22:35:31 GMT, Florian Kirmaier <fkirmaier at openjdk.org> wrote:

>> Fixing deadlock when calling Application.launch in the FXThread after Platform.startup
>
> Florian Kirmaier has updated the pull request incrementally with one additional commit since the last revision:
> 
>   JDK-8263322
>   updated the javadoc to mention the new case.

The fix looks good. I have a couple comments on the tests, mainly about making them more robust by having only a single `@Test` method in each file.

tests/system/src/test/java/test/com/sun/javafx/application/InitializeJavaFXStartupTest.java line 46:

> 44: 
> 45:     @Test (timeout = 15000)
> 46:     public void testStartupThenLaunch() throws Exception {

It will be more robust for these two test methods to be in separate files. Otherwise they might interfere with each other. If `testStartupThenLaunchInFX` runs first, then an error in that test (e.g., if the fix isn't applied and it times out) will cause the `testStartupThenLaunch` to fail. Conversely, if `testStartupThenLaunch` runs first, then the system is in a state where the Application has already been started by the time `testStartupThenLaunchInFX` runs. It will still pass, but won't be a solid test of the fix, since that case would fail today.

tests/system/src/test/java/test/com/sun/javafx/application/InitializeJavaFXLaunchTest.java line 46:

> 44: 
> 45:     @Test (timeout = 15000)
> 46:     public void testStartupThenLaunch() throws Exception {

It will be more robust for these two test methods to be in separate files. Otherwise they might interfere with each other.

tests/system/src/test/java/test/com/sun/javafx/application/InitializeJavaFXBase.java line 80:

> 78:                 System.out.println("Finished launch!");
> 79:                 Assert.fail("Error: No Exception was thrown - expected IllegalStateException");
> 80:             } catch (IllegalStateException e) {

Maybe add a comment that this exception is expected?

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

PR: https://git.openjdk.java.net/jfx/pull/421