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

[Andy Goryachev]

On Thu, 31 Aug 2023 20:13:40 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> 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 properly synchronized 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, including any in any methods it is calling
> 4. For the FX methods note down all fields it accesses, including any in any methods it is calling
> 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.
> 
> edit: further clarifications

You do have a valid point, @hjohn .
Perhaps an easier approach would be to separate fields into awt and fx ones (perhaps renaming the fields to have awt- and fx- prefix?), and use invokeLater/runLater for inter-thread communications?

This way no locking/synchronization is required, we have no shared fields, and everything happens in the right thread (at the expense of some memory allocation).

What do you guys think?

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

PR Comment: https://git.openjdk.org/jfx/pull/1178#issuecomment-1701741640