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

[Florian Kirmaier]

On Wed, 10 Mar 2021 14:17:57 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> The idea behind the fix is good. A few changes are needed:
>> 
>> 1. The check should be split into two separate `if` statements, each with their own error message (see inline).
>> 
>> 2. This should be documented in the API docs for the Application::launch method as an additional case that will throw an IllegalStateException. It will need a reasonably trivial CSR since we are specifying a new case that will cause an exception to be thrown.
>> 
>> 3. The system test need to be broken up into separate files, one for each `@Test` method, since application launching tests need to run in their own JVM. If you want to share any code, you could refactor it into a common class that has methods (not annotated with `@Test`) to perform the testing, and separate classes that will subclass the common class, each with a single `@Test` method that simply calls into the method in the common class to do the testing. See any number of examples in `tests/system/src/test/java/test/com/sun/javafx/application/` (which is also a better location for your new test). You will want to add a cleanup method annotated with `@AfterClass` that calls `Platform.exit()`. I think three tests would be good:
>>     * Initalize the FX runtime via Platform.startup and then launch an Application on another thread (should succeed)
>>     * Initalize the FX runtime via Platform.startup and then launch an Application ~~on another thread~~ the FX Application Thread (should throw ISE)
>>     * Initalize the FX runtime via Application.launch and then launch a second Application on another thread (should throw ISE).
>> 
>> 
>> I note that when I run your test, it hangs (on Windows...I didn't try it on other platforms).
>> 
>> gradle --info -PFULL_TEST=true cleanTest :systemTests:test --test InitializeJavaFXTest
>> 
>> This might be resolved by ensuring each test is in its own JVM as requested above.
>> 
>> 
>> Additional comments are inline.
>
>> * Initalize the FX runtime via Platform.startup and then launch an Application on another thread (should throw ISE)
> 
> That should be:
> 
> * Initalize the FX runtime via Platform.startup and then launch an Application on the FX Application thread (should throw ISE)

Contribution.MD states, that it's automatically checked for whitespace errors? Is this not true?

As a clarification, this PR restores the previous behavior before Platform.startup. With this PR Application.launch and Platform.startup behaves the same.

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

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