Re: RFR: 8275811 Incorrect instance to dispose [v5]

2021-11-02 Thread Xue-Lei Andrew Fan
On Mon, 1 Nov 2021 20:38:32 GMT, Daniel Jeliński wrote: >> The current code that changes cipher suites disposes the new suite instead >> of the old one, which usually silently fails. This patch fixes the code to >> dispose the old instance instead. >> >> DTLS appears to be unaffected:

Re: RFR: 8275811 Incorrect instance to dispose [v5]

2021-11-02 Thread Xue-Lei Andrew Fan
On Mon, 1 Nov 2021 20:38:32 GMT, Daniel Jeliński wrote: >> The current code that changes cipher suites disposes the new suite instead >> of the old one, which usually silently fails. This patch fixes the code to >> dispose the old instance instead. >> >> DTLS appears to be unaffected:

Re: RFR: 8273026: Slow LoginContext.login() on multi threading application [v7]

2021-11-02 Thread Weijun Wang
On Tue, 2 Nov 2021 20:39:47 GMT, Florent Guillaume wrote: >> Larry-N has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Address review notes > > Could the original JDK-8230297 be closed as a duplicate please? @efge Closed. Thanks for

Re: RFR: 8271566: DSA signature length value is not accurate in P11Signature [v2]

2021-11-02 Thread Valerie Peng
On Fri, 13 Aug 2021 17:11:45 GMT, Martin Balao wrote: >> As described in JDK-8271566 [1], this patch proposal is intended to fix a >> problem that arises when using DSA keys that have a 256-bits (or larger) G >> parameter for signatures (either signing or verifying). There were some >>

Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v14]

2021-11-02 Thread Maurizio Cimadamore
> This PR contains the API and implementation changes for JEP-419 [1]. A more > detailed description of such changes, to avoid repetitions during the review > process, is included as a separate comment. > > [1] - https://openjdk.java.net/jeps/419 Maurizio Cimadamore has updated the pull

Re: RFR: 8274736: Concurrent read/close of SSLSockets causes SSLSessions to be invalidated unnecessarily [v3]

2021-11-02 Thread Xue-Lei Andrew Fan
On Tue, 2 Nov 2021 14:03:50 GMT, Jamil Nimeh wrote: >> This fix handles cases where two or more threads may be acting on a single >> SSLSocket instance. In cases where one thread closes the socket while the >> other is in the middle of a read, a SocketException is appropriately thrown, >>

Re: RFR: 8273026: Slow LoginContext.login() on multi threading application [v7]

2021-11-02 Thread Florent Guillaume
On Thu, 28 Oct 2021 18:55:32 GMT, Larry-N wrote: >> This fix adds a cache of service provider classes to LoginContext (in >> particular, it's a cache of LoginModules classes). The approach helps to >> increase the performance of the LoginContext.login() method significantly, >> especially in

Re: RFR: 8276348: Use blessed modifier order in java.base

2021-11-02 Thread Martin Buchholz
On Tue, 2 Nov 2021 19:14:23 GMT, Pavel Rappo wrote: >> Pragmatically, fix the script to ignore those keywords on comment lines. >> Learn Perl, its just a regular expression pattern match and replace >> expression. >> >> All of the changes have to be manually reviewed by the author and then

Re: RFR: 8276348: Use blessed modifier order in java.base

2021-11-02 Thread Martin Buchholz
On Tue, 2 Nov 2021 16:30:56 GMT, Pavel Rappo wrote: > This PR follows up one of the recent PRs, where I used a non-canonical > modifier order. Since the problem was noticed [^1], why not to address it at > mass? > > As far as I remember, the first mass-canonicalization of modifiers took place

Re: RFR: 8276348: Use blessed modifier order in java.base

2021-11-02 Thread Pavel Rappo
On Tue, 2 Nov 2021 19:22:15 GMT, Pavel Rappo wrote: >> src/java.base/share/classes/java/lang/invoke/CallSite.java line 88: >> >>> 86: */ >>> 87: public >>> 88: abstract class CallSite { >> >> I think it's better to move all modifiers to the single line. > > This is a survivorship bias. This

Re: RFR: 8276348: Use blessed modifier order in java.base

2021-11-02 Thread Roger Riggs
On Tue, 2 Nov 2021 16:30:56 GMT, Pavel Rappo wrote: > This PR follows up one of the recent PRs, where I used a non-canonical > modifier order. Since the problem was noticed [^1], why not to address it at > mass? > > As far as I remember, the first mass-canonicalization of modifiers took place

Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v12]

2021-11-02 Thread Maurizio Cimadamore
On Tue, 2 Nov 2021 18:48:57 GMT, Maurizio Cimadamore wrote: >> src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/ArenaAllocator.java >> line 88: >> >>> 86: if (size > arenaSize) { >>> 87: throw new OutOfMemoryError(); >>> 88:

Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v12]

2021-11-02 Thread Maurizio Cimadamore
On Tue, 2 Nov 2021 18:55:47 GMT, Maurizio Cimadamore wrote: >> I'll have to think some more about this. I don't think this is covered >> inside the block - that is, the block tries to allocate, and then in the >> finally we throw if we realized we've allocated too much. > > What is missing, I

Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v12]

