Re: [jdk17] RFR: 8269403: Fix jpackage tests to gracefully handle jpackage app launcher crashes
On Fri, 25 Jun 2021 22:41:10 GMT, Alexey Semenyuk wrote: > jpackage app launcher randomly crashes JVM at termination in EMS enabled JDK > builds. > Until the root cause of the issue is understood and fixed let's add a > workaround to jpackage tests to run test app few more times if the first > attempt resulted in app crash. Marked as reviewed by almatvee (Reviewer). - PR: https://git.openjdk.java.net/jdk17/pull/153
Re: [jdk17] RFR: 8269409: Post JEP 411 refactoring: core-libs with maximum covering > 10K [v2]
> More refactoring to limit the scope of `@SuppressWarnings` annotations. > > Sometimes I introduce new methods. Please feel free to suggest method names > you like to use. Weijun Wang has updated the pull request incrementally with one additional commit since the last revision: one more - Changes: - all: https://git.openjdk.java.net/jdk17/pull/152/files - new: https://git.openjdk.java.net/jdk17/pull/152/files/d8b384df..774eb9da Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk17=152=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk17=152=00-01 Stats: 2 lines in 1 file changed: 1 ins; 1 del; 0 mod Patch: https://git.openjdk.java.net/jdk17/pull/152.diff Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/152/head:pull/152 PR: https://git.openjdk.java.net/jdk17/pull/152
Re: [jdk17] RFR: 8269034: AccessControlException for SunPKCS11 daemon threads [v2]
On Tue, 22 Jun 2021 20:08:03 GMT, Sean Coffey wrote:
>> Sufficient permissions missing if this code was ever to run with
>> SecurityManager.
>>
>> Cleanest approach appears to be use of InnocuousThread to create the
>> cleaner/poller threads.
>> Test case coverage extended to cover the SecurityManager scenario.
>>
>> Reviewer request: @valeriepeng
>
> Sean Coffey has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Move TokenPoller to Runnable
test/jdk/sun/security/pkcs11/Provider/MultipleLogins.java line 63:
> 61: Policy.setPolicy(new SimplePolicy());
> 62: System.setSecurityManager(new SecurityManager());
> 63: }
Just curious, why split the loop into 2 and set the SecurityManager in between
the two loops? Can't we just set the policy/security manager before the loop?
test/jdk/sun/security/pkcs11/Provider/MultipleLogins.java line 137:
> 135: perms.add(new SecurityPermission("insertProvider.*"));
> 136: perms.add(new SecurityPermission("removeProvider.*"));
> 137: }
The test still pass without the following permission:
perms.add(new RuntimePermission("accessClassInPackage.sun.*"));
perms.add(new
RuntimePermission("accessClassInPackage.sun.security.pkcs11.*"));
perms.add(new SecurityPermission("clearProviderProperties.*"));
Remove them?
test/jdk/sun/security/pkcs11/Provider/MultipleLogins.sh line 142:
> 140: -Dtest.src=${TESTSRC} \
> 141: -Dtest.classes=${TESTCLASSES} \
> 142: -Djava.security.debug=${DEBUG} \
Save these java options and use it for both invocation? This way it's easier to
tell that there is no difference among these two except for the extra argument.
test/jdk/sun/security/pkcs11/Provider/MultipleLogins.sh line 143:
> 141: -Dtest.classes=${TESTCLASSES} \
> 142: -Djava.security.debug=${DEBUG} \
> 143: MultipleLogins ${TESTSRC}${FS}MultipleLogins.policy || exit 11
There is no MultipleLogins.policy file. The test just uses the internal
SimplePolicy object. Maybe just use a string like "useSimplePolicy".
-
PR: https://git.openjdk.java.net/jdk17/pull/117
[jdk17] RFR: 8269403: Fix jpackage tests to gracefully handle jpackage app launcher crashes
jpackage app launcher randomly crashes JVM at termination in EMS enabled JDK builds. Until the root cause of the issue is understood and fixed let's add a workaround to jpackage tests to run test app few more times if the first attempt resulted in app crash. - Commit messages: - whitespace clean up - 8269403: Fix jpackage tests to gracefully handle jpackage app launcher crashes Changes: https://git.openjdk.java.net/jdk17/pull/153/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk17=153=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8269403 Stats: 9 lines in 2 files changed: 5 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk17/pull/153.diff Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/153/head:pull/153 PR: https://git.openjdk.java.net/jdk17/pull/153
Re: [jdk17] RFR: 8269409: Post JEP 411 refactoring: core-libs with maximum covering > 10K
On Fri, 25 Jun 2021 20:04:37 GMT, Weijun Wang wrote: > More refactoring to limit the scope of `@SuppressWarnings` annotations. > > Sometimes I introduce new methods. Please feel free to suggest method names > you like to use. LGTM. - Marked as reviewed by naoto (Reviewer). PR: https://git.openjdk.java.net/jdk17/pull/152
Re: RFR: 6766844: ByteArrayInputStream#read with a byte array of length 0 not consistent with InputStream when at EOF [v4]
On Fri, 25 Jun 2021 20:47:21 GMT, Brian Burkhalter wrote: >> Modify `java.io.ByteArrayInputStream` methods `read(byte[])` and >> `read(byte[],int,int)` to return zero per the `InputStream` specification >> when the byte array actual or specified length is zero. > > Brian Burkhalter has refreshed the contents of this pull request, and > previous commits have been removed. The incremental views will show > differences compared to the previous content of the PR. Agree that clarifying the spec would be a much safer solution. - Marked as reviewed by naoto (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4591
Re: [jdk17] RFR: 8269034: AccessControlException for SunPKCS11 daemon threads [v2]
On Fri, 25 Jun 2021 19:39:22 GMT, Valerie Peng wrote:
>> Sean Coffey has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Move TokenPoller to Runnable
>
> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/SunPKCS11.java line
> 952:
>
>> 950: AccessController.doPrivileged((PrivilegedAction) () -> {
>> 951: Thread t = InnocuousThread.newSystemThread(
>> 952: "Poller " + getName(),
>
> nit: "Poller " -> "Poller-" (like before)?
It seems that the AccessController.doPrivileged((PrivilegedAction) () ->
{} is un-necessary? I tried your test without it and test still passes.
-
PR: https://git.openjdk.java.net/jdk17/pull/117
Re: [jdk17] RFR: 8269034: AccessControlException for SunPKCS11 daemon threads [v2]
On Tue, 22 Jun 2021 20:08:03 GMT, Sean Coffey wrote:
>> Sufficient permissions missing if this code was ever to run with
>> SecurityManager.
>>
>> Cleanest approach appears to be use of InnocuousThread to create the
>> cleaner/poller threads.
>> Test case coverage extended to cover the SecurityManager scenario.
>>
>> Reviewer request: @valeriepeng
>
> Sean Coffey has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Move TokenPoller to Runnable
src/java.base/share/lib/security/default.policy line 131:
> 129: permission java.lang.RuntimePermission
> "accessClassInPackage.com.sun.crypto.provider";
> 130: permission java.lang.RuntimePermission
> "accessClassInPackage.jdk.internal.misc";
> 131: permission java.lang.RuntimePermission
> "accessClassInPackage.sun.security.*";
Can we just do necessary changes? I noticed that this file seems to have mixed
style, i.e. some lines are longer than 80 chars and some break into 2 lines
with length less than 80 chars. Since the whole file is mixed, maybe just do
what must be changed.
src/java.base/share/lib/security/default.policy line 142:
> 140: permission java.security.SecurityPermission
> "clearProviderProperties.*";
> 141: permission java.security.SecurityPermission
> "removeProviderProperty.*";
> 142: permission java.security.SecurityPermission
> "getProperty.auth.login.defaultCallbackHandler";
Same "avoid unnecessary changes" comment here.
src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/SunPKCS11.java line
952:
> 950: AccessController.doPrivileged((PrivilegedAction) () -> {
> 951: Thread t = InnocuousThread.newSystemThread(
> 952: "Poller " + getName(),
nit: "Poller " -> "Poller-" (like before)?
src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/SunPKCS11.java line
956:
> 954: assert t.getContextClassLoader() == null;
> 955: t.setDaemon(true);
> 956: t.setPriority(Thread.MIN_PRIORITY);
nit: supply this priority value as an argument to the
InnocuousThread.newSystemThread() call instead?
src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/SunPKCS11.java line
1033:
> 1031: }
> 1032: cleaner = new NativeResourceCleaner();
> 1033: AccessController.doPrivileged((PrivilegedAction) () -> {
It seems that the AccessController.doPrivileged((PrivilegedAction) () -> {} is
un-necessary? I tried your test without it and test still passes.
src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/SunPKCS11.java line
1039:
> 1037: assert t.getContextClassLoader() == null;
> 1038: t.setDaemon(true);
> 1039: t.setPriority(Thread.MIN_PRIORITY);
nit: supply this priority value as an argument to the
InnocuousThread.newSystemThread() call instead?
src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/SunPKCS11.java line
1212:
> 1210:
> 1211: this.token = token;
> 1212: if (cleaner == null) {
This check seems duplicate to the one in createCleaner() call.
test/jdk/sun/security/pkcs11/Provider/MultipleLogins.java line 56:
> 54: System.out.println("No NSS config found. Skipping.");
> 55: return;
> 56: }
Move this if-check block of code up before the for-loop?
-
PR: https://git.openjdk.java.net/jdk17/pull/117
[jdk17] Integrated: 8269036: tools/jpackage/share/AppImagePackageTest.java failed with "hdiutil: create failed - Resource busy"
On Tue, 22 Jun 2021 21:59:43 GMT, Alexander Matveev wrote: > Looks like another "Resource busy" issue similar to recent fixes for "hdiutil > convert" and "hdiutil detach". Workaround in same way by repeating "create" > command. Modified RetryExecutor to pass write to file flag, otherwise > "hdiutil create" might deadlock. > Also, repeat is done only for creating DMG using size instead of creating > from app image. In some cases create DMG from app image might run for very > long time and eventually fail, this is why fallback to create DMG from size > was added. This pull request has now been integrated. Changeset: fb0a95fe Author:Alexander Matveev URL: https://git.openjdk.java.net/jdk17/commit/fb0a95fed46a04475697204de576c57f98d5b55a Stats: 16 lines in 2 files changed: 13 ins; 0 del; 3 mod 8269036: tools/jpackage/share/AppImagePackageTest.java failed with "hdiutil: create failed - Resource busy" Reviewed-by: asemenyuk, herrick - PR: https://git.openjdk.java.net/jdk17/pull/122
Re: [jdk17] RFR: JDK-8269351: Proxy::newProxyInstance and MethodHandleProxies::asInterfaceInstance should reject sealed interfaces
On Fri, 25 Jun 2021 17:24:58 GMT, Mandy Chung wrote: > `java.lang.reflect.Proxy::newProxyInstance` and > `java.lang.invoke.MethodHandleProxies::asInterfaceInstance` do not specify > how to deal with sealed interfaces. These APIs should reject sealed > interface with `IllegalArgumentException` which is thrown if the given > interface is invalid. > > Please review CSR: > https://bugs.openjdk.java.net/browse/JDK-8269396 Marked as reviewed by darcy (Reviewer). - PR: https://git.openjdk.java.net/jdk17/pull/148
Re: RFR: 6766844: ByteArrayInputStream#read with a byte array of length 0 not consistent with InputStream when at EOF [v4]
On Fri, 25 Jun 2021 20:49:16 GMT, Roger Riggs wrote:
>> Brian Burkhalter has refreshed the contents of this pull request, and
>> previous commits have been removed. The incremental views will show
>> differences compared to the previous content of the PR.
>
> src/java.base/share/classes/java/io/ByteArrayInputStream.java line 163:
>
>> 161: * @apiNote
>> 162: * Unlike the {@link InputStream#read(byte[],int,int) equivalent
>> method}
>> 163: * of {@code InputStream}, this method returns {@code -1} instead
>> of zero
>
> I think I would say superclass method instead of equivalent method.
> Or prehaps overridden method.
I think overridden would work best.
-
PR: https://git.openjdk.java.net/jdk/pull/4591
Re: RFR: 6766844: ByteArrayInputStream#read with a byte array of length 0 not consistent with InputStream when at EOF [v4]
On Fri, 25 Jun 2021 20:47:21 GMT, Brian Burkhalter wrote:
>> Modify `java.io.ByteArrayInputStream` methods `read(byte[])` and
>> `read(byte[],int,int)` to return zero per the `InputStream` specification
>> when the byte array actual or specified length is zero.
>
> Brian Burkhalter has refreshed the contents of this pull request, and
> previous commits have been removed. The incremental views will show
> differences compared to the previous content of the PR.
src/java.base/share/classes/java/io/ByteArrayInputStream.java line 163:
> 161: * @apiNote
> 162: * Unlike the {@link InputStream#read(byte[],int,int) equivalent
> method}
> 163: * of {@code InputStream}, this method returns {@code -1} instead of
> zero
I think I would say superclass method instead of equivalent method.
Or prehaps overridden method.
-
PR: https://git.openjdk.java.net/jdk/pull/4591
Re: RFR: 6766844: ByteArrayInputStream#read with a byte array of length 0 not consistent with InputStream when at EOF [v4]
> Modify `java.io.ByteArrayInputStream` methods `read(byte[])` and > `read(byte[],int,int)` to return zero per the `InputStream` specification > when the byte array actual or specified length is zero. Brian Burkhalter has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 6766844: Add an API note instead of changing impl - Changes: - all: https://git.openjdk.java.net/jdk/pull/4591/files - new: https://git.openjdk.java.net/jdk/pull/4591/files/3152cdc7..a53849a2 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4591=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4591=02-03 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/4591.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4591/head:pull/4591 PR: https://git.openjdk.java.net/jdk/pull/4591
Re: RFR: 6766844: ByteArrayInputStream#read with a byte array of length 0 not consistent with InputStream when at EOF [v3]
> Modify `java.io.ByteArrayInputStream` methods `read(byte[])` and > `read(byte[],int,int)` to return zero per the `InputStream` specification > when the byte array actual or specified length is zero. Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision: 6766844: Add an @apiNote instead of changing impl - Changes: - all: https://git.openjdk.java.net/jdk/pull/4591/files - new: https://git.openjdk.java.net/jdk/pull/4591/files/68b6f089..3152cdc7 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4591=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4591=01-02 Stats: 10 lines in 2 files changed: 5 ins; 3 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/4591.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4591/head:pull/4591 PR: https://git.openjdk.java.net/jdk/pull/4591
Re: [jdk17] RFR: 8269409: Post JEP 411 refactoring: core-libs with maximum covering > 10K
On Fri, 25 Jun 2021 20:04:37 GMT, Weijun Wang wrote: > More refactoring to limit the scope of `@SuppressWarnings` annotations. > > Sometimes I introduce new methods. Please feel free to suggest method names > you like to use. Changes look good Max - Marked as reviewed by lancea (Reviewer). PR: https://git.openjdk.java.net/jdk17/pull/152
[jdk17] RFR: 8269409: Post JEP 411 refactoring: core-libs with maximum covering > 10K
More refactoring to limit the scope of `@SuppressWarnings` annotations. Sometimes I introduce new methods. Please feel free to suggest method names you like to use. - Commit messages: - 8269409: Post JEP 411 refactoring: core-libs with maximum covering > 10K Changes: https://git.openjdk.java.net/jdk17/pull/152/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk17=152=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8269409 Stats: 289 lines in 21 files changed: 161 ins; 64 del; 64 mod Patch: https://git.openjdk.java.net/jdk17/pull/152.diff Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/152/head:pull/152 PR: https://git.openjdk.java.net/jdk17/pull/152
Re: RFR: 8268698: Use Objects.check{Index, FromToIndex, FromIndexSize} where possible [v6]
On 6/21/2021 2:02 PM, Paul Sandoz wrote: On Mon, 21 Jun 2021 05:17:09 GMT, Yi Yang wrote: After JDK-8265518(#3615), it's possible to replace all variants of checkIndex by Objects.checkIndex/Objects.checkFromToIndex/Objects.checkFromIndexSize in the whole JDK codebase. Yi Yang has updated the pull request incrementally with one additional commit since the last revision: more replacement 2 All the updates to the check* methods look ok (requires some careful looking!). I cannot recall what others said about the change in exception messages. @jddarcy any advice here? Generally, the JDK does not have the text of exception message as a supported interface meant to be relied on by users. This doesn't stop developers from occasionally parsing those messages, but that is usually a sign of a missing API which we try to rectify more directly. HTH, -Joe
Integrated: 4847239: (spec) File.createTempFile() should make it clear that it doesn't create the temporary directory
On Wed, 23 Jun 2021 00:06:25 GMT, Brian Burkhalter wrote: > Augment the specification of > `java.io.File.createTempFile(String,String,File)` to clarify its behavior > with respect to the `File` parameter `directory`. This pull request has now been integrated. Changeset: 68ef21db Author:Brian Burkhalter URL: https://git.openjdk.java.net/jdk/commit/68ef21db415fb61ac9690290b692594da6b87ff9 Stats: 115 lines in 2 files changed: 114 ins; 0 del; 1 mod 4847239: (spec) File.createTempFile() should make it clear that it doesn't create the temporary directory Reviewed-by: naoto, lancea - PR: https://git.openjdk.java.net/jdk/pull/4561
Integrated: 6633375: FileOutputStream_md.c should be merged into FileOutputStream.c
On Thu, 24 Jun 2021 20:01:27 GMT, Brian Burkhalter wrote: > Merge identical Unix and Windows versions of FileOutputStream_md.c into > single, common FileOutputStream.c. This pull request has now been integrated. Changeset: 3fae4b37 Author:Brian Burkhalter URL: https://git.openjdk.java.net/jdk/commit/3fae4b372065b4293b64514e0679df419cd5c89a Stats: 77 lines in 2 files changed: 0 ins; 74 del; 3 mod 6633375: FileOutputStream_md.c should be merged into FileOutputStream.c Reviewed-by: naoto - PR: https://git.openjdk.java.net/jdk/pull/4589
[jdk17] RFR: JDK-8266313 (JEP-356) - RandomGenerator spec implementation requirements tightly coupled to JDK internal classes
The wording of the @implSpec referred to internal methods in the description. The patch rewords the @implSpec to be more descriptive of the algorithm than the methods used. - Commit messages: - non-infinite to finite - Use limiting - Remove use of clamping - Merge branch 'master' of https://github.com/openjdk/jdk17 into 8266313 - Javadoc changes Changes: https://git.openjdk.java.net/jdk17/pull/151/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk17=151=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8266313 Stats: 51 lines in 2 files changed: 22 ins; 4 del; 25 mod Patch: https://git.openjdk.java.net/jdk17/pull/151.diff Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/151/head:pull/151 PR: https://git.openjdk.java.net/jdk17/pull/151
Re: RFR: 8268457: XML Transformer outputs Unicode supplementary character incorrectly to HTML [v3]
On Wed, 23 Jun 2021 07:34:06 GMT, Masanori Yano wrote: >> Hi all, >> >> Could you please review the 8268457 bug fixes? >> >> The problem is that ToHTMLStream applies processing for non-surrogate pairs >> to the surrogate pair. >> This fix changes the processing for non-surrogate pairs to the else >> condition. > > Masanori Yano has updated the pull request incrementally with one additional > commit since the last revision: > > remove unnecessally comments and add eof line Hi Masanori, you may now type /integrate in a new comment when you're ready. I'll then sponsor your change. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/4474
Re: [jdk17] RFR: JDK-8266269: Lookup::accessClass fails with IAE when accessing an arrayClass with a protected inner class as component class [v2]
> `Lookup::accessClass` should determine the accessibility of the element type. > An array class is accessible if and only if its element type is accessible. > > This also fixes a spec bug to document `@throws NullPointerException` if the > argument is null. > > Please review the CSR: > https://bugs.openjdk.java.net/browse/JDK-8269312 Mandy Chung has updated the pull request incrementally with one additional commit since the last revision: remove extra import - Changes: - all: https://git.openjdk.java.net/jdk17/pull/137/files - new: https://git.openjdk.java.net/jdk17/pull/137/files/96da75be..0579507f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk17=137=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk17=137=00-01 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.java.net/jdk17/pull/137.diff Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/137/head:pull/137 PR: https://git.openjdk.java.net/jdk17/pull/137
[jdk17] Integrated: 8256919: BCEL: Utility.encode forget to close
On Thu, 24 Jun 2021 20:36:29 GMT, Joe Wang wrote: > Fix a regression caused by the previous BCEL update. The issue was fixed in > the current BCEL repo with a reversal of the previous code, adding back > "gos.close();". Note however, doing so will result in a warning: [try] > explicit call to close() on an auto-closeable resource. This fix calls > finish() instead. This pull request has now been integrated. Changeset: d799563a Author:Joe Wang URL: https://git.openjdk.java.net/jdk17/commit/d799563ac06d66acea6dbd9cb1fe78b253e8a0e7 Stats: 73 lines in 2 files changed: 72 ins; 0 del; 1 mod 8256919: BCEL: Utility.encode forget to close Reviewed-by: lancea, bpb, naoto - PR: https://git.openjdk.java.net/jdk17/pull/141
Integrated: 8266901: Clarify the method description of Duration.toDaysPart()
On Mon, 21 Jun 2021 18:22:26 GMT, Naoto Sato wrote: > Please review this doc clarification fix to `toDaysPart()` method. CSR will > also be filed accordingly. This pull request has now been integrated. Changeset: 223759fb Author:Naoto Sato URL: https://git.openjdk.java.net/jdk/commit/223759fb8af4a28f8ff8563e438ca285a87a9f2d Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod 8266901: Clarify the method description of Duration.toDaysPart() Reviewed-by: bpb, rriggs, lancea, iris, scolebourne - PR: https://git.openjdk.java.net/jdk/pull/4542
Re: RFR: 8269383: New java.nio.ByteOrder.isBigEndian and isLittleEndian methods
Hi David, In my humble option, it is reasonable to provide APIs to check whether the underlying platform is big-endian/little-endian.For most cases, we want a checking(byte order) rather than retrieving(byte order). -- From:David Lloyd Send Time:2021 Jun. 25 (Fri.) 21:52 To:Yi Yang Cc:core-libs-dev ; nio-dev Subject:Re: RFR: 8269383: New java.nio.ByteOrder.isBigEndian and isLittleEndian methods Is this better than the current solution of `nativeOrder() == BIG_ENDIAN` other than reducing a few keystrokes? On Fri, Jun 25, 2021 at 8:45 AM Yi Yang wrote: > > Hi, can I have a review of this change that adds two new utility methods for > java.nio.ByteOrder? Looking through the whole JDK codebase, most calls of > ByteOrder.nativeOrder() is to check if the underlying platform is > little-endian/big-endian. There is no reason to only provide > ByteOrder.nativeOrder while leaving big-endian/little-endian checking methods > blank. > > Thanks! > > - > > Commit messages: > - add new methods > > Changes: https://git.openjdk.java.net/jdk/pull/4595/files > Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4595=00 > Issue: https://bugs.openjdk.java.net/browse/JDK-8269383 > Stats: 20 lines in 1 file changed: 20 ins; 0 del; 0 mod > Patch: https://git.openjdk.java.net/jdk/pull/4595.diff > Fetch: git fetch https://git.openjdk.java.net/jdk pull/4595/head:pull/4595 > > PR: https://git.openjdk.java.net/jdk/pull/4595 > -- - DML • he/him
Re: RFR: 6766844: ByteArrayInputStream#read with a byte array of length 0 not consistent with InputStream when at EOF [v2]
On Fri, 25 Jun 2021 17:49:37 GMT, Roger Riggs wrote: >> Brian Burkhalter has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 6766844: Add bug ID to test > > The spec for ByteArrayInputStream.read(byte[], off, len) is pretty specific > about returning -1 if there is nothing to read. > The spec inherited from InputStream doesn't supercede javadoc of BAIS. > If the implementation is going to change then the spec will need to change > too. > It seems safer to document the inconsistency and avoid the risk of infinite > loops. @RogerRiggs I agree. - PR: https://git.openjdk.java.net/jdk/pull/4591
Re: RFR: 6766844: ByteArrayInputStream#read with a byte array of length 0 not consistent with InputStream when at EOF [v2]
On Fri, 25 Jun 2021 01:39:21 GMT, Brian Burkhalter wrote: >> Modify `java.io.ByteArrayInputStream` methods `read(byte[])` and >> `read(byte[],int,int)` to return zero per the `InputStream` specification >> when the byte array actual or specified length is zero. > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 6766844: Add bug ID to test The spec for ByteArrayInputStream.read(byte[], off, len) is pretty specific about returning -1 if there is nothing to read. The spec inherited from InputStream doesn't supercede javadoc of BAIS. If the implementation is going to change then the spec will need to change too. It seems safer to document the inconsistency and avoid the risk of infinite loops. - PR: https://git.openjdk.java.net/jdk/pull/4591
Re: [jdk17] RFR: JDK-8268826: Cleanup Override in Context-Specific Deserialization Filters [v6]
On Fri, 25 Jun 2021 16:48:45 GMT, Brent Christian wrote:
>> Roger Riggs has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Update logging of faults in jdk.serialFilterFactory to log only the
>> exception message
>> Simplify the logging.properties to only the needed settings
>
> test/jdk/java/io/Serializable/serialFilter/SerialFilterFunctionTest.java line
> 49:
>
>> 47: // Enable logging
>> 48: System.setProperty("java.util.logging.config.file",
>> 49: System.getProperty("test.src", ".") +
>> "/logging.properties");
>
> Is `System.setProperty()` needed if it's already being set with -D ?
Fooey; belt and suspenders not needed here.
In some cases, I used the static initializer to avoid adding command line
arguments that would
obscure the command line arguments being tested.
In this case, the log may be interesting for debugging but not integral to the
test correctness.
-
PR: https://git.openjdk.java.net/jdk17/pull/85
Re: RFR: 6766844: ByteArrayInputStream#read with a byte array of length 0 not consistent with InputStream when at EOF [v2]
On Fri, 25 Jun 2021 01:39:21 GMT, Brian Burkhalter wrote: >> Modify `java.io.ByteArrayInputStream` methods `read(byte[])` and >> `read(byte[],int,int)` to return zero per the `InputStream` specification >> when the byte array actual or specified length is zero. > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 6766844: Add bug ID to test CSR JDK-8269399 created. - PR: https://git.openjdk.java.net/jdk/pull/4591
[jdk17] RFR: JDK-8269351: Proxy::newProxyInstance and MethodHandleProxies::asInterfaceInstance should reject sealed interfaces
`java.lang.reflect.Proxy::newProxyInstance` and `java.lang.invoke.MethodHandleProxies::asInterfaceInstance` do not specify how to deal with sealed interfaces. These APIs should reject sealed interface with `IllegalArgumentException` which is thrown if the given interface is invalid. Please review CSR: https://bugs.openjdk.java.net/browse/JDK-8269396 - Commit messages: - JDK-8269351: Proxy.newProxyInstance and MethodHandleProxies::asInterfaceInstance should reject sealed interfaces Changes: https://git.openjdk.java.net/jdk17/pull/148/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk17=148=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8269351 Stats: 101 lines in 4 files changed: 97 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk17/pull/148.diff Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/148/head:pull/148 PR: https://git.openjdk.java.net/jdk17/pull/148
Re: [jdk17] RFR: JDK-8268826: Cleanup Override in Context-Specific Deserialization Filters [v6]
On Fri, 25 Jun 2021 14:54:45 GMT, Roger Riggs wrote:
>> Remove the unnecessary special case "OVERRIDE" in jdk.serialFilterFactory
>> property.
>> Fix description in the example of a filter allowing platform classes.
>> Suppress some warnings about use of SecurityManager in tests.
>
> Roger Riggs has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Update logging of faults in jdk.serialFilterFactory to log only the
> exception message
> Simplify the logging.properties to only the needed settings
Marked as reviewed by bchristi (Reviewer).
test/jdk/java/io/Serializable/serialFilter/SerialFilterFunctionTest.java line
49:
> 47: // Enable logging
> 48: System.setProperty("java.util.logging.config.file",
> 49: System.getProperty("test.src", ".") +
> "/logging.properties");
Is `System.setProperty()` needed if it's already being set with -D ?
-
PR: https://git.openjdk.java.net/jdk17/pull/85
Re: [jdk17] RFR: JDK-8266269: Lookup::accessClass fails with IAE when accessing an arrayClass with a protected inner class as component class
On Fri, 25 Jun 2021 08:44:54 GMT, Chris Hegarty wrote: >> `Lookup::accessClass` should determine the accessibility of the element >> type. An array class is accessible if and only if its element type is >> accessible. >> >> This also fixes a spec bug to document `@throws NullPointerException` if the >> argument is null. >> >> Please review the CSR: >> https://bugs.openjdk.java.net/browse/JDK-8269312 > > test/jdk/java/lang/invoke/t8150782/TestFindClass.java line 42: > >> 40: >> 41: import org.testng.annotations.*; >> 42: import p.Foo; > > maybe a stray import? Yup. Will remove it. - PR: https://git.openjdk.java.net/jdk17/pull/137
Re: RFR: 6766844: ByteArrayInputStream#read with a byte array of length 0 not consistent with InputStream when at EOF [v2]
On Fri, 25 Jun 2021 01:39:21 GMT, Brian Burkhalter wrote: >> Modify `java.io.ByteArrayInputStream` methods `read(byte[])` and >> `read(byte[],int,int)` to return zero per the `InputStream` specification >> when the byte array actual or specified length is zero. > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 6766844: Add bug ID to test Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4591
Re: [jdk17] RFR: 8268766: Desugaring of pattern matching enum switch should be improved [v5]
On Wed, 23 Jun 2021 11:58:06 GMT, Jan Lahoda wrote:
>> Currently, an enum switch with patterns is desugared in a very non-standard,
>> and potentially slow, way. It would be better to use the standard
>> `typeSwitch` bootstrap to classify the enum constants. The bootstrap needs
>> to accept enum constants as labels in order to allow this. A complication is
>> that if an enum constant is missing, that is not an incompatible change for
>> the switch, and the switch should simply work as if the case for the missing
>> constant didn't exist. So, the proposed solution is to have a new bootstrap
>> `enumSwitch` that accepts `String`s in place of the enum constants, and will
>> internally convert them to the appropriate enum constants, and then it will
>> find the proper case similarly to `typeSwitch`.
>>
>> How does this look?
>
> Jan Lahoda has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Improving javadoc.
Bootstrap method looks. At first i thought why not let the class label just be
an actual `Class.class` instance, signaling that target enum class should be
used, but then thought perhaps it's better to be more literal: clarity of
inputs, unlikely label lists will be shared, and further it might be easier to
adapt them.
src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 227:
> 225: String invocationName,
> 226: MethodType invocationType,
> 227: Object... labels) throws
> NullPointerException,
I don't think there are any benefits to declaring the runtime exceptions in the
method declaration.
Other bootstrap methods declare a checked exception when certain linkage
constraints are violated (and the line between whats an
`IllegalArgumentException` or an explicit linkage checked exception can be
blurry). In your case, given the simplicity i think what you have is ok. We
could refine later with a specific checked exception for switch linkage
violations.
src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 256:
> 254: if (labelClass == Class.class) {
> 255: if (label != enumClassTemplate) {
> 256: throw new IllegalArgumentException("illegal Class label:
> " + label);
Can we refine the message to state the class label is not equal to the enum
class that is the target of the switch? LIkewise in the `else` block to state
the label is not of class String or the target enum class.
-
Marked as reviewed by psandoz (Reviewer).
PR: https://git.openjdk.java.net/jdk17/pull/81
Re: RFR: 8269383: New java.nio.ByteOrder.isBigEndian and isLittleEndian methods
On 25/06/2021 14:51, David Lloyd wrote: Is this better than the current solution of `nativeOrder() == BIG_ENDIAN` other than reducing a few keystrokes? I wouldn't expect many developers would need to be concerned with the platform endianness, it's more likely going to be something advanced, maybe a high performance library or something interaction with the native code or the platform. So it would be just a convenience, you can already can use ByteOrder.nativeOrder() and test if it returns BIG or LITTLE_ENDIAN. -Alan
Re: RFR: 8269384: Determine native byte order for StringUTF16 via ByteOrder
On Fri, 25 Jun 2021 13:40:54 GMT, Yi Yang wrote: > Prefer using ByteOrder to compute byte order for StringUTF16 to determining > byte order by native method StringUTF16.isBigEndian. Adding new dependencies in `String` quite probably risks bootstrapping issues. There is a reason why `String` calls into its own native methods for this. - PR: https://git.openjdk.java.net/jdk/pull/4596
Integrated: 8268469: Update java.time to use switch expressions
On Wed, 9 Jun 2021 15:41:59 GMT, Patrick Concannon wrote: > Hi, > > Could someone please review my code for updating the code in the `java.time` > packages to make use of the switch expressions? > > Kind regards, > Patrick This pull request has now been integrated. Changeset: 1d167978 Author:Patrick Concannon URL: https://git.openjdk.java.net/jdk/commit/1d167978e53603ccf1599f476143391e7db51992 Stats: 342 lines in 22 files changed: 6 ins; 87 del; 249 mod 8268469: Update java.time to use switch expressions Reviewed-by: lancea, naoto, dfuchs, iris, chegar - PR: https://git.openjdk.java.net/jdk/pull/4433
Re: RFR: 8269383: (bf) ByteOrder should expose methods to test if platform is big or little endian
On Fri, 25 Jun 2021 13:30:56 GMT, Yi Yang wrote: > Hi, can I have a review of this change that adds two new utility methods for > java.nio.ByteOrder? Looking through the whole JDK codebase, most calls of > ByteOrder.nativeOrder() is to check if the underlying platform is > little-endian/big-endian(e.g. #4596 ). There is no reason to only provide > ByteOrder.nativeOrder while leaving big-endian/little-endian checking methods > blank. For most cases(in JDK or in user code), we want a checking(byte order) > rather than retrieving(byte order). > > Thanks! Hi Chris, > Is there any other potential benefits, performance or otherwise, that than be > achieved by such an API? Unfortunately not. It's more like the enhancement of API expressiveness. Since these operations are trivial, these APIs will not improve/degrade the performance. Although we can use `@IntrinsicCandidate` to intrinsify it for potential? performance benefit, but I don't think it's necessary at present(and in future...), but I think they are good candidates of `@ForceInline` annotations. - PR: https://git.openjdk.java.net/jdk/pull/4595
Re: RFR: 6766844: ByteArrayInputStream#read with a byte array of length 0 not consistent with InputStream when at EOF [v2]
On Fri, 25 Jun 2021 01:39:21 GMT, Brian Burkhalter wrote: >> Modify `java.io.ByteArrayInputStream` methods `read(byte[])` and >> `read(byte[],int,int)` to return zero per the `InputStream` specification >> when the byte array actual or specified length is zero. > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 6766844: Add bug ID to test I had the same misgivings after creating this PR and thought that a CSR would be in order. - PR: https://git.openjdk.java.net/jdk/pull/4591
[jdk17] Withdrawn: JDK-8266310: deadlock between System.loadLibrary and JNI FindClass loading another class
On Fri, 18 Jun 2021 09:50:49 GMT, Aleksei Voitylov wrote: > Resubmitting the following PR https://github.com/openjdk/jdk/pull/3976 > against JDK17. > > This fixes the deadlock in ClassLoader between the two lock objects - a lock > object associated with the class being loaded, and the > ClassLoader.loadedLibraryNames hash map, locked during the native library > load operation. > > Further details can be found in the original PR. > > Testing: jtreg and jck testing with no regressions. A new regression test was > developed. This pull request has been closed without being integrated. - PR: https://git.openjdk.java.net/jdk17/pull/96
Re: [jdk17] RFR: JDK-8266310: deadlock between System.loadLibrary and JNI FindClass loading another class
On Fri, 18 Jun 2021 09:50:49 GMT, Aleksei Voitylov wrote: > Resubmitting the following PR https://github.com/openjdk/jdk/pull/3976 > against JDK17. > > This fixes the deadlock in ClassLoader between the two lock objects - a lock > object associated with the class being loaded, and the > ClassLoader.loadedLibraryNames hash map, locked during the native library > load operation. > > Further details can be found in the original PR. > > Testing: jtreg and jck testing with no regressions. A new regression test was > developed. OK, let it bake in 18 first. - PR: https://git.openjdk.java.net/jdk17/pull/96
Re: [jdk17] RFR: JDK-8268826: Cleanup Override in Context-Specific Deserialization Filters [v6]
On Fri, 25 Jun 2021 14:54:45 GMT, Roger Riggs wrote: >> Remove the unnecessary special case "OVERRIDE" in jdk.serialFilterFactory >> property. >> Fix description in the example of a filter allowing platform classes. >> Suppress some warnings about use of SecurityManager in tests. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Update logging of faults in jdk.serialFilterFactory to log only the > exception message > Simplify the logging.properties to only the needed settings Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk17/pull/85
Re: RFR: 8269383: (bf) ByteOrder should expose methods to test if platform is big or little endian
On Fri, 25 Jun 2021 13:30:56 GMT, Yi Yang wrote: > Hi, can I have a review of this change that adds two new utility methods for > java.nio.ByteOrder? Looking through the whole JDK codebase, most calls of > ByteOrder.nativeOrder() is to check if the underlying platform is > little-endian/big-endian(e.g. #4596 ). There is no reason to only provide > ByteOrder.nativeOrder while leaving big-endian/little-endian checking methods > blank. For most cases(in JDK or in user code), we want a checking(byte order) > rather than retrieving(byte order). > > Thanks! On the face of it, I do like this API enhancement. I've coded similar myself a number of times, well isBigEndian. Is there any other potential benefits, performance or otherwise, that than be achieved by such an API? - PR: https://git.openjdk.java.net/jdk/pull/4595
Re: [jdk17] RFR: JDK-8268826: Cleanup Override in Context-Specific Deserialization Filters [v6]
> Remove the unnecessary special case "OVERRIDE" in jdk.serialFilterFactory > property. > Fix description in the example of a filter allowing platform classes. > Suppress some warnings about use of SecurityManager in tests. Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: Update logging of faults in jdk.serialFilterFactory to log only the exception message Simplify the logging.properties to only the needed settings - Changes: - all: https://git.openjdk.java.net/jdk17/pull/85/files - new: https://git.openjdk.java.net/jdk17/pull/85/files/89df22e7..ca66a7b4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk17=85=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk17=85=04-05 Stats: 69 lines in 2 files changed: 0 ins; 65 del; 4 mod Patch: https://git.openjdk.java.net/jdk17/pull/85.diff Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/85/head:pull/85 PR: https://git.openjdk.java.net/jdk17/pull/85
Re: RFR: 8269383: (bf) ByteOrder should expose methods to test if platform is big or little endian
On Fri, 25 Jun 2021 13:30:56 GMT, Yi Yang wrote: > Hi, can I have a review of this change that adds two new utility methods for > java.nio.ByteOrder? Looking through the whole JDK codebase, most calls of > ByteOrder.nativeOrder() is to check if the underlying platform is > little-endian/big-endian(e.g. #4596 ). There is no reason to only provide > ByteOrder.nativeOrder while leaving big-endian/little-endian checking methods > blank. > > Thanks! If this is accepted as a new enhancement, it will need a CSR. Personally, I don't see enough justification for such a convenience method, but this isn't my area, so my opinion doesn't carry any weight. - PR: https://git.openjdk.java.net/jdk/pull/4595
Re: RFR: 8268469: Update java.time to use switch expressions [v6]
> Hi, > > Could someone please review my code for updating the code in the `java.time` > packages to make use of the switch expressions? > > Kind regards, > Patrick Patrick Concannon has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains eight additional commits since the last revision: - Merge remote-tracking branch 'origin/master' into JDK-8268469 - Merge remote-tracking branch 'origin/master' into JDK-8268469 - 8268469: Removed alignment of arrow operators in some cases; reverted logic of switch/case in LocalDate - Merge remote-tracking branch 'origin/master' into JDK-8268469 - 8268469: Removed excessive spacing; corrected misplaced comments - Merge remote-tracking branch 'origin/master' into JDK-8268469 - Merge remote-tracking branch 'origin/master' into JDK-8268469 - 8268469: Update java.time to use switch expressions - Changes: - all: https://git.openjdk.java.net/jdk/pull/4433/files - new: https://git.openjdk.java.net/jdk/pull/4433/files/a841c3cf..c8c1ab55 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4433=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4433=04-05 Stats: 7048 lines in 208 files changed: 1680 ins; 5063 del; 305 mod Patch: https://git.openjdk.java.net/jdk/pull/4433.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4433/head:pull/4433 PR: https://git.openjdk.java.net/jdk/pull/4433
Re: RFR: 8269383: New java.nio.ByteOrder.isBigEndian and isLittleEndian methods
Is this better than the current solution of `nativeOrder() == BIG_ENDIAN` other than reducing a few keystrokes? On Fri, Jun 25, 2021 at 8:45 AM Yi Yang wrote: > > Hi, can I have a review of this change that adds two new utility methods for > java.nio.ByteOrder? Looking through the whole JDK codebase, most calls of > ByteOrder.nativeOrder() is to check if the underlying platform is > little-endian/big-endian. There is no reason to only provide > ByteOrder.nativeOrder while leaving big-endian/little-endian checking methods > blank. > > Thanks! > > - > > Commit messages: > - add new methods > > Changes: https://git.openjdk.java.net/jdk/pull/4595/files > Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4595=00 > Issue: https://bugs.openjdk.java.net/browse/JDK-8269383 > Stats: 20 lines in 1 file changed: 20 ins; 0 del; 0 mod > Patch: https://git.openjdk.java.net/jdk/pull/4595.diff > Fetch: git fetch https://git.openjdk.java.net/jdk pull/4595/head:pull/4595 > > PR: https://git.openjdk.java.net/jdk/pull/4595 > -- - DML • he/him
RFR: 8269384: Determine native byte order for StringUTF16 via ByteOrder
Prefer using ByteOrder to compute byte order for StringUTF16 to determining byte order by native method StringUTF16.isBigEndian. - Commit messages: - replace Changes: https://git.openjdk.java.net/jdk/pull/4596/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4596=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8269384 Stats: 17 lines in 2 files changed: 3 ins; 13 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4596.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4596/head:pull/4596 PR: https://git.openjdk.java.net/jdk/pull/4596
RFR: 8269383: New java.nio.ByteOrder.isBigEndian and isLittleEndian methods
Hi, can I have a review of this change that adds two new utility methods for java.nio.ByteOrder? Looking through the whole JDK codebase, most calls of ByteOrder.nativeOrder() is to check if the underlying platform is little-endian/big-endian. There is no reason to only provide ByteOrder.nativeOrder while leaving big-endian/little-endian checking methods blank. Thanks! - Commit messages: - add new methods Changes: https://git.openjdk.java.net/jdk/pull/4595/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4595=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8269383 Stats: 20 lines in 1 file changed: 20 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4595.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4595/head:pull/4595 PR: https://git.openjdk.java.net/jdk/pull/4595
Re: [jdk17] RFR: 8269036: tools/jpackage/share/AppImagePackageTest.java failed with "hdiutil: create failed - Resource busy"
On Tue, 22 Jun 2021 21:59:43 GMT, Alexander Matveev wrote: > Looks like another "Resource busy" issue similar to recent fixes for "hdiutil > convert" and "hdiutil detach". Workaround in same way by repeating "create" > command. Modified RetryExecutor to pass write to file flag, otherwise > "hdiutil create" might deadlock. > Also, repeat is done only for creating DMG using size instead of creating > from app image. In some cases create DMG from app image might run for very > long time and eventually fail, this is why fallback to create DMG from size > was added. Marked as reviewed by herrick (Reviewer). - PR: https://git.openjdk.java.net/jdk17/pull/122
RFR: 8269373: some tests in jdk/tools/launcher/ fails on localized Windows platform
Hi all, Could you please review the 8269373 bug fixes? These tests call java.lang.ProcessBuilder in direct, so not used jtreg command option. To run non-localized tests, -Duser.language=en and -Duser.country=US options should be added in ProcessBuilder. - Commit messages: - 8269373: some tests in jdk/tools/launcher/ fails on localized Windows platform Changes: https://git.openjdk.java.net/jdk/pull/4594/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4594=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8269373 Stats: 14 lines in 6 files changed: 5 ins; 0 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/4594.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4594/head:pull/4594 PR: https://git.openjdk.java.net/jdk/pull/4594
Re: [jdk17] RFR: JDK-8268826: Cleanup Override in Context-Specific Deserialization Filters [v5]
On Thu, 24 Jun 2021 21:06:54 GMT, Roger Riggs wrote: >> Remove the unnecessary special case "OVERRIDE" in jdk.serialFilterFactory >> property. >> Fix description in the example of a filter allowing platform classes. >> Suppress some warnings about use of SecurityManager in tests. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Enabled logging of filter factory actions and added logging.properties > config file test/jdk/java/io/Serializable/serialFilter/logging.properties line 75: > 73: # messages: > 74: java.io.serialization.level = FINER > 75: duplicate settings? - PR: https://git.openjdk.java.net/jdk17/pull/85
Re: RFR: 8269124: Update java.time to use switch expressions (part II) [v5]
On Thu, 24 Jun 2021 12:01:04 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review the second half of my update for the `java.time` >> package to make use of switch expressions? >> >> This PR was split into two parts due to the large number of files affected. >> >> Kind regards, >> >> Patrick > > Patrick Concannon has updated the pull request incrementally with one > additional commit since the last revision: > > 8269124: Added instanceof pattern variables Marked as reviewed by aefimov (Committer). - PR: https://git.openjdk.java.net/jdk/pull/4552
Re: [jdk17] RFR: JDK-8266269: Lookup::accessClass fails with IAE when accessing an arrayClass with a protected inner class as component class
On Thu, 24 Jun 2021 18:42:23 GMT, Mandy Chung wrote: > `Lookup::accessClass` should determine the accessibility of the element type. > An array class is accessible if and only if its element type is accessible. > > This also fixes a spec bug to document `@throws NullPointerException` if the > argument is null. > > Please review the CSR: > https://bugs.openjdk.java.net/browse/JDK-8269312 Marked as reviewed by chegar (Reviewer). test/jdk/java/lang/invoke/t8150782/TestFindClass.java line 42: > 40: > 41: import org.testng.annotations.*; > 42: import p.Foo; maybe a stray import? - PR: https://git.openjdk.java.net/jdk17/pull/137
Re: [jdk17] RFR: JDK-8266310: deadlock between System.loadLibrary and JNI FindClass loading another class
> On 24 Jun 2021, at 22:27, Mandy Chung wrote: > > On Fri, 18 Jun 2021 09:50:49 GMT, Aleksei Voitylov > wrote: > >> Resubmitting the following PR https://github.com/openjdk/jdk/pull/3976 >> against JDK17. >> >> This fixes the deadlock in ClassLoader between the two lock objects - a lock >> object associated with the class being loaded, and the >> ClassLoader.loadedLibraryNames hash map, locked during the native library >> load operation. >> >> Further details can be found in the original PR. >> >> Testing: jtreg and jck testing with no regressions. A new regression test >> was developed. > > This is a risky area and I agree it needs some bake time. The fix has been > ready for some time but it takes longer than we hope to get this reviewed and > approved (I was one causing the delay). I am not uncomfortable getting this > in JDK 17 but I will not object if others think this should be fixed in JDK > 18 (and backport to 17 update if desirable) as this is a long standing issue > and no urgency to get this fixed. Fixing initially in 18, allowing some “bake” time, then considering a backport to a 17 update, seems prudent. -Chris.
Re: RFR: 6766844: ByteArrayInputStream#read with a byte array of length 0 not consistent with InputStream when at EOF [v2]
On Fri, 25 Jun 2021 01:39:21 GMT, Brian Burkhalter wrote: >> Modify `java.io.ByteArrayInputStream` methods `read(byte[])` and >> `read(byte[],int,int)` to return zero per the `InputStream` specification >> when the byte array actual or specified length is zero. > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 6766844: Add bug ID to test I realize I submitted this issue a long time ago but I think we have to proceed with caution as it is changing long standing behavior. Reading with len==0 will arise with code that is reading in a loop and the "previous read" completely fills the array and reads all remaining bytes. It's possible that there is existing code that will break with this change. I've added the "csr" label for now. If this changes goes ahead then it will need a release note too. - PR: https://git.openjdk.java.net/jdk/pull/4591
