On Fri, 15 Nov 2024 02:21:57 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 two > additional commits since the last revision: > > - test cleanup > - add missing copyright header tests/system/src/test/java/test/robot/javafx/embed/swing/LinuxScreencastHangCrashTest.java line 58: > 56: @BeforeAll > 57: public static void init() throws Exception { > 58: Assumptions.assumeTrue(!Util.isOnWayland()); // JDK-8335470 Once the AWT fix is in a promoted build of JDK 24, this can be replaced (here and in the other two test classes that are excluded on Wayland) with: assumeTrue(Runtime.version().feature() >= 24); I added this as a comment in the bug report for JDK-8335470 as well. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1639#discussion_r1845075239
