On Wed, 20 Nov 2024 03:00:36 GMT, Alexander Zvegintsev <azveg...@openjdk.org> 
wrote:

>> This is the FX counterpart of the 
>> [openjdk/jdk/pull/22131](https://github.com/openjdk/jdk/pull/22131) / 
>> `b.`(aka crash prevention part)
>> 
>> It does not crash without the 
>> [openjdk/jdk/pull/22131](https://github.com/openjdk/jdk/pull/22131) / `a.` 
>> part.
>> 
>> The crash prevention part  is the same for the JDK and JavaFX, except that 
>> this PR includes a test.
>> 
>> ---- 
>> 
>> Internally the ScreenCast session keeps open for 2s (both JDK and JFX, and 
>> their implementations are almost identical).
>> This is to reduce overhead in case of frequent consecutive screen captures.
>> 
>> When we perform a 
>> [cleanup](https://github.com/openjdk/jdk/blob/db56266ad164b4ecae59451dc0a832097dbfbd8e/src/java.desktop/unix/native/libawt_xawt/awt/screencast_pipewire.c#L91)
>>  to close the session, we internally call 
>> [`pw_deinit`](https://docs.pipewire.org/group__pw__pipewire.html#gafa6045cd7391b467af4575c6752d6c4e).
>> 
>> It becomes a problem if these sessions overlap in time, so that the second 
>> session cleanup crashes when it tries to call pipewire functions without 
>> initializing the pipewire system by  
>> [`pw_init`](https://docs.pipewire.org/group__pw__pipewire.html#ga06c879b2d800579f4724109054368d99)
>>  (please note that `This function can be called multiple times.`).
>> 
>> So the solution is not to call `pw_deinit` because we don't really need it, 
>> and it needs to be applied to both the JDK and JavaFX.
>> 
>> [jdk 
>> commit](https://github.com/openjdk/jdk/pull/22131/commits/19956fda202269e92ec70670bc88c8d3c7accf73)
>>  / [jfx 
>> commit](https://github.com/openjdk/jfx/pull/1639/commits/dba8a8871a38831d0a0da697a2be41f0c240c8f0)
>> 
>> ----
>> 
>> The newly introduced test is disabled for now(as part  of 
>> [JDK-8335470](https://bugs.openjdk.org/browse/JDK-8335470)), because it 
>> requires the fix on both the JavaFX and JDK sides.
>> For the same reason 
>> `tests/system/src/test/java/test/robot/javafx/embed/swing/SwingNodeJDialogTest.java`
>>  and `tests/system/src/test/java/test/robot/javafx/scene/SRGBTest.java` are 
>> not enabled back.
>> But if the JDK and JavaFX have the fixes, everything works fine.
>> Testing in other different scenarios also looks good.
>
> Alexander Zvegintsev has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   improve crash detection

Looks good

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

Marked as reviewed by lkostyra (Committer).

PR Review: https://git.openjdk.org/jfx/pull/1639#pullrequestreview-2448594854

Reply via email to