On Tue, 4 Nov 2025 14:49:56 GMT, Lukasz Kostyra <[email protected]> wrote:

>> This PR fixes NPE thrown when trying to update D3D texture in some rare 
>> scenarios.
>> 
>> On more stressful cases (like the one using Canvas attached to this issue) 
>> it is possible that a D3DTexture.update() call will go through after the 
>> Resource Pool already pruned the underlying Texture's resource. This in turn 
>> caused an NPE, which propagated to higher levels and disrupted the rendering 
>> loop, causing the Canvas to not be drawn anymore. The update() call seems 
>> not to be called more than once on an already freed resource, suggesting 
>> this is some sort of rare race between the pool and the drawing code.
>> 
>> This change prevents the NPE from being thrown. I noticed no visual problems 
>> with the test even when the update() call is rejected by the newly added 
>> check. Best way to verify it is to add a log call inside added `if 
>> (!resource.isValid())` blocks when running the test, it will occasionally 
>> get printed but the test itself won't change its behavior like it does 
>> without this change.
>
> Lukasz Kostyra has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Review comments - MTLTexture: add isValid() check to update(MediaFrame)

Apologies for the delay, I got a bit consumed by D3D12 EA2 release and only now 
managed to get a closer look at the stack traces.

> I see this NPE (with this fix applied) when I go to more extremes with the 
> amount of images shown

I think both of these don't directly relate to Texture.update causing problems, 
but the fixes for them are trivial. I filed separate issues for those and will 
take a closer look at them later down the line:
- https://bugs.openjdk.org/browse/JDK-8372276
- https://bugs.openjdk.org/browse/JDK-8372275


This was waiting a bit for me to go through all the NPEs and stack traces 
reported, but I think I addressed them all. I'll leave this PR for a bit more 
time and if there is no objections I will integrate it tomorrow.

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

PR Comment: https://git.openjdk.org/jfx/pull/1951#issuecomment-3558436154

Reply via email to