RFR: 8277299: STACK_OVERFLOW in Java_sun_awt_shell_Win32ShellFolder2_getIconBits [v3]

[Alexey Ivanov]

On Sat, 4 Dec 2021 19:29:37 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>>> Can't we call it explicitly in the case where colorBits == NULL?
>> 
>> I would use the default handlers for bad_alloc since they can be changed in feature and keeping code that is intended to be used in two places is usually not a good idea.
>> 
>>> Alternatively, since you don't use the macros for try and catch, you can re-arrange code and perform cleanup after the catch-block. Just an idea… It may not work.
>> 
>> That might require a lot of code reformatting and the benefits are unclear.
>
>> > Alternatively, since you don't use the macros for try and catch, you can re-arrange code and perform cleanup after the catch-block. Just an idea… It may not work.
>> 
>> That might require a lot of code reformatting and the benefits are unclear.
> 
> I tried it, it doesn't, [see this diff](https://github.com/aivanov-jdk/jdk/commit/ccf7500cf0e4614b12c0882204a81d928fce0a82). It's close to your latest code, however, there's only one `catch` that handles both allocations and there are no `if`'s inside the `try`-block.

What about this approach?
[Compared to master](https://github.com/openjdk/jdk/compare/master...aivanov-jdk:8277299-bad-alloc-w32)
[Compared to your latest code](https://github.com/azuev-java/jdk/compare/JDK-8277299...aivanov-jdk:8277299-bad-alloc-w32)

I think it's cleaner in the use of exceptions. Each try-block allocates memory that it uses and frees it before the catch-block. There's no need for if's: if exception is thrown, the memory wasn't allocated, therefore the memory is freed only if no exception is thrown.

What do you think?

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

PR: https://git.openjdk.java.net/jdk/pull/6473