2021-11-02 Thread Jorn Vernee
On Tue, 2 Nov 2021 19:02:51 GMT, Maurizio Cimadamore wrote: >> What is missing, I think, is a check (size > arenaSize) at the beginning of >> the method (we only check this in one of the paths). But we need to check >> before and after, I think, as it is possible to allocate a segment and

Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v13]

2021-11-02 Thread Maurizio Cimadamore
> This PR contains the API and implementation changes for JEP-419 [1]. A more > detailed description of such changes, to avoid repetitions during the review > process, is included as a separate comment. > > [1] - https://openjdk.java.net/jeps/419 Maurizio Cimadamore has updated the pull

Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v13]

2021-11-02 Thread Alan Bateman
On Tue, 2 Nov 2021 19:35:29 GMT, Maurizio Cimadamore wrote: >> This PR contains the API and implementation changes for JEP-419 [1]. A more >> detailed description of such changes, to avoid repetitions during the review >> process, is included as a separate comment. >> >> [1] -

Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v12]

2021-11-02 Thread Maurizio Cimadamore
On Tue, 2 Nov 2021 16:51:06 GMT, Jorn Vernee wrote: >> Maurizio Cimadamore has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Tweak javadoc of loaderLookup > >

Re: RFR: 8276348: Use blessed modifier order in java.base

2021-11-02 Thread Pavel Rappo
On Tue, 2 Nov 2021 19:15:26 GMT, Andrey Turbanov wrote: >> This PR follows up one of the recent PRs, where I used a non-canonical >> modifier order. Since the problem was noticed [^1], why not to address it at >> mass? >> >> As far as I remember, the first mass-canonicalization of modifiers

Re: RFR: 8276348: Use blessed modifier order in java.base

2021-11-02 Thread Andrey Turbanov
On Tue, 2 Nov 2021 16:30:56 GMT, Pavel Rappo wrote: > This PR follows up one of the recent PRs, where I used a non-canonical > modifier order. Since the problem was noticed [^1], why not to address it at > mass? > > As far as I remember, the first mass-canonicalization of modifiers took place

Re: RFR: 8276348: Use blessed modifier order in java.base

2021-11-02 Thread Pavel Rappo
On Tue, 2 Nov 2021 18:48:20 GMT, Roger Riggs wrote: > Pragmatically, fix the script to ignore those keywords on comment lines. > Learn Perl, its just a regular expression pattern match and replace > expression. I understand in principle how to modify that script to ignore doc comments. The

Re: RFR: 8276348: Use blessed modifier order in java.base

2021-11-02 Thread Mark Sheppard
On Tue, 2 Nov 2021 18:17:36 GMT, Pavel Rappo wrote: >> It's tough when a natural language clashes with a programming language. I >> appreciate that this particular clash might cause discomfort to native >> English speakers. (This reminds me of that _DOSASCOMP_ mnemonic for >> adjective

Re: RFR: 8276348: Use blessed modifier order in java.base

