RFR: 8201567: QuantumRenderer modifies buffer in use by JavaFX Application Thread [v2]
Kevin Rushforth
kcr at openjdk.java.net
Thu Jul 2 12:43:50 UTC 2020
On Wed, 1 Jul 2020 05:43:29 GMT, John Neffenger <github.com+1413266+jgneff at openjdk.org> wrote:
>>> It will also need to be tested using the SW pipeline on all platforms.
>>
>> Thanks for the reminder. I managed to build JavaFX on Windows and macOS today, so I'll test this pull request on those
>> platforms in addition to Linux desktop and embedded.
>
> I tested this pull request on all of the following platforms:
>
> * JavaFX desktop platforms (*amd64* architecture)
> * Windows SDK on Windows 10 Pro Version 2004
> * Mac OS X SDK on macOS 10.15.5 (Catalina)
> * Linux SDK on Ubuntu 16.04.6 LTS (Xenial Xerus)
>
> * JavaFX embedded platforms (*armhf* architecture)
> * armv6hf SDK (Monocle EPD) on Ubuntu 14.04.6 LTS (Trusty Tahr)
> * armv6hf SDK (Monocle VNC) on Ubuntu 20.04 LTS (Focal Fossa)
>
> This is a tricky timing problem on most platforms, so I used rather intrusive techniques to catch the error and verify
> it was fixed. The bug is easily visible only on the Monocle VNC platform.
> ### Desktop
>
> I tested the desktop platforms as follows:
>
> 1. I added `assert` statements and calls to `Thread.sleep` as shown by [this
> commit](https://github.com/jgneff/javafx-graphics/commit/ffc639e4) to catch the error. It looks like a lot of changes,
> but they simply call `sleep` to change the timing of the threads so that the JavaFX Application Thread is unable to
> keep up with the QuantumRenderer. The `assert` statements catch the error when the rendering thread starts looking for
> an unused buffer.
> All platforms printed many `InvalidMarkException` errors as the buffer position was modified while in use on the JavaFX
> application thread. The Linux and Windows platforms printed, "Exception in thread 'JavaFX Application Thread'
> java.nio.InvalidMarkException," while the macOS platform printed, "Exception in thread 'AppKit Thread'
> java.nio.InvalidMarkException."
>
> 2. I applied the fix to call `getBuffer` instead of `getPixels` in `QueuedPixelSource`.
>
> 3. After the fix, no errors were detected by the assertions on any of the platforms.
>
> ### Embedded
>
> I tested the embedded platforms as follows:
>
> 1. I added only `assert` statements to the `HeadlessScreen` and `EPDScreen` classes, as shown below. Both platforms
> printed many `InvalidMarkException` errors as the buffer position was modified while in use on the JavaFX application
> thread. 2. I applied the fix to call `getBuffer` instead of `getPixels` in `QueuedPixelSource`.
>
> 3. After the fix, no errors were detected by the assertions on either platform.
>
> #### `HeadlessScreen` and `EPDScreen`
>
> @Override
> public synchronized void uploadPixels(Buffer b, int x, int y,
> int width, int height, float alpha) {
> + assert b.mark() == b;
> pixels.composePixels(b, x, y, width, height, alpha);
> + assert b.reset() == b;
> }
>
> For the Monocle VNC platform, you can also simply connect and watch the bug corrupt the frames sent to the VNC client,
> as shown in my prior comment on this pull request.
> ### Other results
>
> I found some unexpected results, listed below.
>
> * JavaFX on Linux does not limit its frame rate to 60 Hz when using the hardware pipeline, reaching over 350 frames per
> second.
>
> * JavaFX on macOS ignores the system property `-Djavafx.animation.pulse=8` and runs at 60 Hz, even though it prints the
> message "Setting PULSE_DURATION to 8 hz."
>
> * JavaFX on macOS prints the error shown below when `Platform.exit` is called to end the application. The error also
> occurs on JavaFX 14.0.1 and 15-ea+6. The error does not occur when the window is closed manually using the mouse.
>
> Java has been detached already, but someone is still trying to use it at
> -[GlassViewDelegate dealloc]:
> /Users/john/src/jfx/modules/javafx.graphics/src/main/native-glass/mac/GlassViewDelegate.m:204
> 0 libglass.dylib 0x000000010eb6c05d -[GlassViewDelegate dealloc] + 221
> 1 libglass.dylib 0x000000010eb71af6 -[GlassView3D dealloc] + 246
> 2 libobjc.A.dylib 0x00007fff66937054 _ZN19AutoreleasePoolPage12releaseUntilEPP11objc_object + 134
> 3 libobjc.A.dylib 0x00007fff6691bdba objc_autoreleasePoolPop + 175
> 4 CoreFoundation 0x00007fff2dad23c5 __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__ + 23
> 5 CoreFoundation 0x00007fff2dad22f7 __CFRunLoopDoObservers + 457
> 6 CoreFoundation 0x00007fff2dad1895 __CFRunLoopRun + 874
> 7 CoreFoundation 0x00007fff2dad0ece CFRunLoopRunSpecific + 462
> 8 libjli.dylib 0x000000010be945c1 CreateExecutionEnvironment + 399
> 9 libjli.dylib 0x000000010be90752 JLI_Launch + 1354
> 10 java 0x000000010be83ca1 main + 375
> 11 libdyld.dylib 0x00007fff67acdcc9 start + 1
> 12 ??? 0x000000000000000b 0x0 + 11
>
> ### Test scripts
>
> Below are the scripts I used to run the tests. The scripts include the system property `-Djavafx.animation.pulse=8`,
> but I removed it when trying to recreate the bug with the added `assert` and `sleep` statements.
> #### Linux
>
> #!/bin/bash
> # Linux desktop
> $HOME/opt/jdk-14.0.1/bin/java -ea --show-version \
> --module-path=$HOME/lib/javafx-sdk-dev/lib \
> --add-modules=javafx.graphics \
> -Dprism.order=sw -Djavafx.animation.pulse=8 \
> -jar dist/epd-javafx.jar \
> --width=800 --height=600 --pattern=3 --loops=0
>
> #### macOS
>
> #!/bin/bash
> # macOS desktop
> $HOME/opt/jdk-14.0.1.jdk/Contents/Home/bin/java -ea --show-version \
> --module-path=$HOME/lib/javafx-sdk-dev/lib \
> --add-modules=javafx.graphics \
> -Dprism.order=sw -Djavafx.animation.pulse=8 \
> -jar dist/epd-javafx.jar \
> --width=800 --height=600 --pattern=3 --loops=0
>
> #### Windows
>
> #!/bin/bash
> # Windows desktop under Cygwin
> $HOME/opt/jdk-14.0.1/bin/java -ea --show-version \
> --module-path=$(cygpath -m $HOME/lib/javafx-sdk-dev/lib) \
> --add-modules=javafx.graphics \
> -Dprism.order=sw -Djavafx.animation.pulse=8 \
> -jar dist/epd-javafx.jar \
> --width=800 --height=600 --pattern=3 --loops=0
>
> REM Windows desktop under Command Prompt
> C:\cygwin64\home\john\opt\jdk-14.0.1\bin\java -ea --show-version^
> --module-path=C:/cygwin64/home/john/lib/javafx-sdk-dev/lib^
> --add-modules=javafx.graphics^
> -Dprism.order=sw -Djavafx.animation.pulse=8^
> -jar dist\epd-javafx.jar^
> --width=800 --height=600 --pattern=3 --loops=0
>
> #### Monocle EPD
>
> Run with `sudo`.
>
> #!/bin/bash
> # Monocle EPD platform on Linux armhf
> $HOME/opt/jdk-14.0.1+7/bin/java -ea --show-version \
> --module-path=$HOME/lib/armv6hf-sdk/lib \
> --add-modules=javafx.graphics \
> -Dglass.platform=Monocle -Dmonocle.platform=EPD -Dprism.order=sw \
> -Dmonocle.input.18/0/0/0.maxX=800 -Dmonocle.input.18/0/0/0.maxY=600 \
> -Dmonocle.epd.waveformMode=4 -jar dist/epd-javafx.jar \
> --width=800 --height=600 --pattern=3 --loops=0
>
> #### Monocle VNC
>
> #!/bin/bash
> # Monocle VNC platform on Linux armhf
> /usr/lib/jvm/java-14-openjdk-armhf/bin/java -ea --show-version \
> --module-path=$HOME/lib/armv6hf-sdk/lib \
> --add-modules=javafx.graphics \
> -Dprism.order=sw -Djavafx.animation.pulse=8 \
> -Dglass.platform=Monocle -Dmonocle.platform=VNC \
> -jar dist/epd-javafx.jar \
> --width=1024 --height=600 --pattern=3 --loops=0
> Does this fix the years old Linux JavaFX buffer reset bug?
Possibly. This is a race condition that can affect the use of `UploadingPainter`, which is used by the SW pipeline.
-------------
PR: https://git.openjdk.java.net/jfx/pull/255
More information about the openjfx-dev
mailing list