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

[Thiago Milczarek Sayao]

On Tue, 8 Oct 2024 13:22:42 GMT, Ambarish Rapte <arapte at openjdk.org> wrote:

>> Thiago Milczarek Sayao has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Let `createGraphics` fail if called after dispose
>
> modules/javafx.graphics/src/main/java/com/sun/prism/es2/IOSGLDrawable.java line 32:
> 
>> 30: 
>> 31:     private static native long nCreateDrawable(long nativeWindow, long nativeCtxInfo);
>> 32:     private static native void nDestroyDrawable(long nativeCtxInfo);
> 
> I would recommend to rename these method as `nReleaseDrawable`.
> I see we use names as  ReleaseXxxx and DeleteXxxx. The DestroyXxxx name would be a new addition, I think we could avoid the variation.

Renamed as suggested.

> modules/javafx.graphics/src/main/native-prism-es2/ios/IOSGLDrawable.c line 55:
> 
>> 53: 
>> 54:     /* initialize the structure */
>> 55:     memset(dInfo, 0, sizeof(DrawableInfo));
> 
> Minor: The declaration of `initializeDrawableInfo` from line#35 should be removed.

I've forgot it. Removed.

> modules/javafx.graphics/src/main/native-prism-es2/ios/IOSGLDrawable.c line 126:
> 
>> 124: 
>> 125:     free(dInfo);
>> 126: }
> 
> Minor: Please add a new line here.

Added.

> modules/javafx.graphics/src/main/native-prism-es2/macosx/MacGLDrawable.c line 120:
> 
>> 118: 
>> 119:     free(dInfo);
>> 120: }
> 
> Minor: Please add a new line here.

Added

> modules/javafx.graphics/src/main/native-prism-es2/windows/WinGLDrawable.c line 76:
> 
>> 74: 
>> 75:     /* initialize the structure */
>> 76:     memset(dInfo, 0, sizeof(DrawableInfo));
> 
> Minor: The declaration of `initializeDrawableInfo` from line#38 should be removed.

I've forgot it. Removed.

> modules/javafx.graphics/src/main/native-prism-es2/windows/WinGLDrawable.c line 148:
> 
>> 146: 
>> 147:     free(dInfo);
>> 148: }
> 
> Minor: Please add a new line here.

Added

> modules/javafx.graphics/src/main/native-prism-es2/x11/X11GLDrawable.c line 36:
> 
>> 34: #include "com_sun_prism_es2_X11GLDrawable.h"
>> 35: 
>> 36: extern void initializeDrawableInfo(DrawableInfo *dInfo);
> 
> Minor: The declaration of function `initializeDrawableInfo` should be removed as well.

I've forgot it. Removed.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1586#discussion_r1792039652
PR Review Comment: https://git.openjdk.org/jfx/pull/1586#discussion_r1792038878
PR Review Comment: https://git.openjdk.org/jfx/pull/1586#discussion_r1792037269
PR Review Comment: https://git.openjdk.org/jfx/pull/1586#discussion_r1792039091
PR Review Comment: https://git.openjdk.org/jfx/pull/1586#discussion_r1792038660
PR Review Comment: https://git.openjdk.org/jfx/pull/1586#discussion_r1792037511
PR Review Comment: https://git.openjdk.org/jfx/pull/1586#discussion_r1792038383