RFR: 8255248: NullPointerException in JFXPanel due to race condition in HostContainer [v7]

[John Hendrikx]

On Wed, 30 Aug 2023 06:49:52 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:

>> Due to transient datatype of scenePeer, it can become null which can result in NPE in scenarios where scene is continuously been reset and set, which warrants a null check, as is done in other places for the same variable.
>
> Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add nullcheck for sceneState

The changes as they are now are IMHO entirely inadequate.  Simply surrounding all accesses to shared fields with a narrowly scoped `synchronized` block is not a magic solution. You will want the fields to be coherent for a longer period than that of a single field access.  For example:

        int lWidth = getCompWidth();
        int lHeight = getCompHeight();

The new methods `getCompWidth` and `getCompHeight` are synchronized, but the overall code here is not.  This means that if the control resizes from `1000 x 1` to `1 x 1000` you may end up with a `lWidth` and `lWeight` of `1000 x 1000` or `1 x 1`.

IMHO the steps to resolve this to proper code is:
1. Find all external entry points (all non-private methods, and any private methods used by a listener callback)
2. For the entry points found above, mark which are called by FX and which by AWT
3. For the AWT methods note down all fields it accesses
4. For the FX methods note down all fields it accesses
5. Find the fields that are accessed in both the AWT and FX lists
6. Mark any methods that access those fields as `synchronized` -- this may be too course grained if these methods are large, do I/O, call back into other systems -- in that case you may need to move some code around to do all the code that needs synchronization first or last and use a `synchronized` block.

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

PR Review: https://git.openjdk.org/jfx/pull/1178#pullrequestreview-1605639201