RFR: 8263893: getPrinterNames() leaks nameArray if Java String allocation fails

[Alexey Ivanov-2]

If `JNU_NewStringPlatform` fails to allocate new Java String object for printer name, `std::bad_alloc` is thrown. The handler for the exception does not release the local reference to the `nameArray`, thus it will be leaked.

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

Commit messages:
 - 8263893: getPrinterNames() leaks nameArray if Java String allocation fails

Changes: https://git.openjdk.java.net/jdk/pull/3096/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3096&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8263893
  Stats: 4 lines in 1 file changed: 4 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3096.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3096/head:pull/3096

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

[Phil Race]

On Fri, 19 Mar 2021 20:36:57 GMT, Alexey Ivanov <[hidden email]> wrote:

> If `JNU_NewStringPlatform` fails to allocate new Java String object for printer name, `std::bad_alloc` is thrown. The handler for the exception does not release the local reference to the `nameArray`, thus it will be leaked.

This is fine but there's not really a leak since when the caller returns to Java localrefs are released anyway.

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

Marked as reviewed by prr (Reviewer).

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

[Alexander Zvegintsev-2]

On Fri, 19 Mar 2021 20:36:57 GMT, Alexey Ivanov <[hidden email]> wrote:

> If `JNU_NewStringPlatform` fails to allocate new Java String object for printer name, `std::bad_alloc` is thrown. The handler for the exception does not release the local reference to the `nameArray`, thus it will be leaked.

Marked as reviewed by azvegint (Reviewer).

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

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

[Sergey Bylokhov-2]

On Fri, 19 Mar 2021 20:36:57 GMT, Alexey Ivanov <[hidden email]> wrote:

> If `JNU_NewStringPlatform` fails to allocate new Java String object for printer name, `std::bad_alloc` is thrown. The handler for the exception does not release the local reference to the `nameArray`, thus it will be leaked.

src/java.desktop/windows/native/libawt/windows/WPrinterJob.cpp line 178:

> 176:     } catch (std::bad_alloc&) {
> 177:         delete [] pPrinterEnum;
> 178:         if (nameArray != NULL) {

Not sure that we usually clean the local refs in the native JNI methods, that only required in the native loop which are never returned to the java(I have check that by the grep of env->NewObjectArray)

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

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

[Alexander Zuev-3]

On Fri, 19 Mar 2021 20:36:57 GMT, Alexey Ivanov <[hidden email]> wrote:

> If `JNU_NewStringPlatform` fails to allocate new Java String object for printer name, `std::bad_alloc` is thrown. The handler for the exception does not release the local reference to the `nameArray`, thus it will be leaked.

Marked as reviewed by kizune (Reviewer).

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

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

[Alexey Ivanov-2]

On Fri, 19 Mar 2021 22:31:48 GMT, Sergey Bylokhov <[hidden email]> wrote:

>> If `JNU_NewStringPlatform` fails to allocate new Java String object for printer name, `std::bad_alloc` is thrown. The handler for the exception does not release the local reference to the `nameArray`, thus it will be leaked.
>
> src/java.desktop/windows/native/libawt/windows/WPrinterJob.cpp line 178:
>
>> 176:     } catch (std::bad_alloc&) {
>> 177:         delete [] pPrinterEnum;
>> 178:         if (nameArray != NULL) {
>
> Not sure that we usually clean the local refs in the native JNI methods, that only required in the native loop which are never returned to the java(I have check that by the grep of env->NewObjectArray)

You're right. It's not required: all local refs are cleared when the JNI method exits.
My reasoning was that `env->DeleteLocalRef(utf_str)` is called in the loop. Strictly, it's not required either. Yet if there's a large number of elements in the array, it could cause a problem.

I'm going to withdraw the PR as the fix is not necessary.

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

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

[Alexey Ivanov-2]

On Fri, 19 Mar 2021 20:36:57 GMT, Alexey Ivanov <[hidden email]> wrote:

> If `JNU_NewStringPlatform` fails to allocate new Java String object for printer name, `std::bad_alloc` is thrown. The handler for the exception does not release the local reference to the `nameArray`, thus it will be leaked.

This pull request has been closed without being integrated.

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

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