[OpenJDK 2D-Dev] Integrated: 8264002: Delete outdated assumptions about ColorSpace initialization
On Tue, 23 Mar 2021 00:14:59 GMT, Sergey Bylokhov wrote: > Some codes have outdated assumptions about the initialization of ColorSpace > class. > - The ColorSpace.getInstance() will never throw an IllegalArgumentException > for the builtin profiles > - The ColorSpace.getInstance() will not trigger initialisation of the CMM > classes This pull request has now been integrated. Changeset: cfc9aa34 Author:Sergey Bylokhov URL: https://git.openjdk.java.net/jdk/commit/cfc9aa34 Stats: 168 lines in 6 files changed: 45 ins; 94 del; 29 mod 8264002: Delete outdated assumptions about ColorSpace initialization Reviewed-by: azvegint - PR: https://git.openjdk.java.net/jdk/pull/3140
Re: [OpenJDK 2D-Dev] RFR: 8263984: Invalidate printServices when there are no printers
On Wed, 24 Mar 2021 13:07:21 GMT, Prasanta Sadhukhan wrote: > Since this is windows specific code, I am not sure if system will not have > any printers. I guess by default, Microsoft XPS Document Writer, Microsoft > Print-to-PDF, Fax are present in printers list. Yes, that's correct. But you can remove them. You can also configure the image so that these are not installed by default. > That is the reason we do not have getPrintService() return 0 printer although > there may not be any real printer present in windows system...reason for some > failure in jtreg test which caused us to use `@key printer `tag in those > tests to make real printers are configured. I agree that getting zero printers in Windows is unlikely. But it's still possible. Consider the following scenario: Let's assume there are 10 printers in the system. The user removes 5 of them. In this case, `invalidateService()` is called on the instances of `Win32PrintService` which were removed from the system. Then the user removes the remaining 5 printers. In this case, `invalidateService()` is not called at all. If an application has references to any of these instances, they will continue to appear operational. However, the flag `isInvalid` in `Win32PrintService` is used in two methods only: `getPrinterState` and `getPrinterStateReasons`. This fix is minor, probably this situation never occurs in the real life. The difference in handling the deleted services caught my attention. If everyone agrees it's not problem, I'll withdraw the PR and close the bug as _Not an Issue_. - PR: https://git.openjdk.java.net/jdk/pull/3151
Re: [OpenJDK 2D-Dev] RFR: 8263893: getPrinterNames() leaks nameArray if Java String allocation fails
On Fri, 19 Mar 2021 22:31:48 GMT, Sergey Bylokhov 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
Re: [OpenJDK 2D-Dev] RFR: 8264002: Delete outdated assumptions about ColorSpace initialization
On Tue, 23 Mar 2021 00:14:59 GMT, Sergey Bylokhov wrote: > Some codes have outdated assumptions about the initialization of ColorSpace > class. > - The ColorSpace.getInstance() will never throw an IllegalArgumentException > for the builtin profiles > - The ColorSpace.getInstance() will not trigger initialisation of the CMM > classes Looks good to me. - Marked as reviewed by azvegint (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3140
Re: [OpenJDK 2D-Dev] RFR: 8263984: Invalidate printServices when there are no printers
On Wed, 24 Mar 2021 09:25:56 GMT, Alexey Ivanov wrote: >> src/java.desktop/windows/classes/sun/print/PrintServiceLookupProvider.java >> line 120: >> >>> 118: // don't get the default. >>> 119: invalidateServices(); >>> 120: printServices = new PrintService[0]; >> >> I am not sure this call to invalidateServices() is needed as we are creating >> a new `printServices` object of 0 length so making the old printServices >> invalid is redundant as it is same global variable. > > That's exactly the problem I am addressing. > > In the regular case, the existing services left in `printServices` are > invalidated before `newServices` is assigned to it. Yet no existing services > are invalidated if all the printers were removed from the system. > > Why is it needed to invalidate deleted services if the list of printers is > updated and if at least one printer is left in the system? Why is it not > needed to invalidate deleted services if all the printers are removed from > the system? Since this is windows specific code, I am not sure if system will not have any printers. I guess by default, Microsoft XPS Document Writer, Microsoft Print-to-PDF, Fax are present in printers list. That is the reason we do not have getPrintService() return 0 printer although there may not be any real printer present in windows system...reason for some failure in jtreg test which caused us to use `@key printer `tag in those tests to make real printers are configured. - PR: https://git.openjdk.java.net/jdk/pull/3151
Re: [OpenJDK 2D-Dev] RFR: 8263984: Invalidate printServices when there are no printers
On Wed, 24 Mar 2021 09:16:33 GMT, Prasanta Sadhukhan wrote: >> When `getAllPrinterNames()` returns null, the list of `printServices` is >> assigned a new empty array without invalidating old services which were in >> the array before. >> >> The old print services should be invalidated. > > src/java.desktop/windows/classes/sun/print/PrintServiceLookupProvider.java > line 120: > >> 118: // don't get the default. >> 119: invalidateServices(); >> 120: printServices = new PrintService[0]; > > I am not sure this call to invalidateServices() is needed as we are creating > a new `printServices` object of 0 length so making the old printServices > invalid is redundant as it is same global variable. That's exactly the problem I am addressing. In the regular case, the existing services left in `printServices` are invalidated before `newServices` is assigned to it. Yet no existing services are invalidated if all the printers were removed from the system. Why is it needed to invalidate deleted services if the list of printers is updated and if at least one printer is left in the system? Why is it not needed to invalidate deleted services if all the printers are removed from the system? - PR: https://git.openjdk.java.net/jdk/pull/3151
Re: [OpenJDK 2D-Dev] RFR: 8263984: Invalidate printServices when there are no printers
On Tue, 23 Mar 2021 13:45:33 GMT, Alexey Ivanov wrote: > When `getAllPrinterNames()` returns null, the list of `printServices` is > assigned a new empty array without invalidating old services which were in > the array before. > > The old print services should be invalidated. src/java.desktop/windows/classes/sun/print/PrintServiceLookupProvider.java line 120: > 118: // don't get the default. > 119: invalidateServices(); > 120: printServices = new PrintService[0]; I am not sure this call to invalidateServices() is needed as we are creating a new `printServices` object of 0 length so making the old printServices invalid is redundant as it is same global variable. - PR: https://git.openjdk.java.net/jdk/pull/3151
[OpenJDK 2D-Dev] Integrated: 8263981: java.awt.image.ComponentSampleModel equals/hashcode use numBands twice
On Mon, 22 Mar 2021 15:55:16 GMT, Aleksey Shipilev wrote: > SonarCloud reports the problem in ComponentSampleModel.equals: > Correct one of the identical sub-expressions on both sides of operator "&&" > > ...near "this.numBands == that.numBands". It is checked twice. hashCode also > processes it twice. This pull request has now been integrated. Changeset: cb776edf Author:Aleksey Shipilev URL: https://git.openjdk.java.net/jdk/commit/cb776edf Stats: 4 lines in 1 file changed: 0 ins; 3 del; 1 mod 8263981: java.awt.image.ComponentSampleModel equals/hashcode use numBands twice Reviewed-by: serb, azvegint - PR: https://git.openjdk.java.net/jdk/pull/3125
Re: [OpenJDK 2D-Dev] RFR: 8263981: java.awt.image.ComponentSampleModel equals/hashcode use numBands twice
On Tue, 23 Mar 2021 22:17:56 GMT, Sergey Bylokhov wrote: >> Please do. I'll wait for a bit. > > I have found this bug, and the fix for it just implemented both methods at > once, so this is a typo. > https://bugs.openjdk.java.net/browse/JDK-4430788 Brilliant, thanks. I'll integrate now. - PR: https://git.openjdk.java.net/jdk/pull/3125
