On Thu, 13 Nov 2025 03:28:51 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 two > additional commits since the last revision: > > - debug log fix > - debug log fix The debug print logging is OK for debugging purposes to help validate your analysis, as long as you realize that it isn't a substitute for analysis. I do agree with John and Michael that keeping the debug statements isn't needed (or wanted) as part of the product, so I'd like to see it removed once this has proceeded to the point that we are otherwise ready to approve it. On that note, there are still unanswered questions about the thread safety of the `JFXPanel` class. We do know that, with the exception of setScene (and by extension, getScene()), all _public_ methods are specified to be called on the EDT (the AWT thread). `setScene` checks the thread and always calls `setSceneImpl` on the FX thread to do the "heavy lifting". The question then is around the rest of the implementation methods, some of which are called on the FX thread and some on the EDT. The NPE being hit by the test program is coming because `setScene(null)`, which is called on the FX app thread, will lead to calling `HostContainer::setEmbeddedStage(null)` and `HostContainer::setEmbeddedScene(null)`. Those methods will set the `stagePeer` and `scenePeer` fields to null. Meanwhile, a repaint operation or a mouse move or ... on the EDT could be trying to access the scene peer. Locally capturing the `stagePeer` and `scenePeer` fields in methods that are called on the EDT so that it doesn't change out from under us will prevent the NPE, but doesn't guarantee thread safety. In the case where it prevents the NPE, we go one to call a method on a scene peer that is no longer being used. This might be OK or it might not be. As long as this doesn't make things worse (and I don't see how it would), we could consider taking some variant of your proposed fix as a "workaround" to solve the NPE, but I'd like to understand the problem better. If we do take this fix, we would need to file a follow-on bug to fix the root cause (which could involve some design work to ensure thread safety without introducing deadlocks). ------------- PR Review: https://git.openjdk.org/jfx/pull/1968#pullrequestreview-3467010262
