On Tue, 10 Dec 2019 16:27:10 GMT, Kevin Rushforth <k...@openjdk.org> wrote:

>> Issue: NPE in GlassScene.frameRendered().
>> 
>> Cause: scenePaintListener is set in setTKScenePaintListener(), used in 
>> frameRendered() and 
>> set to null in dispose().
>> setTKScenePaintListener() and dispose() are called on JavaFX Application 
>> Thread and 
>> frameRendered() is called by QuantumRenderer thread.
>> setTKScenePaintListener() and frameRendered() are synchronized but dispose() 
>> is not.
>> 
>> Fix:
>> dispose() should use the synchronized setTKScenePaintListener() to set 
>> scenePaintListener to null.
>> 
>> Verification:
>> This is a very rare issue and cannot be reliably reproduced with a test case.
> 
> The change looks OK as far as it goes, meaning that it will fix the specific 
> NPE reported by the bug and is looks like a safe fix.
> 
> Two questions:
> 
> 1. In addition to calling the synchronized `setTKScenePaintListener` method, 
> you moved the call to the beginning of `dispose`. Is there a reason you 
> needed to do this?
> 2. Do any of the other listeners or variables that are set in `dispose` have 
> the same problem (i.e., are any of the rest accessed from another thread)?

>     1. In addition to calling the synchronized `setTKScenePaintListener` 
> method, you moved the call to the beginning of `dispose`. Is there a reason 
> you needed to do this?

This is moved only to make first operation when disposing. However the race 
condition is very rare, but this will just reduce the chances little more.

>     2. Do any of the other listeners or variables that are set in `dispose` 
> have the same problem (i.e., are any of the rest accessed from another 
> thread)?

Other variable do not have a race condition. All are set or accessed on JavaFX 
Application thread.
There are some other variable (not in dispose()) like entireSceneDirty, 
painting, which are accessed on both threads but those are synchronized access.

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

PR: https://git.openjdk.java.net/jfx/pull/64

Reply via email to