On Tue, 4 Nov 2025 08:04:51 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 - ES2 check + extra code comments

The updates look good with one question about whether to update `MTLTexture` as 
well (see inline).

We still need to do one of two things relating to the JBS bug management:

1. If this solves [JDK-8368629](https://bugs.openjdk.org/browse/JDK-8368629), 
close [JDK-8352209](https://bugs.openjdk.org/browse/JDK-8352209) as a duplicate 
and change the title of this PR to `8368629: Texture.update sometimes invoked 
for a disposed Texture`
2. Otherwise, Change the title of this PR and 
[JDK-8352209](https://bugs.openjdk.org/browse/JDK-8352209) to describe the more 
general problem being solved rather than a D3D-specific problem.

@arapte Have you evaluated whether this solves 
[JDK-8368629](https://bugs.openjdk.org/browse/JDK-8368629)?

modules/javafx.graphics/src/main/java/com/sun/prism/es2/ES2Texture.java line 
784:

> 782:             return;
> 783:         }
> 784: 

I see that you applied this to both D3D and ES2. The corresponding MTLTexture 
also has an `update(MediaFrame frame, ...)` method which does not have this 
check. For consistency, it might be worth adding it there, too.

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

PR Review: https://git.openjdk.org/jfx/pull/1951#pullrequestreview-3416733053
PR Review Comment: https://git.openjdk.org/jfx/pull/1951#discussion_r2490708044

Reply via email to