On Tue, 8 Mar 2022 20:47:45 GMT, Alexey Ivanov wrote:
> You're right. The old icon handles in `hOldIcon` and `hOldIconSm` will be
> leaked here if `CreateIconFromRaster` throws an exception.
I've submitted [JDK-8282862](https://bugs.openjdk.java.net/browse/JDK-8282862):
AwtWindow::SetIconData
On Tue, 8 Mar 2022 15:54:45 GMT, Zhengyu Gu wrote:
>> Please review this small patch that fixes a potential memory leak that
>> exception return fails to release allocated `cacheDirs`
>>
>> Test:
>>
>> - [x] jdk_desktop on Linux x86_64
>
> Zhengyu Gu has updated the pull request incrementally
On Tue, 8 Mar 2022 15:54:45 GMT, Zhengyu Gu wrote:
>> Please review this small patch that fixes a potential memory leak that
>> exception return fails to release allocated `cacheDirs`
>>
>> Test:
>>
>> - [x] jdk_desktop on Linux x86_64
>
> Zhengyu Gu has updated the pull request incrementally
On Tue, 8 Mar 2022 21:12:33 GMT, Zhengyu Gu wrote:
>>> I think `NewStringUTF()` can throw OOM and also return `NULL`, just which
>>> one you pick.
>>
>> Yes.
>>
>> But we're discussing whether a check for exception is necessary after
>> `(*env)->SetObjectArrayElement`.
>
>
On Tue, 8 Mar 2022 21:05:13 GMT, Alexey Ivanov wrote:
>> I think `NewStringUTF()` can throw OOM and also return `NULL`, just which
>> one you pick.
>
>> I think `NewStringUTF()` can throw OOM and also return `NULL`, just which
>> one you pick.
>
> Yes.
>
> But we're discussing whether a
On Tue, 8 Mar 2022 21:02:04 GMT, Zhengyu Gu wrote:
> I think `NewStringUTF()` can throw OOM and also return `NULL`, just which one
> you pick.
Yes.
But we're discussing whether a check for exception is necessary after
`(*env)->SetObjectArrayElement`.
-
PR:
On Tue, 8 Mar 2022 20:57:27 GMT, Alexey Ivanov wrote:
>>> ??? You want to check and cleanup if NewStringUTF fails. You only want to
>>> call SetObjectElementArray if NewStringUTF succeeds.
>>
>> If the SetObjectArrayElement will throw an exception we will call the
>> NewStringUTF while the
On Tue, 8 Mar 2022 20:12:29 GMT, Sergey Bylokhov wrote:
>> Can `SetObjectElementArray` raise an exception?
>> The index is within the array length and we store a string. I assume
>> `cacheDirArray` is a string array.
>
>> ??? You want to check and cleanup if NewStringUTF fails. You only want
On Tue, 8 Mar 2022 15:54:45 GMT, Zhengyu Gu wrote:
>> Please review this small patch that fixes a potential memory leak that
>> exception return fails to release allocated `cacheDirs`
>>
>> Test:
>>
>> - [x] jdk_desktop on Linux x86_64
>
> Zhengyu Gu has updated the pull request incrementally
On Tue, 8 Mar 2022 13:37:44 GMT, Alexey Ivanov wrote:
>> ??? You want to check and cleanup if NewStringUTF fails. You only want to
>> call SetObjectElementArray if NewStringUTF succeeds.
>
> Can `SetObjectElementArray` raise an exception?
> The index is within the array length and we store a
On Tue, 8 Mar 2022 15:58:57 GMT, Alexey Ivanov wrote:
> > I did a quick grep, there are a few suspicious spots, e.g.
> > [https://github.com/openjdk/jdk/blob/master/src/java.desktop/windows/native/libawt/windows/awt_Window.cpp#L2711](url),
> > I have yet take a close look.
>
> It doesn't leak
On Tue, 8 Mar 2022 15:54:45 GMT, Zhengyu Gu wrote:
>> Please review this small patch that fixes a potential memory leak that
>> exception return fails to release allocated `cacheDirs`
>>
>> Test:
>>
>> - [x] jdk_desktop on Linux x86_64
>
> Zhengyu Gu has updated the pull request incrementally
> Please review this small patch that fixes a potential memory leak that
> exception return fails to release allocated `cacheDirs`
>
> Test:
>
> - [x] jdk_desktop on Linux x86_64
Zhengyu Gu has updated the pull request incrementally with one additional
commit since the last revision:
On Fri, 4 Mar 2022 13:25:12 GMT, Zhengyu Gu wrote:
> Please review this small patch that fixes a potential memory leak that
> exception return fails to release allocated `cacheDirs`
>
> Test:
>
> - [x] jdk_desktop on Linux x86_64
>
> > The macros are used to hide the different syntax of
On Tue, 8 Mar 2022 12:20:38 GMT, David Holmes wrote:
>> src/java.desktop/unix/native/common/awt/fontpath.c line 940:
>>
>>> 938: JNU_CHECK_EXCEPTION_AND_CLEANUP(env,
>>> (*FcStrListDone)(cacheDirs));
>>> 939:
>>> 940: (*env)->SetObjectArrayElement(env,
On Tue, 8 Mar 2022 09:18:01 GMT, Sergey Bylokhov wrote:
>> Please review this small patch that fixes a potential memory leak that
>> exception return fails to release allocated `cacheDirs`
>>
>> Test:
>>
>> - [x] jdk_desktop on Linux x86_64
>
>
On Tue, 8 Mar 2022 09:20:19 GMT, Sergey Bylokhov wrote:
>> Please review this small patch that fixes a potential memory leak that
>> exception return fails to release allocated `cacheDirs`
>>
>> Test:
>>
>> - [x] jdk_desktop on Linux x86_64
>
>
On Fri, 4 Mar 2022 13:25:12 GMT, Zhengyu Gu wrote:
> Please review this small patch that fixes a potential memory leak that
> exception return fails to release allocated `cacheDirs`
>
> Test:
>
> - [x] jdk_desktop on Linux x86_64
src/java.desktop/unix/native/common/awt/fontpath.c line 938:
On Fri, 4 Mar 2022 21:54:57 GMT, Zhengyu Gu wrote:
> The macros are used to hide the different syntax of c and c++ to avoid
> polluting code here, it seems to be a common pattern in client code.
That's to allow for general use by C or C++ source files. But you are only
needing this in a C
On Fri, 4 Mar 2022 21:30:53 GMT, David Holmes wrote:
> I would have just inlined JNU_CHECK_EXCEPTION and added the cleanup code
> directly. The additional macro wasn't really necessary and would not really
> be usable for any kind of general cleanup actions beyond a one-liner.
>
> But it is
On Fri, 4 Mar 2022 13:25:12 GMT, Zhengyu Gu wrote:
> Please review this small patch that fixes a potential memory leak that
> exception return fails to release allocated `cacheDirs`
>
> Test:
>
> - [x] jdk_desktop on Linux x86_64
I would have just inlined JNU_CHECK_EXCEPTION and added the
On Fri, 4 Mar 2022 13:25:12 GMT, Zhengyu Gu wrote:
> Please review this small patch that fixes a potential memory leak that
> exception return fails to release allocated `cacheDirs`
>
> Test:
>
> - [x] jdk_desktop on Linux x86_64
I've increased the number of reviewers because as well as the
On Fri, 4 Mar 2022 13:25:12 GMT, Zhengyu Gu wrote:
> Please review this small patch that fixes a potential memory leak that
> exception return fails to release allocated `cacheDirs`
>
> Test:
>
> - [x] jdk_desktop on Linux x86_64
Looks good.
-
Marked as reviewed by stuefe
Please review this small patch that fixes a potential memory leak that
exception return fails to release allocated `cacheDirs`
Test:
- [x] jdk_desktop on Linux x86_64
-
Commit messages:
- 8282628: Potential memory leak in sun.font.FontConfigManager.getFontConfig()
Changes:
24 matches
Mail list logo
