RFR: 8201567: QuantumRenderer modifies buffer in use by JavaFX Application Thread

[Kevin Rushforth]

On Sun, 28 Jun 2020 02:35:16 GMT, John Neffenger <github.com+1413266+jgneff at openjdk.org> wrote:

>> modules/javafx.graphics/src/main/java/com/sun/glass/ui/Pixels.java line 194:
>> 
>>> 193:     public final Buffer getBuffer() {
>>> 194:         assert this.bytes != null || this.ints != null;
>>> 195:         return this.bytes != null ? this.bytes : this.ints;
>> 
>> We should not use `assert` statements in the JavaFX runtime. If this is a condition that needs to be checked, then use
>> an `if` test and throw an appropriate `RuntimeException` or an `InternalError`.
>
> Thanks, Kevin. I saw `assert` used elsewhere (`EventLoop` in the same package, for example), and thought it was
> okay—even preferable—for these "should never occur" checks. Is that a general policy not to use `asserts` anywhere in
> JavaFX? Should I also remove the asserts I added to `HeadlessScreen` and `EPDScreen` in lieu of a unit test case?

I don't know whether it is documented, but we did at one point have a policy to avoid them and removed a few elsewhere
in glass (which is where most of the usage was). The main reason is that when running applications that want to use
assertion checking themselves it's too easy for a stray assert in a library to cause problems if we don't test for it.

In case it really is an invariant, and nothing that could be provoked by API calls (which I think this case is), then
checking explicitly seems best.

I wouldn't recommend removing any existing `assert` statements, but we could file a follow-up issue to remove them

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

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