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

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

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
Reply | Threaded
Open this post in threaded view
|

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

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
Reply | Threaded
Open this post in threaded view
|

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

Alexander Zvegintsev-2
In reply to this post by 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.

Marked as reviewed by azvegint (Reviewer).

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

PR: https://git.openjdk.java.net/jdk/pull/3096
Reply | Threaded
Open this post in threaded view
|

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

Sergey Bylokhov-2
In reply to this post by 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.

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
Reply | Threaded
Open this post in threaded view
|

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

Alexander Zuev-3
In reply to this post by 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.

Marked as reviewed by kizune (Reviewer).

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

PR: https://git.openjdk.java.net/jdk/pull/3096
Reply | Threaded
Open this post in threaded view
|

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

Alexey Ivanov-2
In reply to this post by Sergey Bylokhov-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
Reply | Threaded
Open this post in threaded view
|

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

Alexey Ivanov-2
In reply to this post by 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