RFR: 8201567: QuantumRenderer modifies buffer in use by JavaFX Application Thread [v2]
Kevin Rushforth
kcr at openjdk.java.net
Thu Jul 2 23:50:17 UTC 2020
On Thu, 2 Jul 2020 12:41:34 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:
>> 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.sh
>>
>> #!/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.sh
>>
>> #!/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.sh
>>
>> #!/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
>>
>> #### windows.bat
>>
>> 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.sh
>>
>> 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.sh
>>
>> #!/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.
This fix might be a candidate for JavaFX 15, so I recommend to _not_ merge the master branch.
If we don't spot anything of concern during the review, then we might ask you to retarget your PR to the `jfx15` branch.
-------------
PR: https://git.openjdk.java.net/jfx/pull/255
More information about the openjfx-dev
mailing list