2021-11-02 Thread Roger Riggs
On Tue, 2 Nov 2021 18:48:06 GMT, Mark Sheppard wrote: >> Here's a bit of archaeology. I found the original JDK-8136583 patch, which >> has moved from where it was in the RFR to >> https://cr.openjdk.java.net/~martin/webrevs/jdk9/blessed-modifier-order/. >> That patch doesn't change

Re: previously prevented exploit now possible with JDK 18

2021-11-02 Thread Sean Mullan
Hello Rick, It is behaving as expected. Let me explain in more detail. First, loading a signed JAR off the classpath only verifies the signature and digests of the JAR file. It does not validate the signer's certificate chain or determine if the signer is trusted. The JarFile API class

Re: RFR: 8276348: Use blessed modifier order in java.base

2021-11-02 Thread Phil Race
On Tue, 2 Nov 2021 16:30:56 GMT, Pavel Rappo wrote: > This PR follows up one of the recent PRs, where I used a non-canonical > modifier order. Since the problem was noticed [^1], why not to address it at > mass? > > As far as I remember, the first mass-canonicalization of modifiers took place

Re: RFR: 8276348: Use blessed modifier order in java.base

2021-11-02 Thread Pavel Rappo
On Tue, 2 Nov 2021 17:45:07 GMT, Pavel Rappo wrote: >>> In comments, I think the 'synchronized static 'reads better, the >>> conventional order is for the code. >> >> I think Roger is right and maybe the change to the javadoc could be dropped >> from this patch. > > It's tough when a natural

Re: RFR: 8276348: Use blessed modifier order in java.base

2021-11-02 Thread Pavel Rappo
On Tue, 2 Nov 2021 17:39:17 GMT, Alan Bateman wrote: >> src/java.base/share/classes/java/lang/Object.java line 282: >> >>> 280: * For objects of type {@code Class,} by executing a >>> 281: * static synchronized method of that class. >>> 282: * >> >> In comments, I think the

Re: RFR: 8276348: Use blessed modifier order in java.base

2021-11-02 Thread Alan Bateman
On Tue, 2 Nov 2021 17:13:47 GMT, Roger Riggs wrote: > In comments, I think the 'synchronized static 'reads better, the conventional > order is for the code. I think Roger is right and maybe the change to the javadoc could be dropped from this patch. - PR:

Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v10]

2021-11-02 Thread Jorn Vernee
On Mon, 1 Nov 2021 15:38:18 GMT, Jorn Vernee wrote: >> Maurizio Cimadamore has updated the pull request with a new target base due >> to a merge or a rebase. The pull request now contains 17 commits: >> >> - Add cache for memory address var handles >> - Merge branch 'master' into JEP-419 >>

Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v10]

2021-11-02 Thread Jorn Vernee
On Mon, 1 Nov 2021 12:05:32 GMT, Maurizio Cimadamore wrote: >> This PR contains the API and implementation changes for JEP-419 [1]. A more >> detailed description of such changes, to avoid repetitions during the review >> process, is included as a separate comment. >> >> [1] -

Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v12]

2021-11-02 Thread Jorn Vernee
On Mon, 1 Nov 2021 22:36:40 GMT, Maurizio Cimadamore wrote: >> This PR contains the API and implementation changes for JEP-419 [1]. A more >> detailed description of such changes, to avoid repetitions during the review >> process, is included as a separate comment. >> >> [1] -

Re: RFR: 8276348: Use blessed modifier order in java.base

2021-11-02 Thread Iris Clark
On Tue, 2 Nov 2021 16:30:56 GMT, Pavel Rappo wrote: > This PR follows up one of the recent PRs, where I used a non-canonical > modifier order. Since the problem was noticed [^1], why not to address it at > mass? > > As far as I remember, the first mass-canonicalization of modifiers took place

Re: RFR: 8276348: Use blessed modifier order in java.base

2021-11-02 Thread Joe Darcy
On Tue, 2 Nov 2021 16:30:56 GMT, Pavel Rappo wrote: > This PR follows up one of the recent PRs, where I used a non-canonical > modifier order. Since the problem was noticed [^1], why not to address it at > mass? > > As far as I remember, the first mass-canonicalization of modifiers took place

