On Thu, 31 Aug 2023 20:13:40 GMT, John Hendrikx <jhendr...@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
