On Mon, 17 Nov 2025 16:35:50 GMT, Prasanta Sadhukhan <[email protected]> wrote:
>> NPE is seen while accessing transient "scenePeer" variable between reads.. >> Fix is made to store it in a temp variable rather than reading it twice >> since the value can change between successive reads in many places it is >> accessed. >> Also some debug logs added to be enabled via `jfxpanel.debug` property > > Prasanta Sadhukhan has updated the pull request incrementally with one > additional commit since the last revision: > > Remove debug, store transient var in temp var in EDT methods > Ideally, this component should be redesigned to ensure proper communication > between threads. That would be the best long-term solution, but that would be a large effort. > I wonder if we are going to continue chasing the issues until we do it right. Possibly. This gets back to the question I asked earlier: is the approach proposed in this PR a reasonable workaround that we might employ until such time as we "do it right"? If so, then we might continue down this path, addressing all outstanding comments, and making sure that we locally capture the scene/stage peer in all places where they are accessed from the EDT. > What do you think of the following idea: > > 1. rename all the variables to have `fx` and `edt` (or `sw` ?) prefix to > clearly indicate which thread controls the variable (sets, mutates, etc.) > 2. similarly rename the methods that can be renamed, and add comments to > methods that cannot be renamed I think adding comments as to which methods are called on which threads would be very useful (I wouldn't bother renaming any of the methods, since many/most of the interesting ones can't be). Similarly, adding comments to the fields as to which are modified on which thread, would be useful; I guess there might be some value in renaming the fields, but let's at least document this clearly. > 3. whenever the fields are read in a wrong thread, make a local copy (and > declare the field as `volatile`) This is basically what this PR attempts to do. It is a workaround for the lack of a proper MT solution, but it might very well be "good enough" until we can get a proper MT solution. > and maybe even go a bit further, instead of suggestion in 3), we explicitly > disallow accessing of fields from a wrong thread, and use the message passing > via Platform.runLater() and EventQueue.invokeLater() ? I don't think this is practical, or even desired. There are other ways to ensure that we consistently communicate values between the threads, possibly synchronizing operations at the right level of granularity (not just when you read or write the value). ------------- PR Comment: https://git.openjdk.org/jfx/pull/1968#issuecomment-3543886235
