Re: [OpenJDK 2D-Dev] RFR: 8263893: getPrinterNames() leaks nameArray if Java String allocation fails
On Fri, 19 Mar 2021 20:36:57 GMT, Alexey Ivanov 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
Re: [OpenJDK 2D-Dev] RFR: 8263893: getPrinterNames() leaks nameArray if Java String allocation fails
On Fri, 19 Mar 2021 20:36:57 GMT, Alexey Ivanov 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
Re: [OpenJDK 2D-Dev] RFR: 8263894: Convert defaultPrinter and printers fields to local variables [v2]
On Fri, 19 Mar 2021 21:28:54 GMT, Alexey Ivanov wrote: >> `PrintServiceLookupProvider` has `defaultPrinter` and `printers` fields but >> they are used only in `getDefaultPrintService()` and `refreshServices()` >> correspondingly. Thus these two fields can be converted to local variables >> in the corresponding methods. > > Alexey Ivanov has updated the pull request incrementally with one additional > commit since the last revision: > > Bring back the comment for printServices Marked as reviewed by azvegint (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3095
Re: [OpenJDK 2D-Dev] RFR: 8263894: Convert defaultPrinter and printers fields to local variables [v2]
On Fri, 19 Mar 2021 21:28:54 GMT, Alexey Ivanov wrote: >> `PrintServiceLookupProvider` has `defaultPrinter` and `printers` fields but >> they are used only in `getDefaultPrintService()` and `refreshServices()` >> correspondingly. Thus these two fields can be converted to local variables >> in the corresponding methods. > > Alexey Ivanov has updated the pull request incrementally with one additional > commit since the last revision: > > Bring back the comment for printServices Marked as reviewed by prr (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3095
Re: [OpenJDK 2D-Dev] RFR: 8263894: Convert defaultPrinter and printers fields to local variables [v2]
On Fri, 19 Mar 2021 21:05:57 GMT, Phil Race wrote: > Why did you delete the comments ? The comments highlighted the difference between what is stored in `printers` and in `printServices`: _excludes_ vs. _includes_ the default printer. However, according to the code, `printers` _included_ the default printer too. Now there's only `printServices` which contains all print services. I decided the comment didn't have much value now. Yet, upon thinking about it, the comment for `printServices` still provides additional insight: the array also contains the `defaultPrintService`. - PR: https://git.openjdk.java.net/jdk/pull/3095
Re: [OpenJDK 2D-Dev] RFR: 8263894: Convert defaultPrinter and printers fields to local variables [v2]
> `PrintServiceLookupProvider` has `defaultPrinter` and `printers` fields but > they are used only in `getDefaultPrintService()` and `refreshServices()` > correspondingly. Thus these two fields can be converted to local variables in > the corresponding methods. Alexey Ivanov has updated the pull request incrementally with one additional commit since the last revision: Bring back the comment for printServices - Changes: - all: https://git.openjdk.java.net/jdk/pull/3095/files - new: https://git.openjdk.java.net/jdk/pull/3095/files/cfe460c2..2b98fec2 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3095=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3095=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/3095.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3095/head:pull/3095 PR: https://git.openjdk.java.net/jdk/pull/3095
Re: [OpenJDK 2D-Dev] RFR: 8263894: Convert defaultPrinter and printers fields to local variables
On Fri, 19 Mar 2021 20:35:03 GMT, Alexey Ivanov wrote: > `PrintServiceLookupProvider` has `defaultPrinter` and `printers` fields but > they are used only in `getDefaultPrintService()` and `refreshServices()` > correspondingly. Thus these two fields can be converted to local variables in > the corresponding methods. Why did you delete the comments ? - PR: https://git.openjdk.java.net/jdk/pull/3095
Re: [OpenJDK 2D-Dev] RFR: 8263893: getPrinterNames() leaks nameArray if Java String allocation fails
On Fri, 19 Mar 2021 20:36:57 GMT, Alexey Ivanov 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
[OpenJDK 2D-Dev] RFR: 8263893: getPrinterNames() leaks nameArray if Java String allocation fails
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=3096=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
[OpenJDK 2D-Dev] RFR: 8263894: Convert defaultPrinter and printers fields to local variables
`PrintServiceLookupProvider` has `defaultPrinter` and `printers` fields but they are used only in `getDefaultPrintService()` and `refreshServices()` correspondingly. Thus these two fields can be converted to local variables in the corresponding methods. - Commit messages: - 8263894: Convert defaultPrinter and printers fields to local variables Changes: https://git.openjdk.java.net/jdk/pull/3095/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3095=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8263894 Stats: 5 lines in 1 file changed: 0 ins; 2 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/3095.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3095/head:pull/3095 PR: https://git.openjdk.java.net/jdk/pull/3095
[OpenJDK 2D-Dev] Integrated: 8247370: Clean up unused printing code in awt_PrintJob.cpp
On Thu, 18 Mar 2021 22:59:57 GMT, Phil Race wrote: > This removes a long time un-used code path. This pull request has now been integrated. Changeset: d41f7512 Author:Phil Race URL: https://git.openjdk.java.net/jdk/commit/d41f7512 Stats: 29 lines in 1 file changed: 0 ins; 19 del; 10 mod 8247370: Clean up unused printing code in awt_PrintJob.cpp Reviewed-by: serb, psadhukhan - PR: https://git.openjdk.java.net/jdk/pull/3083
Re: [OpenJDK 2D-Dev] RFR: 8247370: Clean up unused printing code in awt_PrintJob.cpp
On Fri, 19 Mar 2021 04:45:46 GMT, Prasanta Sadhukhan wrote: > Maybe we should put noreg-cleanup in JBS. done - PR: https://git.openjdk.java.net/jdk/pull/3083
Re: [OpenJDK 2D-Dev] RFR: 8263583: Emoji rendering on macOS
On Fri, 19 Mar 2021 13:54:34 GMT, Dmitry Batrak wrote: >> You the current image from the MacEmoji test as a golden image for other >> formats/transforms/extraalpha/etc. For example, if DST type is changed then >> it is unlikely the shape of the emoji will be changed as well, or if it is >> too big/ too small. >> >> BTW It will be good if the test will fail on the current metal >> implementation. > > Implementation for Metal pipeline has been added. > > I've also updated the test case to check rendering into different types of > images, including VolatileImage. The test will fail now, if corresponding > OpenGL (or Metal, if it's explicitly enabled) implementation part would be > rolled back. Looks good! - PR: https://git.openjdk.java.net/jdk/pull/3007
Re: [OpenJDK 2D-Dev] RFR: 8263583: Emoji rendering on macOS
On Thu, 18 Mar 2021 10:02:11 GMT, Sergey Bylokhov wrote: >> @mrserb I don't know how to check automatically that glyph painting works >> correctly. Could you please suggest a way to do it? In JetBrains Runtime we >> have a test that checks that rendered emoji glyph matches one of stored >> 'golden images', but I don't think that's suitable for OpenJDK, unless >> someone will volunteer to update that list when the need arises (e.g. when >> newer version of macOS changes the font, or rendering details). At the >> moment, we have already 7 golden images for a single glyph in our repository. > > You the current image from the MacEmoji test as a golden image for other > formats/transforms/extraalpha/etc. For example, if DST type is changed then > it is unlikely the shape of the emoji will be changed as well, or if it is > too big/ too small. > > BTW It will be good if the test will fail on the current metal implementation. Implementation for Metal pipeline has been added. I've also updated the test case to check rendering into different types of images, including VolatileImage. The test will fail now, if corresponding OpenGL (or Metal, if it's explicitly enabled) implementation part would be rolled back. - PR: https://git.openjdk.java.net/jdk/pull/3007
