RFR: 8341418: Prism/es2 DrawableInfo is never freed (leak) [v2]

[Thiago Milczarek Sayao]

On Sun, 6 Oct 2024 12:03:47 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:

>> Thiago Milczarek Sayao has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Review changes
>
> modules/javafx.graphics/src/main/java/com/sun/prism/es2/GLDrawable.java line 56:
> 
>> 54:     abstract boolean swapBuffers(GLContext glCtx);
>> 55: 
>> 56:     abstract void dispose();
> 
> `GLDrawable` could also get the dispose method by implementing `GraphicsResource`.

Yes, better.

> modules/javafx.graphics/src/main/java/com/sun/prism/es2/IOSGLDrawable.java line 57:
> 
>> 55:     @Override
>> 56:     void dispose() {
>> 57:         nDestroyDrawable(getNativeDrawableInfo());
> 
> You should set `nativeDrawableInfo` to zero in this method (and check this before calling the native method), as otherwise we run the risk of freeing a stale pointer when the `dispose()` method is called repeatedly. The same should be done in all other implementations.

You're right. done.

> modules/javafx.graphics/src/main/native-prism-es2/GLDrawable.c line 44:
> 
>> 42: }
>> 43: 
>> 44: void deleteDrawableInfo(DrawableInfo *dInfo)
> 
> Maybe delete this whole file, as its only content is now a function that just calls `memset()`.

Agreed. deleted it.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1586#discussion_r1789249594
PR Review Comment: https://git.openjdk.org/jfx/pull/1586#discussion_r1789249716
PR Review Comment: https://git.openjdk.org/jfx/pull/1586#discussion_r1789249838