Re: RFR: 8276348: Use blessed modifier order in java.base

2021-11-02 Thread Roger Riggs
On Tue, 2 Nov 2021 16:30:56 GMT, Pavel Rappo wrote: > This PR follows up one of the recent PRs, where I used a non-canonical > modifier order. Since the problem was noticed [^1], why not to address it at > mass? > > As far as I remember, the first mass-canonicalization of modifiers took place

Re: RFR: 8276348: Use blessed modifier order in java.base

2021-11-02 Thread Daniel Fuchs
On Tue, 2 Nov 2021 16:30:56 GMT, Pavel Rappo wrote: > This PR follows up one of the recent PRs, where I used a non-canonical > modifier order. Since the problem was noticed [^1], why not to address it at > mass? > > As far as I remember, the first mass-canonicalization of modifiers took place

Re: RFR: 8276348: Use blessed modifier order in java.base

2021-11-02 Thread Pavel Rappo
On Tue, 2 Nov 2021 16:30:56 GMT, Pavel Rappo wrote: > This PR follows up one of the recent PRs, where I used a non-canonical > modifier order. Since the problem was noticed [^1], why not to address it at > mass? > > As far as I remember, the first mass-canonicalization of modifiers took place

RFR: 8276348: Use blessed modifier order in java.base

2021-11-02 Thread Pavel Rappo
This PR follows up one of the recent PRs, where I used a non-canonical modifier order. Since the problem was noticed [^1], why not to address it at mass? As far as I remember, the first mass-canonicalization of modifiers took place in JDK-8136583 in 2015 [^2]. That change affected 1780 lines

Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v10]

2021-11-02 Thread Paul Sandoz
On Tue, 2 Nov 2021 10:30:42 GMT, Maurizio Cimadamore wrote: >> src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/Utils.java line >> 111: >> >>> 109: class VarHandleCache { >>> 110: private static final Map handleMap >>> = new ConcurrentHashMap<>(); >>> 111:

Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v12]

2021-11-02 Thread Paul Sandoz
On Mon, 1 Nov 2021 22:36:40 GMT, Maurizio Cimadamore wrote: >> This PR contains the API and implementation changes for JEP-419 [1]. A more >> detailed description of such changes, to avoid repetitions during the review >> process, is included as a separate comment. >> >> [1] -

Re: RFR: 8225181: KeyStore should have a getAttributes method [v2]

2021-11-02 Thread Sean Mullan
On Thu, 28 Oct 2021 16:58:47 GMT, Weijun Wang wrote: >> Add `KeyStore::getAttributes` so that one can get the attributes of an entry >> without retrieving the entry first. This is especially useful for a private >> key entry which can only be retrieved with a password. > > Weijun Wang has

Re: RFR: 8274736: Concurrent read/close of SSLSockets causes SSLSessions to be invalidated unnecessarily [v3]

2021-11-02 Thread Jamil Nimeh
> This fix handles cases where two or more threads may be acting on a single > SSLSocket instance. In cases where one thread closes the socket while the > other is in the middle of a read, a SocketException is appropriately thrown, > but it results in SSLSession invalidation even though the

Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v10]

2021-11-02 Thread Maurizio Cimadamore
On Tue, 2 Nov 2021 00:24:12 GMT, Paul Sandoz wrote: >> Maurizio Cimadamore has updated the pull request with a new target base due >> to a merge or a rebase. The pull request now contains 17 commits: >> >> - Add cache for memory address var handles >> - Merge branch 'master' into JEP-419 >>

Re: RFR: 8275811 Incorrect instance to dispose

2021-11-02 Thread Daniel Jeliński
On Sun, 24 Oct 2021 04:58:45 GMT, Xue-Lei Andrew Fan wrote: >> Did you want to cover the update for line 222 at OutputRecord.java as well? > >> After reviewing the scope of changes to fix writeCipher disposal I decided >> to remove it entirely. It would probably be a nice follow-up enhancement,