Integrated: 8287390: Cleanup Map usage in AuthenticationInfo.requestAuthentication
On Sat, 30 Apr 2022 10:17:43 GMT, Andrey Turbanov wrote: > `AuthenticationInfo.requestAuthentication` uses separate `HashMap`'s `get` > +`put` calls. > > https://github.com/openjdk/jdk/blob/176bb23de18d9ab448e77e85a9c965a7c02f2c50/src/java.base/share/classes/sun/net/www/protocol/http/AuthenticationInfo.java#L155-L165 > > Instead we can use the `HashMap.putIfAbsent` to make code a bit easier to > follow. We know that `requests` can contain only non-null values. This pull request has now been integrated. Changeset: 4caf1ef3 Author:Andrey Turbanov URL: https://git.openjdk.java.net/jdk/commit/4caf1ef389fd02bf53a9b7ed33d3b57fdaa79bd2 Stats: 12 lines in 1 file changed: 0 ins; 6 del; 6 mod 8287390: Cleanup Map usage in AuthenticationInfo.requestAuthentication Reviewed-by: dfuchs, jpai - PR: https://git.openjdk.java.net/jdk/pull/8484
Re: RFR: 8287390: Cleanup Map usage in AuthenticationInfo.requestAuthentication [v2]
> `AuthenticationInfo.requestAuthentication` uses separate `HashMap`'s `get` > +`put` calls. > > https://github.com/openjdk/jdk/blob/176bb23de18d9ab448e77e85a9c965a7c02f2c50/src/java.base/share/classes/sun/net/www/protocol/http/AuthenticationInfo.java#L155-L165 > > Instead we can use the `HashMap.putIfAbsent` to make code a bit easier to > follow. We know that `requests` can contain only non-null values. Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision: 8287390: Cleanup Map usage in AuthenticationInfo.requestAuthentication remove obvious assert - Changes: - all: https://git.openjdk.java.net/jdk/pull/8484/files - new: https://git.openjdk.java.net/jdk/pull/8484/files/b493aab1..f850d61f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8484&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8484&range=00-01 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8484.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8484/head:pull/8484 PR: https://git.openjdk.java.net/jdk/pull/8484
Re: RFR: 8287390: Cleanup Map usage in AuthenticationInfo.requestAuthentication [v2]
On Wed, 1 Jun 2022 04:08:53 GMT, Jaikiran Pai wrote:
>> Andrey Turbanov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8287390: Cleanup Map usage in AuthenticationInfo.requestAuthentication
>> remove obvious assert
>
> src/java.base/share/classes/sun/net/www/protocol/http/AuthenticationInfo.java
> line 159:
>
>> 157: if (t == null || t == c) {
>> 158: assert cached == null;
>> 159: return cached;
>
> Hello Andrey, while you are in this code, I think changing these 2 lines:
>
>
> assert cached == null;
> return cached;
>
> to just:
>
>
> return null;
>
> would be better. There's already a `if (cached != null) return cached;` code,
> a few lines above and after that line there's no other modifications to this
> `cached` local variable, so changing this line to just return null would
> remove any confusion while reading this code.
Good idea. Updated.
-
PR: https://git.openjdk.java.net/jdk/pull/8484
RFR: 8287390: Cleanup Map usage in AuthenticationInfo.requestAuthentication
`AuthenticationInfo.requestAuthentication` uses separate `HashMap`'s `get`
+`put` calls.
Thread t, c;
c = Thread.currentThread();
if ((t = requests.get(key)) == null) {
requests.put (key, c);
assert cached == null;
return cached;
}
if (t == c) {
assert cached == null;
return cached;
}
Instead we can use the `HashMap.putIfAbsent` to make code a bit easier to
follow. We know that `requests` can contain only non-null values.
-
Commit messages:
- [PATCH] Cleanup Map usage in AuthenticationInfo.requestAuthentication
Changes: https://git.openjdk.java.net/jdk/pull/8484/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8484&range=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8287390
Stats: 10 lines in 1 file changed: 0 ins; 5 del; 5 mod
Patch: https://git.openjdk.java.net/jdk/pull/8484.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/8484/head:pull/8484
PR: https://git.openjdk.java.net/jdk/pull/8484
Re: RFR: 8284893: Fix typos in java.base
On Thu, 14 Apr 2022 19:07:09 GMT, Magnus Ihse Bursie wrote: > I ran `codespell` on the `src/java.base` directory, and accepted those > changes where it indeed discovered real typos. > > (Due to false positives this can unfortunately not be run automatically) > > The majority of fixes are in comments. A handful is in strings, one in a > local variable name, and a couple in parameter declarations. > > Annoyingly, there are several instances of "childs" (instead of "children") > in the source code, but they were not local and I dared not change them. > Someone braver than me might take a stab at it, perhaps.. Shouldn't copyright year be updated too? - PR: https://git.openjdk.java.net/jdk/pull/8250
Integrated: 8284567: Collapse identical catch branches in java.base
On Sat, 2 Apr 2022 16:05:06 GMT, Andrey Turbanov wrote: > Let's take advantage of Java 7 language feature - "Catching Multiple > Exception Types". > It simplifies code. Reduces duplication. > Found by IntelliJ IDEA inspection Identical 'catch' branches in 'try' > statement This pull request has now been integrated. Changeset: f4edb59a Author:Andrey Turbanov URL: https://git.openjdk.java.net/jdk/commit/f4edb59a6e44d99ba215ee6970ffa6fb26b4798c Stats: 106 lines in 17 files changed: 0 ins; 60 del; 46 mod 8284567: Collapse identical catch branches in java.base Reviewed-by: darcy, iris, wetmore - PR: https://git.openjdk.java.net/jdk/pull/8081
RFR: 8284567: Collapse identical catch branches in java.base
Let's take advantage of Java 7 language feature - "Catching Multiple Exception Types". It simplifies code. Reduces duplication. Found by IntelliJ IDEA inspection Identical 'catch' branches in 'try' statement - Commit messages: - Merge remote-tracking branch 'origin/master' into collapse_identical_catch_java.base - [PATCH] Collapse identical catch branches in java.base - [PATCH] Collapse identical catch branches in java.base Changes: https://git.openjdk.java.net/jdk/pull/8081/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8081&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8284567 Stats: 106 lines in 17 files changed: 0 ins; 60 del; 46 mod Patch: https://git.openjdk.java.net/jdk/pull/8081.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8081/head:pull/8081 PR: https://git.openjdk.java.net/jdk/pull/8081
Confusing startLock.wait()/finishLock.wait() calls in WindowsSelectorImpl
Hello.
I found confusing calls to Object.wait() in 2 methods:
1. sun.nio.ch.WindowsSelectorImpl.StartLock#waitForStart
https://github.com/openjdk/jdk/blob/4df67426ed02f18af0757897acb28b636a317a91/src/java.base/windows/classes/sun/nio/ch/WindowsSelectorImpl.java#L252
2. sun.nio.ch.WindowsSelectorImpl.FinishLock#waitForHelperThreads
https://github.com/openjdk/jdk/blob/4df67426ed02f18af0757897acb28b636a317a91/src/java.base/windows/classes/sun/nio/ch/WindowsSelectorImpl.java#L304
Methods 'sun.nio.ch.WindowsSelectorImpl.FinishLock#waitForHelperThreads'
and 'sun.nio.ch.WindowsSelectorImpl.StartLock#waitForStart'
are synchronized. It means they are synchronized on 'this'.
But, somewhy, this methods calls to 'wait()' via reference fields in
outer class:
private synchronized boolean waitForStart(SelectThread thread) {
...
startLock.wait();
private synchronized void waitForHelperThreads() {
...
finishLock.wait();
It seems confusing to me. I would expect that method 'wait()' to be
called directly on 'this' too.
For StartLock it even allows to mark it as a 'static' nested class
(minus one redundant field).
Is it intentional to call 'wait()' like this?
Andrey Turbanov
Integrated: 8278263: Remove redundant synchronized from URLStreamHandler.openConnection methods
On Fri, 12 Nov 2021 19:35:10 GMT, Andrey Turbanov wrote: > All this Handler's are stateless and there is nothing to protect via > synchronization. This pull request has now been integrated. Changeset: c6ed2046 Author:Andrey Turbanov URL: https://git.openjdk.java.net/jdk/commit/c6ed2046b4ba8eafb6b7e934b134829760d56ecd Stats: 31 lines in 3 files changed: 0 ins; 22 del; 9 mod 8278263: Remove redundant synchronized from URLStreamHandler.openConnection methods Reviewed-by: dfuchs - PR: https://git.openjdk.java.net/jdk/pull/6373
Integrated: 8277412: Use String.isBlank to simplify code in sun.net.www.protocol.mailto.Handler
On Fri, 12 Nov 2021 19:11:36 GMT, Andrey Turbanov wrote: > All this manually written code actually can be replaced with single > String.isBlank() call. > `file` variable is guaranteed to be non-null. This pull request has now been integrated. Changeset: 268880b4 Author: Andrey Turbanov URL: https://git.openjdk.java.net/jdk/commit/268880b471eed54535927fba953347160f447fcd Stats: 26 lines in 1 file changed: 0 ins; 21 del; 5 mod 8277412: Use String.isBlank to simplify code in sun.net.www.protocol.mailto.Handler Reviewed-by: dfuchs - PR: https://git.openjdk.java.net/jdk/pull/6372
Integrated: 8277120: Use Optional.isEmpty instead of !Optional.isPresent in java.net.http
On Mon, 18 Oct 2021 07:55:52 GMT, Andrey Turbanov wrote: > I propose to replace usages of !Optional.isPresent() with Optional.isEmpty > method. > It's makes code a bit easier to read. This pull request has now been integrated. Changeset: 47b1c51b Author:Andrey Turbanov URL: https://git.openjdk.java.net/jdk/commit/47b1c51bbd28582d209db07052e553a76acced65 Stats: 11 lines in 5 files changed: 0 ins; 2 del; 9 mod 8277120: Use Optional.isEmpty instead of !Optional.isPresent in java.net.http Reviewed-by: dfuchs - PR: https://git.openjdk.java.net/jdk/pull/5985
Integrated: 8280010: Remove double buffering of InputStream for Properties.load
On Mon, 10 Jan 2022 20:46:36 GMT, Andrey Turbanov wrote: > `Properties.load` uses `java.util.Properties.LineReader`. LineReader already > buffers input stream. Hence wrapping InputStream in BufferedInputStream is > redundant. This pull request has now been integrated. Changeset: 9eb50a5e Author: Andrey Turbanov URL: https://git.openjdk.java.net/jdk/commit/9eb50a5ee4a069fbb248748ebee09132e2450420 Stats: 30 lines in 8 files changed: 0 ins; 11 del; 19 mod 8280010: Remove double buffering of InputStream for Properties.load Reviewed-by: amenkov, sspitsyn, serb - PR: https://git.openjdk.java.net/jdk/pull/7021
Re: RFR: 8280010: Remove double buffering of InputStream for Properties.load
On Mon, 10 Jan 2022 20:46:36 GMT, Andrey Turbanov wrote:
> `Properties.load` uses `java.util.Properties.LineReader`. LineReader already
> buffers input stream. Hence wrapping InputStream in BufferedInputStream is
> redundant.
Checked. `BufferedInputStream` add a bit of overhead.
Benchmark
@BenchmarkMode(value = Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@Warmup(iterations = 6, time = 1, timeUnit = TimeUnit.SECONDS)
@Measurement(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
@Fork(1)
@State(Scope.Benchmark)
public class PropertiesLoad {
Properties properties;
private InputStream arrayInputStream;
private InputStream fileInputStream;
private Path filePath;
@Setup
public void setupStrings() throws IOException {
properties = new Properties();
String content = """
currentVersion=IdealGraphVisualizer {0}
LBL_splash_window_title=Starting IdealGraphVisualizer
SPLASH_WIDTH=475
SplashProgressBarBounds=0,273,475,6
SplashProgressBarColor=0xFF
SplashRunningTextBounds=10,283,460,12
SplashRunningTextColor=0xFF
""";
filePath = Files.createTempFile("benchmark", ".properties");
Files.writeString(filePath, content);
arrayInputStream = new
ByteArrayInputStream(content.getBytes(StandardCharsets.UTF_8));
fileInputStream = Files.newInputStream(filePath);
}
@TearDown
public void tearDown() throws IOException {
fileInputStream.close();
Files.delete(filePath);
}
@Benchmark
public Properties plain_ByteStream() throws IOException {
properties.load(arrayInputStream);
return properties;
}
@Benchmark
public Properties plain_fileStream() throws IOException {
properties.load(fileInputStream);
return properties;
}
@Benchmark
public Properties buffered_ByteStream() throws IOException {
properties.load(new BufferedInputStream(arrayInputStream));
return properties;
}
@Benchmark
public Properties buffered_fileStream() throws IOException {
properties.load(new BufferedInputStream(fileInputStream));
return properties;
}
public static void main(String[] args) throws RunnerException {
Options opt = new OptionsBuilder()
.include(PropertiesLoad.class.getSimpleName())
.build();
new Runner(opt).run();
}
}
Results:
Benchmark Mode Cnt Score Error Units
PropertiesLoad.buffered_ByteStream avgt5 2416,364 ± 46,209 ns/op
PropertiesLoad.buffered_fileStream avgt5 4276,015 ± 139,094 ns/op
PropertiesLoad.plain_ByteStream avgt5 1445,864 ± 649,779 ns/op
PropertiesLoad.plain_fileStream avgt5 3183,012 ± 198,974 ns/op
-
PR: https://git.openjdk.java.net/jdk/pull/7021
RFR: 8280010: Remove double buffering of InputStream for Properties.load
`Properties.load` uses `java.util.Properties.LineReader`. LineReader already buffers input stream. Hence wrapping InputStream in BufferedInputStream is redundant. - Commit messages: - [PATCH] Remove double buffering of InputStream for Properties.load - [PATCH] Remove double buffering of InputStream for Properties.load Changes: https://git.openjdk.java.net/jdk/pull/7021/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7021&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8280010 Stats: 30 lines in 8 files changed: 0 ins; 11 del; 19 mod Patch: https://git.openjdk.java.net/jdk/pull/7021.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7021/head:pull/7021 PR: https://git.openjdk.java.net/jdk/pull/7021
Integrated: 8274809: Update java.base classes to use try-with-resources
On Tue, 5 Oct 2021 09:36:23 GMT, Andrey Turbanov wrote: > 8274809: Update java.base classes to use try-with-resources This pull request has now been integrated. Changeset: dee447f8 Author: Andrey Turbanov Committer: Daniel Fuchs URL: https://git.openjdk.java.net/jdk/commit/dee447f8ae788c6c1f6cd1e1fcb93faceab37b6c Stats: 60 lines in 7 files changed: 2 ins; 40 del; 18 mod 8274809: Update java.base classes to use try-with-resources Reviewed-by: mullan, alanb, dfuchs - PR: https://git.openjdk.java.net/jdk/pull/5818
Re: RFR: 8274809: Update java.base classes to use try-with-resources [v4]
On Fri, 10 Dec 2021 18:18:02 GMT, Andrey Turbanov wrote: >> This is legacy code with probably poor testing coverage. Usually we don't >> print anything on the console directly and unconditionally - except in some >> well identified cases. Ignoring exceptions thrown by `close` is a common >> idiom. That said - it does seem that this method is indeed never called - so >> for the sake of simplification I'd agree to keep this change as proposed. >> Remind me to sponsor this after the fork. > >>Remind me to sponsor this after the fork. > > fork is done :) Looks like `/sponsor` in comment is not considered as a command by bot - PR: https://git.openjdk.java.net/jdk/pull/5818
Re: RFR: 8274809: Update java.base classes to use try-with-resources [v4]
On Thu, 9 Dec 2021 01:25:20 GMT, Bradford Wetmore wrote: >> That's one of the places where I personally would always use the var keyword >> too: it makes the line shorter and the type is already clearly visible in >> the RHS of the assignment, so repeating it on the left hand side does not >> bring much value... > > Just interesting that the style wasn't consistent. Both ways are fine by me. I used `var` only in places, where it allows to reduce line length to be less than 80. - PR: https://git.openjdk.java.net/jdk/pull/5818
Re: RFR: 8274809: Update java.base classes to use try-with-resources [v4]
On Tue, 7 Dec 2021 14:48:17 GMT, Daniel Fuchs wrote: >Remind me to sponsor this after the fork. fork is done :) - PR: https://git.openjdk.java.net/jdk/pull/5818
Re: RFR: 8274809: Update java.base classes to use try-with-resources [v4]
On Tue, 7 Dec 2021 07:59:54 GMT, Alan Bateman wrote:
>> Andrey Turbanov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8274809: Update java.base classes to use try-with-resources
>> fix review comments
>
> src/java.base/share/classes/sun/net/NetProperties.java line 73:
>
>> 71: try (FileInputStream in = new FileInputStream(fname);
>> 72: BufferedInputStream bin = new BufferedInputStream(in))
>> 73: {
>
> Style-wisee I'd probably put the "{" at the end of L72.
As you wish.
But I personally like to have `{` on separate line, in case of multi-line
`try`. It makes clear when `try` header is actually finished and body starts.
-
PR: https://git.openjdk.java.net/jdk/pull/5818
Re: RFR: 8274809: Update java.base classes to use try-with-resources [v5]
> 8274809: Update java.base classes to use try-with-resources
Andrey Turbanov has updated the pull request incrementally with one additional
commit since the last revision:
8274809: Update java.base classes to use try-with-resources
move { to previous line
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/5818/files
- new: https://git.openjdk.java.net/jdk/pull/5818/files/3b05ec49..033ee3c2
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5818&range=04
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5818&range=03-04
Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod
Patch: https://git.openjdk.java.net/jdk/pull/5818.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/5818/head:pull/5818
PR: https://git.openjdk.java.net/jdk/pull/5818
Re: RFR: 8274809: Update java.base classes to use try-with-resources [v4]
On Tue, 7 Dec 2021 11:41:12 GMT, Daniel Fuchs wrote:
>> Andrey Turbanov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8274809: Update java.base classes to use try-with-resources
>> fix review comments
>
> src/java.base/share/classes/sun/net/www/MimeTable.java line 385:
>
>> 383:
>> 384: protected boolean saveAsProperties(File file) {
>> 385: try (FileOutputStream os = new FileOutputStream(file)) {
>
> This is not strictly equivalent as now an exception thrown during `close()`
> will be printed instead of being ignored. I have no idea whether it's a good
> thing or a bad thing, but that's a _different_ thing...
> So unless someone with more historical knowledge of this area can comment,
> maybe this change should be reverted.
I believe it's always good to have at least _some_ trace if we got Exception.
If closing FileOutputStream failed - there is no guarantee, that all written
content is actually stored to filesystem.
BTW, this method `saveAsProperties` seems unused...
-
PR: https://git.openjdk.java.net/jdk/pull/5818
Re: RFR: 8277412: Use String.isBlank to simplify code in sun.net.www.protocol.mailto.Handler
On Tue, 7 Dec 2021 08:07:22 GMT, Alan Bateman wrote: >> All this manually written code actually can be replaced with single >> String.isBlank() call. >> `file` variable is guaranteed to be non-null. > > src/java.base/share/classes/sun/net/www/protocol/mailto/Handler.java line 125: > >> 123: * Let's just make sure we DO have an Email address in the URL. >> 124: */ >> 125: if (file.isBlank()) > > The mail protocol handler is a legacy protocol handler and I don't know if > there is good test coverage. The change proposed here changes the exception > from RuntimeException to NPE when there isn't a file component in the URL. > Neither is specified by URLStreamHandler.parseURL so I suspect there may be > follow-on issues to clarify the spec on several points. `file` can't be `null` here. It's either empty string, or result of String.substring (which is also guarantee to be non-null). Check code before this line: https://github.com/openjdk/jdk/blob/07669e3bc65b1728d784e21ec83b437374f9fa19/src/java.base/share/classes/sun/net/www/protocol/mailto/Handler.java#L121-L125 IDEA confirms that  - PR: https://git.openjdk.java.net/jdk/pull/6372
RFR: 8278263: Remove redundant synchronized from URLStreamHandler.openConnection methods
All this Handler's are stateless and there is nothing to protect via synchronization. - Commit messages: - [PATCH] Remove redundant synchronized from URLStreamHandler.openConnection methods Changes: https://git.openjdk.java.net/jdk/pull/6373/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6373&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8278263 Stats: 36 lines in 3 files changed: 0 ins; 26 del; 10 mod Patch: https://git.openjdk.java.net/jdk/pull/6373.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6373/head:pull/6373 PR: https://git.openjdk.java.net/jdk/pull/6373
Integrated: 8274949: Use String.contains() instead of String.indexOf() in java.base
On Fri, 17 Sep 2021 08:56:47 GMT, Andrey Turbanov wrote: > String.contains was introduced in Java 5. > Some code in java.base still uses old approach with `String.indexOf` to check > if String contains specified substring. > I propose to migrate such usages. Makes code shorter and easier to read. This pull request has now been integrated. Changeset: 66775543 Author:Andrey Turbanov Committer: Roger Riggs URL: https://git.openjdk.java.net/jdk/commit/6677554374ade32c9f2ddaaa093064de8aebd831 Stats: 38 lines in 17 files changed: 0 ins; 3 del; 35 mod 8274949: Use String.contains() instead of String.indexOf() in java.base Reviewed-by: weijun, dfuchs, vtewari, lancea - PR: https://git.openjdk.java.net/jdk/pull/5559
RFR: 8277412: Use String.isBlank to simplify code in sun.net.www.protocol.mailto.Handler
All this manually written code actually can be replaced with single String.isBlank() call. `file` variable is guaranteed to be non-null. - Commit messages: - [PATCH] Use String.isBlank to simplify code Changes: https://git.openjdk.java.net/jdk/pull/6372/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6372&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8277412 Stats: 26 lines in 1 file changed: 0 ins; 21 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/6372.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6372/head:pull/6372 PR: https://git.openjdk.java.net/jdk/pull/6372
RFR: 8277120: Use Optional.isEmpty instead of !Optional.isPresent in java.net.http
I propose to replace usages of !Optional.isPresent() with Optional.isEmpty method. It's makes code a bit easier to read. - Commit messages: - [PATCH] Use Optional.isEmpty instead of !Optional.isPresent in java.net.http Changes: https://git.openjdk.java.net/jdk/pull/5985/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5985&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8277120 Stats: 13 lines in 5 files changed: 0 ins; 3 del; 10 mod Patch: https://git.openjdk.java.net/jdk/pull/5985.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5985/head:pull/5985 PR: https://git.openjdk.java.net/jdk/pull/5985
Integrated: 8274835: Remove unnecessary castings in java.base
On Thu, 9 Sep 2021 20:12:47 GMT, Andrey Turbanov wrote: > Redundant castings make code harder to read. > Found them by IntelliJ IDEA. > I tried to select only casts which are definitely safe to remove. Also didn't > touch primitive types casts. This pull request has now been integrated. Changeset: 5a2452c8 Author: Andrey Turbanov Committer: Daniel Fuchs URL: https://git.openjdk.java.net/jdk/commit/5a2452c80e64b8b7a1799caa1a8a6e9e6a7dab6d Stats: 36 lines in 15 files changed: 1 ins; 4 del; 31 mod 8274835: Remove unnecessary castings in java.base Reviewed-by: mullan, naoto, lancea, bpb - PR: https://git.openjdk.java.net/jdk/pull/5454
Re: RFR: 8276348: Use blessed modifier order in java.base
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
> in JDK-8136583 in 2015 [^2]. That change affected 1780 lines spanning 453
> files. Since then modifiers have become a bit loose, and it makes sense to
> re-bless (using the JDK-8136583 terminology) them.
>
> This change was produced by running the below command followed by updating
> the copyright years on the affected files where necessary:
>
> $ sh ./bin/blessed-modifier-order.sh src/java.base
>
> The resulting change is much smaller than that of 2015: 39 lines spanning 21
> files.
>
> [^1]:
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-November/082987.html
> (or https://github.com/openjdk/jdk/pull/6191#pullrequestreview-79465)
> [^2]:
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-September/035217.html
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.
-
PR: https://git.openjdk.java.net/jdk/pull/6213
Integrated: 8274879: Replace uses of StringBuffer with StringBuilder within java.base classes
On Thu, 9 Sep 2021 06:50:21 GMT, Andrey Turbanov wrote: > StringBuffer is a legacy synchronized class. There are more modern > alternatives which perform better: > 1. Plain String concatenation should be preferred > 2. StringBuilder is a direct replacement to StringBuffer which generally have > better performance > > In [JDK-8264029](https://bugs.openjdk.java.net/browse/JDK-8264029) I > migrated only usages which were automatically detected by IDEA. It turns out > there are more usages which can be migrated. This pull request has now been integrated. Changeset: 9a3e9542 Author:Andrey Turbanov Committer: Naoto Sato URL: https://git.openjdk.java.net/jdk/commit/9a3e9542997860de79d07a4411b1007e9cd5c348 Stats: 31 lines in 11 files changed: 0 ins; 0 del; 31 mod 8274879: Replace uses of StringBuffer with StringBuilder within java.base classes Reviewed-by: naoto - PR: https://git.openjdk.java.net/jdk/pull/5432
Integrated: 8275079: Remove unnecessary conversion to String in java.net.http
On Sat, 2 Oct 2021 20:05:37 GMT, Andrey Turbanov wrote: > Cleanup unnecessary String.valueOf calls (and similar) when conversion will > happen implicitly anyway This pull request has now been integrated. Changeset: 19f76c21 Author:Andrey Turbanov Committer: Daniel Fuchs URL: https://git.openjdk.java.net/jdk/commit/19f76c215dbe9528dde10acd744be54618ea5e4c Stats: 34 lines in 13 files changed: 0 ins; 3 del; 31 mod 8275079: Remove unnecessary conversion to String in java.net.http Reviewed-by: dfuchs - PR: https://git.openjdk.java.net/jdk/pull/5795
Re: RFR: 8274879: Replace uses of StringBuffer with StringBuilder within java.base classes [v2]
> StringBuffer is a legacy synchronized class. There are more modern > alternatives which perform better: > 1. Plain String concatenation should be preferred > 2. StringBuilder is a direct replacement to StringBuffer which generally have > better performance > > In [JDK-8264029](https://bugs.openjdk.java.net/browse/JDK-8264029) I > migrated only usages which were automatically detected by IDEA. It turns out > there are more usages which can be migrated. Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision: 8274879: Replace uses of StringBuffer with StringBuilder within java.base classes revert changes in spec to avoid CSR - Changes: - all: https://git.openjdk.java.net/jdk/pull/5432/files - new: https://git.openjdk.java.net/jdk/pull/5432/files/14005d1d..c8d68c2a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5432&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5432&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5432.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5432/head:pull/5432 PR: https://git.openjdk.java.net/jdk/pull/5432
Re: RFR: 8274879: Replace uses of StringBuffer with StringBuilder within java.base classes [v2]
On Tue, 12 Oct 2021 20:33:20 GMT, Naoto Sato wrote: >> reverted changes in this spec. > > Did you modify the PR? I am unable to locate the revert. Oops. Forgot to push - PR: https://git.openjdk.java.net/jdk/pull/5432
Integrated: 8274894: Use Optional.empty() instead of ofNullable(null) in HttpResponse.BodySubscribers.discarding
On Wed, 18 Aug 2021 07:55:35 GMT, Andrey Turbanov wrote: > Usage of `Optional.ofNullable` is unnecessary when value is known. If it's > `null` value Optional.empty() should be preferred, as it doesn't perform > redundant `null` check. This pull request has now been integrated. Changeset: d04d4ee2 Author:Andrey Turbanov Committer: Daniel Fuchs URL: https://git.openjdk.java.net/jdk/commit/d04d4ee2c193baf4339ee3025e3fbcd31d62f484 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod 8274894: Use Optional.empty() instead of ofNullable(null) in HttpResponse.BodySubscribers.discarding Reviewed-by: dfuchs - PR: https://git.openjdk.java.net/jdk/pull/5159
Re: RFR: 8274879: Replace uses of StringBuffer with StringBuilder within java.base classes
On Thu, 7 Oct 2021 16:48:06 GMT, Naoto Sato wrote:
>> StringBuffer is a legacy synchronized class. There are more modern
>> alternatives which perform better:
>> 1. Plain String concatenation should be preferred
>> 2. StringBuilder is a direct replacement to StringBuffer which generally
>> have better performance
>>
>> In [JDK-8264029](https://bugs.openjdk.java.net/browse/JDK-8264029) I
>> migrated only usages which were automatically detected by IDEA. It turns out
>> there are more usages which can be migrated.
>
> src/java.base/share/classes/java/lang/Character.java line 123:
>
>> 121: * than U+ are called supplementary characters. The Java
>> 122: * platform uses the UTF-16 representation in {@code char} arrays and
>> 123: * in the {@code String} and {@code StringBuilder} classes. In
>
> Not sure simple replacement applies here, as `StringBuffer` still uses
> `UTF-16` representation. You may add `StringBuilder` as well here, but I
> think you might want to file a CSR to clarify.
reverted changes in this spec.
-
PR: https://git.openjdk.java.net/jdk/pull/5432
Re: RFR: 8275079: Remove unnecessary conversion to String in java.net.http
On Mon, 4 Oct 2021 13:39:00 GMT, Weijun Wang wrote:
>> Cleanup unnecessary String.valueOf calls (and similar) when conversion will
>> happen implicitly anyway
>
> src/java.net.http/share/classes/jdk/internal/net/http/Http1AsyncReceiver.java
> line 738:
>
>> 736: if (flowTag != null) {
>> 737: dbgTag = tag = "Http1AsyncReceiver("+ flowTag + ")";
>> 738: } else {
>
> If the only use of `flowTag` is `flow` in string, it looks like `flow` is
> enough. i.e.
>
>
> if (flow != null) {
> dbgTag = tag = "Http1AsyncReceiver("+ flow + ")";
> }
Good idea. Improved.
> src/java.net.http/share/classes/jdk/internal/net/http/Http1Response.java line
> 831:
>
>> 829: @Override
>> 830: public String toString() {
>> 831: return super.toString() + "/parser=" + parser;
>
> Can we use `super` instead of `super.toString()`?
Nope. `super` is not a valid java expression.
-
PR: https://git.openjdk.java.net/jdk/pull/5795
RFR: 8275079: Remove unnecessary conversion to String in java.net.http
Cleanup unnecessary String.valueOf calls (and similar) when conversion will happen implicitly anyway - Commit messages: - 8275079: Remove unnecessary conversion to String in java.net.http - [PATCH] Remove unnecessary conversion to String Changes: https://git.openjdk.java.net/jdk/pull/5795/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5795&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8275079 Stats: 34 lines in 13 files changed: 0 ins; 3 del; 31 mod Patch: https://git.openjdk.java.net/jdk/pull/5795.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5795/head:pull/5795 PR: https://git.openjdk.java.net/jdk/pull/5795
Re: RFR: 8274949: Use String.contains() instead of String.indexOf() in java.base
On Fri, 8 Oct 2021 09:35:25 GMT, Daniel Fuchs wrote: >Did you run tier1, tier2? I did run tier2. (tier1 is automatically checked by GithubActions). No new tests failed. Only _usual_ tests, which always fail on my machine, were failed. - PR: https://git.openjdk.java.net/jdk/pull/5559
RFR: 8274949: Use String.contains() instead of String.indexOf() in java.base
String.contains was introduced in Java 5. Some code in java.base still uses old approach with `String.indexOf` to check if String contains specified substring. I propose to migrate such usages. Makes code shorter and easier to read. - Commit messages: - [PATCH] Use String.contains() instead of String.indexOf() in java.base - [PATCH] Use String.contains() instead of String.indexOf() in java.base Changes: https://git.openjdk.java.net/jdk/pull/5559/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5559&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274949 Stats: 40 lines in 17 files changed: 0 ins; 3 del; 37 mod Patch: https://git.openjdk.java.net/jdk/pull/5559.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5559/head:pull/5559 PR: https://git.openjdk.java.net/jdk/pull/5559
Integrated: 8273910: Redundant condition and assignment in java.net.URI
On Thu, 19 Aug 2021 19:08:59 GMT, Andrey Turbanov wrote: > 1. Assignment `ru.host = child.host;` is duplicated and hence redundant. > 2. Condition `q > p` is always `true`, because it just bellow inverse check > > if (q <= p) > break; This pull request has now been integrated. Changeset: 7de2cf85 Author:Andrey Turbanov Committer: Daniel Fuchs URL: https://git.openjdk.java.net/jdk/commit/7de2cf852d75ea6eb039e69067d4e32421283de5 Stats: 11 lines in 1 file changed: 4 ins; 7 del; 0 mod 8273910: Redundant condition and assignment in java.net.URI Reviewed-by: dfuchs - PR: https://git.openjdk.java.net/jdk/pull/5190
Re: RFR: 8274894: Use Optional.empty() instead of ofNullable(null) in HttpResponse.BodySubscribers.discarding
On Wed, 18 Aug 2021 07:55:35 GMT, Andrey Turbanov wrote: > Usage of `Optional.ofNullable` is unnecessary when value is known. If it's > `null` value Optional.empty() should be preferred, as it doesn't perform > redundant `null` check. Not sure if it worth separate issue. But this this is small improvement in http client code. - PR: https://git.openjdk.java.net/jdk/pull/5159
RFR: 8274894: Use Optional.empty() instead of ofNullable(null) in HttpResponse.BodySubscribers.discarding
Usage of `Optional.ofNullable` is unnecessary when value is known. If it's `null` value Optional.empty() should be preferred, as it doesn't perform redundant `null` check. - Commit messages: - [PATCH] Cleanup Optional usage in HttpResponse.BodySubscribers.discarding Changes: https://git.openjdk.java.net/jdk/pull/5159/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5159&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274894 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/5159.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5159/head:pull/5159 PR: https://git.openjdk.java.net/jdk/pull/5159
RFR: 8274879: Replace uses of StringBuffer with StringBuilder within java.base classes
StringBuffer is a legacy synchronized class. There are more modern alternatives which perform better: 1. Plain String concatenation should be preferred 2. StringBuilder is a direct replacement to StringBuffer which generally have better performance In [JDK-8264029](https://bugs.openjdk.java.net/browse/JDK-8264029) I migrated only usages which were automatically detected by IDEA. It turns out there are more usages which can be migrated. - Commit messages: - [PATCH] Cleanup usages of StringBuffer in java.base module - [PATCH] Cleanup usages of StringBuffer in java.base module Changes: https://git.openjdk.java.net/jdk/pull/5432/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5432&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274879 Stats: 32 lines in 12 files changed: 0 ins; 0 del; 32 mod Patch: https://git.openjdk.java.net/jdk/pull/5432.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5432/head:pull/5432 PR: https://git.openjdk.java.net/jdk/pull/5432
Re: RFR: 8274809: Update java.base classes to use try-with-resources [v4]
On Wed, 6 Oct 2021 16:00:26 GMT, Bradford Wetmore wrote:
>> Andrey Turbanov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8274809: Update java.base classes to use try-with-resources
>> fix review comments
>
> src/java.base/share/classes/sun/security/timestamp/HttpTimestamper.java line
> 115:
>
>> 113:
>> 114: // Send the request
>> 115: try (DataOutputStream output = new
>> DataOutputStream(connection.getOutputStream())) {
>
> Please break this up so it doesn't have lines > 80. e.g.
>
> try (DataOutputStream output =
> new DataOutputStream(connection.getOutputStream())) {
>
> This makes side-by-side viewing very hard without a wider monitor.
>
> Thank you.
Update.
-
PR: https://git.openjdk.java.net/jdk/pull/5818
Re: RFR: 8274809: Update java.base classes to use try-with-resources [v4]
> 8274809: Update java.base classes to use try-with-resources Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision: 8274809: Update java.base classes to use try-with-resources fix review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/5818/files - new: https://git.openjdk.java.net/jdk/pull/5818/files/423c3069..3b05ec49 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5818&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5818&range=02-03 Stats: 5 lines in 2 files changed: 2 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/5818.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5818/head:pull/5818 PR: https://git.openjdk.java.net/jdk/pull/5818
Re: RFR: 8274809: Update java.base classes to use try-with-resources [v3]
> 8274809: Update java.base classes to use try-with-resources Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision: 8274809: Update java.base classes to use try-with-resources update copyright year - Changes: - all: https://git.openjdk.java.net/jdk/pull/5818/files - new: https://git.openjdk.java.net/jdk/pull/5818/files/78893df9..423c3069 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5818&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5818&range=01-02 Stats: 5 lines in 4 files changed: 1 ins; 1 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/5818.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5818/head:pull/5818 PR: https://git.openjdk.java.net/jdk/pull/5818
Re: RFR: 8274809: Update java.base classes to use try-with-resources [v3]
On Wed, 6 Oct 2021 16:11:25 GMT, Bradford Wetmore wrote: >Files like HttpTimestamper need the copyright dates updated to 2021. Updated - PR: https://git.openjdk.java.net/jdk/pull/5818
Re: RFR: 8274809: Update java.base classes to use try-with-resources [v2]
On Wed, 6 Oct 2021 16:20:48 GMT, Pavel Rappo wrote:
>> src/java.base/share/classes/sun/security/timestamp/HttpTimestamper.java line
>> 127:
>>
>>> 125: // Receive the reply
>>> 126: byte[] replyBuffer = null;
>>> 127: try (BufferedInputStream input = new
>>> BufferedInputStream(connection.getInputStream())) {
>>
>> Same comment as above.
>
> In this and the immediately preceding case, it might be better to use `var`.
I like variant with `var` more. Updated
-
PR: https://git.openjdk.java.net/jdk/pull/5818
Re: RFR: 8274809: Update java.base classes to use try-with-resources [v2]
> 8274809: Update java.base classes to use try-with-resources Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision: 8274809: Update java.base classes to use try-with-resources fix post-review comment - Changes: - all: https://git.openjdk.java.net/jdk/pull/5818/files - new: https://git.openjdk.java.net/jdk/pull/5818/files/6d8ce900..78893df9 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5818&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5818&range=00-01 Stats: 7 lines in 2 files changed: 1 ins; 1 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/5818.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5818/head:pull/5818 PR: https://git.openjdk.java.net/jdk/pull/5818
Re: RFR: 8274809: Update java.base classes to use try-with-resources
On Wed, 6 Oct 2021 16:24:03 GMT, Andrey Turbanov
wrote:
>> src/java.base/share/classes/sun/net/NetProperties.java line 71:
>>
>>> 69: f = new File(f, "net.properties");
>>> 70: fname = f.getCanonicalPath();
>>> 71: try (FileInputStream fis = new FileInputStream(fname)) {
>>
>> Why did the BufferedInputStream get pulled out here?
>
> I decieded to remove it because `Properties.load` already buffers
> inputstream. So usage of BufferedInputStream seems redundant.
> https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/Properties.java#L471
I will revert back to `BufferedInputStream` and will create a separate PR to
cleanup redundant buffering around Properties.load calls.
-
PR: https://git.openjdk.java.net/jdk/pull/5818
Re: RFR: 8274809: Update java.base classes to use try-with-resources
On Wed, 6 Oct 2021 16:04:54 GMT, Bradford Wetmore wrote:
>> 8274809: Update java.base classes to use try-with-resources
>
> src/java.base/share/classes/sun/net/NetProperties.java line 71:
>
>> 69: f = new File(f, "net.properties");
>> 70: fname = f.getCanonicalPath();
>> 71: try (FileInputStream fis = new FileInputStream(fname)) {
>
> Why did the BufferedInputStream get pulled out here?
I decieded to remove it because `Properties.load` already buffers inputstream.
So usage of BufferedInputStream seems redundant.
https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/Properties.java#L471
-
PR: https://git.openjdk.java.net/jdk/pull/5818
RFR: 8274835: Remove unnecessary castings in java.base
Redundant castings make code harder to read. Found them by IntelliJ IDEA. I tried to select only casts which are definitely safe to remove. Also didn't touch primitive types casts. - Commit messages: - [PATCH] Remove unnecessary castings in java.base - [PATCH] Remove unnecessary castings in java.base Changes: https://git.openjdk.java.net/jdk/pull/5454/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5454&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274835 Stats: 38 lines in 15 files changed: 1 ins; 4 del; 33 mod Patch: https://git.openjdk.java.net/jdk/pull/5454.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5454/head:pull/5454 PR: https://git.openjdk.java.net/jdk/pull/5454
RFR: 8274809: Update java.base classes to use try-with-resources
8274809: Update java.base classes to use try-with-resources - Commit messages: - [PATCH] Use try-with-resources to close FileInputStream in java.base Changes: https://git.openjdk.java.net/jdk/pull/5818/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5818&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274809 Stats: 54 lines in 7 files changed: 2 ins; 40 del; 12 mod Patch: https://git.openjdk.java.net/jdk/pull/5818.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5818/head:pull/5818 PR: https://git.openjdk.java.net/jdk/pull/5818
Integrated: 8274079: Cleanup unnecessary calls to Throwable.initCause() in java.base module
On Thu, 16 Sep 2021 19:03:26 GMT, Andrey Turbanov wrote: > Pass "cause" exception as constructor parameter is shorter and easier to read. This pull request has now been integrated. Changeset: 1459180f Author:Andrey Turbanov Committer: Weijun Wang URL: https://git.openjdk.java.net/jdk/commit/1459180f352a5632c0afca2ed55abf31e4b0bfb0 Stats: 132 lines in 22 files changed: 0 ins; 77 del; 55 mod 8274079: Cleanup unnecessary calls to Throwable.initCause() in java.base module Reviewed-by: weijun - PR: https://git.openjdk.java.net/jdk/pull/5551
Integrated: 8274237: Replace 'for' cycles with iterator with enhanced-for in java.base
On Thu, 23 Sep 2021 20:42:48 GMT, Andrey Turbanov wrote: > There are few places in code where manual `for` loop is used with Iterator to > iterate over Collection. > Instead of manual `for` cycles it's preferred to use enhanced-for cycle > instead: it's less verbose, makes code easier to read and it's less > error-prone. > Sometimes we even don't need cycle at all: we can just create one ArrayList > as a copy of another. > It doesn't have any performance impact: java compiler generates similar code > when compiling enhanced-for cycle. > This is continuation of > [JDK-8273261](https://bugs.openjdk.java.net/browse/JDK-8273261) This pull request has now been integrated. Changeset: baafa605 Author:Andrey Turbanov Committer: Weijun Wang URL: https://git.openjdk.java.net/jdk/commit/baafa6059e7aa50978b0b29946fddb8b198a28d2 Stats: 32 lines in 4 files changed: 0 ins; 14 del; 18 mod 8274237: Replace 'for' cycles with iterator with enhanced-for in java.base Reviewed-by: dfuchs, weijun - PR: https://git.openjdk.java.net/jdk/pull/5665
RFR: 8274237: Replace 'for' cycles with iterator with enhanced-for in java.base
There are few places in code where manual `for` loop is used with Iterator to iterate over Collection. Instead of manual `for` cycles it's preferred to use enhanced-for cycle instead: it's less verbose, makes code easier to read and it's less error-prone. Sometimes we even don't need cycle at all: we can just create one ArrayList as a copy of another. It doesn't have any performance impact: java compiler generates similar code when compiling enhanced-for cycle. This is continuation of [JDK-8273261](https://bugs.openjdk.java.net/browse/JDK-8273261) - Commit messages: - [PATCH] Replace 'for' cycles with iterator with enhanced-for in java.base Changes: https://git.openjdk.java.net/jdk/pull/5665/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5665&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274237 Stats: 32 lines in 4 files changed: 0 ins; 14 del; 18 mod Patch: https://git.openjdk.java.net/jdk/pull/5665.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5665/head:pull/5665 PR: https://git.openjdk.java.net/jdk/pull/5665
Integrated: 8273261: Replace 'while' cycles with iterator with enhanced-for in java.base
On Wed, 1 Sep 2021 07:37:53 GMT, Andrey Turbanov wrote: > There are few places in code where manual while loop is used with Iterator to > iterate over Collection. > Instead of manual while cycles it's preferred to use enhanced-for cycle > instead: it's less verbose, makes code easier to read and it's less > error-prone. > It doesn't have any performance impact: java compiler generates similar code > when compiling enhanced-for cycle. > > Similar cleanups: > * https://bugs.openjdk.java.net/browse/JDK-8258006 > * https://bugs.openjdk.java.net/browse/JDK-8257912 This pull request has now been integrated. Changeset: 56b8b352 Author:Andrey Turbanov Committer: Roger Riggs URL: https://git.openjdk.java.net/jdk/commit/56b8b35286634f2d2224ca445bc9f32ff284ae74 Stats: 93 lines in 11 files changed: 1 ins; 50 del; 42 mod 8273261: Replace 'while' cycles with iterator with enhanced-for in java.base Reviewed-by: dfuchs, rriggs, iris, mullan - PR: https://git.openjdk.java.net/jdk/pull/5328
Re: RFR: 8273261: Replace 'while' cycles with iterator with enhanced-for in java.base
On Wed, 1 Sep 2021 07:37:53 GMT, Andrey Turbanov wrote: > There are few places in code where manual while loop is used with Iterator to > iterate over Collection. > Instead of manual while cycles it's preferred to use enhanced-for cycle > instead: it's less verbose, makes code easier to read and it's less > error-prone. > It doesn't have any performance impact: java compiler generates similar code > when compiling enhanced-for cycle. > > Similar cleanups: > * https://bugs.openjdk.java.net/browse/JDK-8258006 > * https://bugs.openjdk.java.net/browse/JDK-8257912 Can someone sponsor changes, please? I believe we have enough approvals. - PR: https://git.openjdk.java.net/jdk/pull/5328
RFR: 8274079: Cleanup unnecessary calls to Throwable.initCause() in java.base module
Pass "cause" exception as constructor parameter is shorter and easier to read. - Commit messages: - [PATCH] Cleanup unnecessary calls to Throwable.initCause() in java.base module Changes: https://git.openjdk.java.net/jdk/pull/5551/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5551&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274079 Stats: 133 lines in 22 files changed: 0 ins; 77 del; 56 mod Patch: https://git.openjdk.java.net/jdk/pull/5551.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5551/head:pull/5551 PR: https://git.openjdk.java.net/jdk/pull/5551
RFR: 8273910: Redundant condition and assignment in java.net.URI
1. Assignment `ru.host = child.host;` is duplicated and hence redundant. 2. Condition `q > p` is always `true`, because it just bellow inverse check if (q <= p) break; - Commit messages: - [PATCH] Cleanup redundant assignment and condition in URI Changes: https://git.openjdk.java.net/jdk/pull/5190/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5190&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8273910 Stats: 11 lines in 1 file changed: 4 ins; 7 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/5190.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5190/head:pull/5190 PR: https://git.openjdk.java.net/jdk/pull/5190
Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server
On Tue, 14 Sep 2021 08:52:37 GMT, Julia Boes wrote: > This change implements a simple web server that can be run on the > command-line with `java -m jdk.httpserver`. > > This is facilitated by adding an entry point for the `jdk.httpserver` module, > an implementation class whose main method is run when the above command is > executed. This is the first such module entry point in the JDK. > > The server is a minimal HTTP server that serves the static files of a given > directory, similar to existing alternatives on other platforms and convenient > for testing, development, and debugging. > > Additionally, a small API is introduced for programmatic creation and > customization. > > Testing: tier1-3. src/jdk.httpserver/share/classes/com/sun/net/httpserver/Headers.java line 288: > 286: } > 287: > 288: private static final Headers EMPTY = new UnmodifiableHeaders(new > Headers()); IDEA warns here: >Referencing subclass UnmodifiableHeaders from superclass Headers initializer >might lead to class loading deadlock >Such references can cause JVM-level deadlocks in multithreaded environment, >when one thread tries to load the superclass and another thread tries to load >the subclass at the same time. - PR: https://git.openjdk.java.net/jdk/pull/5505
Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server
On Tue, 14 Sep 2021 08:52:37 GMT, Julia Boes wrote: > This change implements a simple web server that can be run on the > command-line with `java -m jdk.httpserver`. > > This is facilitated by adding an entry point for the `jdk.httpserver` module, > an implementation class whose main method is run when the above command is > executed. This is the first such module entry point in the JDK. > > The server is a minimal HTTP server that serves the static files of a given > directory, similar to existing alternatives on other platforms and convenient > for testing, development, and debugging. > > Additionally, a small API is introduced for programmatic creation and > customization. > > Testing: tier1-3. src/jdk.httpserver/share/classes/com/sun/net/httpserver/Headers.java line 106: > 104: var h = headers.entrySet().stream() > 105: .collect(Collectors.toUnmodifiableMap( > 106: Entry::getKey, e -> new > LinkedList<>(e.getValue(; I wonder, what the reason of `LinkedList` usages here? As I know ArrayList is faster in all real-world scenarios. - PR: https://git.openjdk.java.net/jdk/pull/5505
Re: RFR: 8273261: Replace 'while' cycles with iterator with enhanced-for in java.base
On Wed, 1 Sep 2021 07:37:53 GMT, Andrey Turbanov wrote: > There are few places in code where manual while loop is used with Iterator to > iterate over Collection. > Instead of manual while cycles it's preferred to use enhanced-for cycle > instead: it's less verbose, makes code easier to read and it's less > error-prone. > It doesn't have any performance impact: java compiler generates similar code > when compiling enhanced-for cycle. > > Similar cleanups: > * https://bugs.openjdk.java.net/browse/JDK-8258006 > * https://bugs.openjdk.java.net/browse/JDK-8257912 Tested "tier2" on linux-x86_64-server-fastdebug Ubuntu 21.04 == Test summary == TEST TOTAL PASS FAIL ERROR >> jtreg:test/jdk:tier2 3772 3771 1 0 << jtreg:test/langtools:tier2 1111 0 0 jtreg:test/jaxp:tier2 452 452 0 0 == TEST FAILURE Only one test `java/nio/file/FileStore/Basic.java` failed, but it fails without my changes too. --System.out:(46/2591)-- - PR: https://git.openjdk.java.net/jdk/pull/5328
RFR: 8273261: Replace 'while' cycles with iterator with enhanced-for in java.base
There are few places in code where manual while loop is used with Iterator to iterate over Collection. Instead of manual while cycles it's preferred to use enhanced-for cycle instead: it's less verbose, makes code easier to read and it's less error-prone. It doesn't have any performance impact: java compiler generates similar code when compiling enhanced-for cycle. Similar cleanups: * https://bugs.openjdk.java.net/browse/JDK-8258006 * https://bugs.openjdk.java.net/browse/JDK-8257912 - Commit messages: - [PATCH] Replace while cycles with iterator with enhanced-for in java.base Changes: https://git.openjdk.java.net/jdk/pull/5328/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5328&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8273261 Stats: 93 lines in 11 files changed: 1 ins; 50 del; 42 mod Patch: https://git.openjdk.java.net/jdk/pull/5328.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5328/head:pull/5328 PR: https://git.openjdk.java.net/jdk/pull/5328
Integrated: 8272863: Replace usages of Collections.sort with List.sort call in public java modules
On Mon, 23 Aug 2021 21:01:48 GMT, Andrey Turbanov wrote: > Collections.sort is just a wrapper, so it is better to use an instance method > directly. This pull request has now been integrated. Changeset: d732c309 Author:Andrey Turbanov Committer: Sergey Bylokhov URL: https://git.openjdk.java.net/jdk/commit/d732c3091fea0a7c6d6767227de89002564504e5 Stats: 27 lines in 10 files changed: 0 ins; 8 del; 19 mod 8272863: Replace usages of Collections.sort with List.sort call in public java modules Reviewed-by: serb, dfuchs, naoto - PR: https://git.openjdk.java.net/jdk/pull/5229
Re: RFR: 8272863: Replace usages of Collections.sort with List.sort call in public java modules [v3]
> Collections.sort is just a wrapper, so it is better to use an instance method > directly. Andrey Turbanov 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. - Changes: - all: https://git.openjdk.java.net/jdk/pull/5229/files - new: https://git.openjdk.java.net/jdk/pull/5229/files/d6dfc8bf..e31936a5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5229&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5229&range=01-02 Stats: 21 lines in 10 files changed: 4 ins; 0 del; 17 mod Patch: https://git.openjdk.java.net/jdk/pull/5229.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5229/head:pull/5229 PR: https://git.openjdk.java.net/jdk/pull/5229
Re: RFR: 8272863: Replace usages of Collections.sort with List.sort call in public java modules [v2]
On Wed, 25 Aug 2021 08:29:57 GMT, Daniel Fuchs wrote: >> Andrey Turbanov has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8272863: Replace usages of Collections.sort with List.sort call in public >> java modules >> replace Collections.sort without comparator too > > src/java.base/share/classes/java/net/URLPermission.java line 222: > >> 220: >> 221: List l = normalizeMethods(methods); >> 222: l.sort(null); > > I am not opposed to this change, but I find this is slightly more ugly than > `Collections.sort(l)`; so I have to ask: Is there any noticeable benefit? Actually I agree with you. One more point is that List.sort(null) doesn't check at compile time that class implements `Comparable`. But Collections.sort have this check. I will revert last commit. @azvegint are you ok with that? - PR: https://git.openjdk.java.net/jdk/pull/5229
Re: RFR: 8272863: Replace usages of Collections.sort with List.sort call in public java modules
On Tue, 24 Aug 2021 11:48:46 GMT, Alexander Zvegintsev wrote: > Is there any reason to not touch them along with this fix? Update them too. - PR: https://git.openjdk.java.net/jdk/pull/5229
Re: RFR: 8272863: Replace usages of Collections.sort with List.sort call in public java modules [v2]
> Collections.sort is just a wrapper, so it is better to use an instance method > directly. Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision: 8272863: Replace usages of Collections.sort with List.sort call in public java modules replace Collections.sort without comparator too - Changes: - all: https://git.openjdk.java.net/jdk/pull/5229/files - new: https://git.openjdk.java.net/jdk/pull/5229/files/e31936a5..d6dfc8bf Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5229&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5229&range=00-01 Stats: 21 lines in 10 files changed: 0 ins; 4 del; 17 mod Patch: https://git.openjdk.java.net/jdk/pull/5229.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5229/head:pull/5229 PR: https://git.openjdk.java.net/jdk/pull/5229
RFR: 8272863: Replace usages of Collections.sort with List.sort call in public java modules
Collections.sort is just a wrapper, so it is better to use an instance method directly. - Commit messages: - [PATCH] Replace usages of Collections.sort with List.sort call in public java modules Changes: https://git.openjdk.java.net/jdk/pull/5229/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5229&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8272863 Stats: 27 lines in 10 files changed: 0 ins; 8 del; 19 mod Patch: https://git.openjdk.java.net/jdk/pull/5229.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5229/head:pull/5229 PR: https://git.openjdk.java.net/jdk/pull/5229
Re: RFR: 8272805: Avoid looking up standard charsets [v2]
On Sun, 22 Aug 2021 23:02:06 GMT, Sergey Bylokhov wrote:
>> This is the continuation of JDK-8233884, JDK-8271456, and JDK-8272120.
>>
>> In many places standard charsets are looked up via their names, for example:
>> absolutePath.getBytes("UTF-8");
>>
>> This could be done more efficiently(up to x20 time faster) with use of
>> java.nio.charset.StandardCharsets:
>> absolutePath.getBytes(StandardCharsets.UTF_8);
>>
>> The later variant also makes the code cleaner, as it is known not to throw
>> UnsupportedEncodingException in contrary to the former variant.
>>
>> This change includes:
>> * demo/utils
>> * jdk.xx packages
>> * Some places were missed in the previous changes. I have found it by
>> tracing the calls to the Charset.forName() by executing tier1,2,3 and
>> desktop tests.
>>
>> Some performance discussion: https://github.com/openjdk/jdk/pull/5063
>>
>> Code excluded in this fix: the Xerces library(should be fixed upstream),
>> J2DBench(should be compatible to 1.4), some code in the network(the change
>> there are not straightforward, will do it later).
>>
>> Tested by the tier1/tier2/tier3 tests on Linux/Windows/macOS.
>
> Sergey Bylokhov 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 14 additional
> commits since the last revision:
>
> - Update the usage of Files.readAllLines()
> - Update datatransfer
> - Merge branch 'master' into standard-encodings-in-non-public-modules
> - Merge branch 'master' into standard-encodings-in-non-public-modules
> - Fix related imports
> - Merge branch 'master' into standard-encodings-in-non-public-modules
> - Cleanup UnsupportedEncodingException
> - Update PacketStream.java
> - Rollback TextTests, should be compatible with jdk1.4
> - Rollback TextRenderTests, should be compatible with jdk1.4
> - ... and 4 more:
> https://git.openjdk.java.net/jdk/compare/aaa7beaf...e7127644
Marked as reviewed by turban...@github.com (no known OpenJDK username).
-
PR: https://git.openjdk.java.net/jdk/pull/5210
Re: RFR: 8272805: Avoid looking up standard charsets
On Sun, 22 Aug 2021 02:53:44 GMT, Sergey Bylokhov wrote:
> This is the continuation of JDK-8233884, JDK-8271456, and JDK-8272120.
>
> In many places standard charsets are looked up via their names, for example:
> absolutePath.getBytes("UTF-8");
>
> This could be done more efficiently(up to x20 time faster) with use of
> java.nio.charset.StandardCharsets:
> absolutePath.getBytes(StandardCharsets.UTF_8);
>
> The later variant also makes the code cleaner, as it is known not to throw
> UnsupportedEncodingException in contrary to the former variant.
>
> This change includes:
> * demo/utils
> * jdk.xx packages
> * Some places were missed in the previous changes. I have found it by
> tracing the calls to the Charset.forName() by executing tier1,2,3 and desktop
> tests.
>
> Some performance discussion: https://github.com/openjdk/jdk/pull/5063
>
> Code excluded in this fix: the Xerces library(should be fixed upstream),
> J2DBench(should be compatible to 1.4), some code in the network(the change
> there are not straightforward, will do it later).
>
> Tested by the tier1/tier2/tier3 tests on Linux/Windows/macOS.
I think it's worth to update _static_ initializer in
`sun.datatransfer.DataFlavorUtil.CharsetComparator` too.
-
PR: https://git.openjdk.java.net/jdk/pull/5210
Redundant Math.min call in Http2ClientImpl#getConnectionWindowSize
Hello. During investigation of results of IDEA inspections I found a redundant call to Math.min in a method jdk.internal.net.http.Http2ClientImpl#getConnectionWindowSize https://github.com/openjdk/jdk/blob/master/src/java.net.http/share/classes/jdk/internal/net/http/Http2ClientImpl.java#L240 int defaultValue = Math.min(Integer.MAX_VALUE, Math.max(streamWindow, K*K*32)); Call of method Math.min(int, int) is redundant if one of parameters is known to be Integer.MAX_VALUE (or Integer.MIN_VALUE) Andrey Turbanov
Suspicious 'completed' variable in ResponseContent.UnknownLengthBodyParser#accept
Hello
During investigation of results of IDEA inspections I found suspicious
code in a method
'jdk.internal.net.http.ResponseContent.UnknownLengthBodyParser#accept'
https://github.com/openjdk/jdk/blob/master/src/java.net.http/share/classes/jdk/internal/net/http/ResponseContent.java#L492
boolean completed = false;
try {
if (debug.on())
debug.log("Parser got %d bytes ", b.remaining());
if (b.hasRemaining()) {
// only reduce demand if we actually push something.
// we would not have come here if there was no
// demand.
boolean hasDemand = sub.demand().tryDecrement();
assert hasDemand;
breceived += b.remaining();
pusher.onNext(List.of(b.asReadOnlyBuffer()));
}
} catch (Throwable t) {
if (debug.on()) debug.log("Unexpected exception", t);
closedExceptionally = t;
if (!completed) {
onComplete.accept(t);
}
}
Variable 'completed' has an initial value 'false' and never assigned again.
Then it's checked in the 'catch' section.
It seems it should be set to 'true' somewhere.
Andrey Turbanov
Integrated: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy
On Sun, 20 Dec 2020 17:05:21 GMT, Andrey Turbanov wrote: > 8080272 Refactor I/O stream copying to use > InputStream.transferTo/readAllBytes and Files.copy This pull request has now been integrated. Changeset: 68deb24b Author:Andrey Turbanov Committer: Julia Boes URL: https://git.openjdk.java.net/jdk/commit/68deb24b Stats: 105 lines in 7 files changed: 3 ins; 78 del; 24 mod 8080272: Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy Reviewed-by: mcimadamore, alanb - PR: https://git.openjdk.java.net/jdk/pull/1853
Re: RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy [v13]
> 8080272 Refactor I/O stream copying to use > InputStream.transferTo/readAllBytes and Files.copy Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy drop changes in X509CertPath - Changes: - all: https://git.openjdk.java.net/jdk/pull/1853/files - new: https://git.openjdk.java.net/jdk/pull/1853/files/1b30471d..96920ee6 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1853&range=12 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1853&range=11-12 Stats: 25 lines in 1 file changed: 22 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/1853.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1853/head:pull/1853 PR: https://git.openjdk.java.net/jdk/pull/1853
Re: RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy [v11]
On Fri, 19 Feb 2021 15:03:11 GMT, Sean Mullan wrote:
>> As I can see only ByteArrayInputStream is actually passed in `InputStream`
>> in current JDK code:
>>
>> PKCS7 pkcs7 = new PKCS7(is.readAllBytes());
>> private static List parsePKCS7(InputStream is)
>> certs = parsePKCS7(is);
>> public X509CertPath(InputStream is, String encoding)
>> return new X509CertPath(new ByteArrayInputStream(data),
>> encoding);
>>
>> PKCS7 pkcs7 = new PKCS7(is.readAllBytes());
>> private static List parsePKCS7(InputStream is)
>> certs = parsePKCS7(is);
>> public X509CertPath(InputStream is, String encoding)
>> this(is, PKIPATH_ENCODING);
>> public X509CertPath(InputStream is) throws
>> CertificateException {
>> return new X509CertPath(new
>> ByteArrayInputStream(encoding));
>>
>> 
>>
>> Perhaps original marking approach was lost during refactoring?
>
> You are right, the code was refactored (way back in 2010) [1] to read one
> block at a time, so this check on mark can be removed. So, in this case, I
> think it is probably safe to just pass the InputStream as-is to
> PKCS7(InputStream), but maybe you can add a comment that says this should
> always be a ByteArrayInputStream. We can look at refactoring this code and
> clean it up a bit more later.
>
> [1] https://hg.openjdk.java.net/jdk/jdk/rev/337ae296b6d6
I find implementation of `sun.security.pkcs.PKCS7#PKCS7(java.io.InputStream)` a
bit confusing (or even buggy). It uses only `InputStream.available()` to parse
block.
So I would prefer to not use it.
-
PR: https://git.openjdk.java.net/jdk/pull/1853
Re: RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy [v11]
On Thu, 18 Feb 2021 19:21:45 GMT, Sean Mullan wrote:
>> Andrey Turbanov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8080272: Refactor I/O stream copying to use java.io.InputStream.transferTo
>> remove unnecessary file.exists() check
>
> src/java.base/share/classes/sun/security/provider/certpath/X509CertPath.java
> line 228:
>
>> 226: try {
>> 227: if (is.markSupported() == false) {
>> 228: // Copy the entire input stream into an InputStream
>> that does
>
> I don't think you should remove lines 228-232. These methods are called by
> methods of CertificateFactory that take InputStream (which may contain a
> stream of security data) and they are designed such that they try to read one
> Certificate, CRL, or CertPath from the InputStream and leave the InputStream
> ready to parse the next structure instead of consuming all of the bytes. Thus
> they check if the InputStream supports mark in order to try to preserve that
> behavior. If mark is not supported, then it's ok to use
> InputStream.readAllBytes, otherwise, leave the stream as-is.
As I can see only ByteArrayInputStream is actually passed in `InputStream` in
current JDK code:
PKCS7 pkcs7 = new PKCS7(is.readAllBytes());
private static List parsePKCS7(InputStream is)
certs = parsePKCS7(is);
public X509CertPath(InputStream is, String encoding)
return new X509CertPath(new ByteArrayInputStream(data),
encoding);
PKCS7 pkcs7 = new PKCS7(is.readAllBytes());
private static List parsePKCS7(InputStream is)
certs = parsePKCS7(is);
public X509CertPath(InputStream is, String encoding)
this(is, PKIPATH_ENCODING);
public X509CertPath(InputStream is) throws
CertificateException {
return new X509CertPath(new
ByteArrayInputStream(encoding));
Perhaps original marking approach was lost during refactoring?
-
PR: https://git.openjdk.java.net/jdk/pull/1853
Re: RFR: 8260366: ExtendedSocketOptions can deadlock in some circumstances [v2]
On Thu, 18 Feb 2021 07:35:51 GMT, Jaikiran Pai wrote: >> Yes. Once here and once inside `synchronized` block. >> Reading `volatile` fields cost _something_ on some architectures, so I think >> we could optimize it a bit. > > Hello @turbanoff, the double read is necessary to correctly avoid any race > conditions and is a typical strategy used in cases like these. I am not aware > of any other alternate more performant strategy, for code like this. Let me be more clear: I think that it's enough to have only 2 `volatile` field reads _in worst case_. We can use local variable to avoid more than needed reads. Current code can perform read 4 times _in worst case_: twice outside `synchronized` block and twice inside `synchronized` block. There are many examples of similar code in the JDK: https://github.com/openjdk/jdk/blob/master/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Util.java#L48 https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/lang/StackFrameInfo.java#L125 - PR: https://git.openjdk.java.net/jdk/pull/2601
Re: RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy
On Tue, 16 Feb 2021 17:20:37 GMT, Julia Boes wrote: >> 8080272 Refactor I/O stream copying to use >> InputStream.transferTo/readAllBytes and Files.copy > > Hi @turbanoff, I'm happy to sponsor but I see two comments by @marschall - > have they been addressed? > https://github.com/openjdk/jdk/pull/1853#discussion_r572815422 > https://github.com/openjdk/jdk/pull/1853#discussion_r572380746 @FrauBoes fixed in the last commit. Is there any way to de-_integrate_ PR (to include last commit)? - PR: https://git.openjdk.java.net/jdk/pull/1853
Re: RFR: 8260366: ExtendedSocketOptions can deadlock in some circumstances [v2]
On Thu, 18 Feb 2021 01:08:30 GMT, Jaikiran Pai wrote:
>> src/java.base/share/classes/sun/net/ext/ExtendedSocketOptions.java line 171:
>>
>>> 169:
>>> 170: public static ExtendedSocketOptions getInstance() {
>>> 171: if (instance != null) {
>>
>> May be it's worth to avoid reading `volatile` field twice?
>
> Hello @turbanoff, do you mean why read it twice - once here and once inside
> the `synchronized` block?
Yes. Once here and once inside `synchronized` block.
Reading `volatile` fields cost _something_ on some architectures, so I think we
could optimize it a bit.
-
PR: https://git.openjdk.java.net/jdk/pull/2601
Re: RFR: 8260366: ExtendedSocketOptions can deadlock in some circumstances [v2]
On Wed, 17 Feb 2021 13:42:57 GMT, Jaikiran Pai wrote:
>> Can I please get a review for this change which proposes to fix the issue
>> reported in https://bugs.openjdk.java.net/browse/JDK-8260366?
>>
>> The issue relates to the concurrent classloading of
>> `sun.net.ext.ExtendedSocketOptions` and `jdk.net.ExtendedSocketOptions`
>> leading to a deadlock. This is because the
>> `sun.net.ext.ExtendedSocketOptions` in its static block does a
>> `Class.forName("jdk.net.ExtendedSocketOptions")`. The
>> `jdk.net.ExtendedSocketOptions` in its own static block calls the `register`
>> method on `sun.net.ext.ExtendedSocketOptions`. If 2 threads concurrently try
>> loading these classes (one loading the `sun.net.ext.ExtendedSocketOptions`
>> and the other loading `jdk.net.ExtendedSocketOptions`), it can so happen
>> that each one ends up holding a classloading lock in the static block of the
>> respective class, while waiting for the other thread to release the lock,
>> thus leading to a deadlock. The stacktrace posted in the linked JBS issue
>> has the necessary details on the deadlock.
>>
>> The commit here breaks this deadlock by moving out the
>> `Class.forName("jdk.net.ExtendedSocketOptions")` call from the static block
>> of `sun.net.ext.ExtendedSocketOptions` to the `getInstance()` method, thus
>> lazily loading (on first call to `getInstance()`) and registering the
>> `jdk.net.ExtendedSocketOptions`.
>>
>> Extra attention needs to be given to the
>> `sun.net.ext.ExtendedSocketOptions#register(ExtendedSocketOptions
>> extOptions)` method. Before the change in this PR, when the
>> `sun.net.ext.ExtendedSocketOptions` would successfully complete loading, it
>> was guaranteed that the registered `ExtendedSocketOptions` would either be
>> the one registered from the `jdk.net.ExtendedSocketOptions` or a
>> `NoExtendedSocketOptions`. There wasn't any window of chance for any code
>> (be it in the JDK or in application code) to call the
>> `sun.net.ext.ExtendedSocketOptions#register` to register any different/other
>> implementation/instance of the `ExtendedSocketOptions`. However, with this
>> change in the PR, there is now a window of chance where any code in the JDK
>> (or even application code?) can potentially call the
>> `sun.net.ext.ExtendedSocketOptions#register` before either the
>> `jdk.net.ExtendedSocketOptions` is loaded or the
>> `sun.net.ext.ExtendedSocketOptions#getInstance()` method is called, thus
>> allowing for some
other implementation of the `ExtendedSocketOptions` to be registered. However,
I'm not sure if it's a practical scenario - although the
`sun.net.ext.ExtendedSocketOptions#register` is marked `public`, the comment on
that method and the fact that it resides in an internal, not exposed by default
class/module, makes me believe that this `register` method isn't supposed to be
called by anyone other than the `jdk.net.ExtendedSocketOptions`. If at all this
`register` method is allowed to be called from other places, then due to the
change in this PR, additional work needs to be probably done in its
implementation to allow for the `jdk.net.ExtendedSocketOptions` to be given
first priority(?) to be registered first. I'll need input on whether I should
worry about this case or if it's fine in its current form in this PR.
>>
>> This PR also contains a jtreg test which reproduces the issue and verifies
>> the fix.
>
> Jaikiran Pai has updated the pull request incrementally with three additional
> commits since the last revision:
>
> - Incorporate the review suggestion to run the test multiple times to
> improve the chances of reproducing any potential deadlock
> - Incorporate the review suggestion to use @modules instead of --add-exports
> option while launching the test
> - Fix copyright message on test
src/java.base/share/classes/sun/net/ext/ExtendedSocketOptions.java line 171:
> 169:
> 170: public static ExtendedSocketOptions getInstance() {
> 171: if (instance != null) {
May be it's worth to avoid reading `volatile` field twice?
-
PR: https://git.openjdk.java.net/jdk/pull/2601
Re: RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy [v12]
> 8080272 Refactor I/O stream copying to use > InputStream.transferTo/readAllBytes and Files.copy Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision: 8080272: Refactor I/O stream copying to use java.io.InputStream.transferTo cleanup - Changes: - all: https://git.openjdk.java.net/jdk/pull/1853/files - new: https://git.openjdk.java.net/jdk/pull/1853/files/6e71e961..1b30471d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1853&range=11 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1853&range=10-11 Stats: 7 lines in 2 files changed: 0 ins; 5 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/1853.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1853/head:pull/1853 PR: https://git.openjdk.java.net/jdk/pull/1853
Re: RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy [v9]
On Tue, 9 Feb 2021 11:40:09 GMT, Philippe Marschall
wrote:
>> Andrey Turbanov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8080272: Refactor I/O stream copying to use java.io.InputStream.transferTo
>> fix review comments
>
> src/java.base/share/classes/java/util/jar/JarInputStream.java line 93:
>
>> 91: if (e != null &&
>> JarFile.MANIFEST_NAME.equalsIgnoreCase(e.getName())) {
>> 92: man = new Manifest();
>> 93: byte[] bytes = new BufferedInputStream(this).readAllBytes();
>
> I wonder if it makes sense to avoid reading the entire manifest into a byte[]
> when we don't need to verify the JAR. This may help avoiding some
> intermediate allocation and copying. This make be noticeable for some of the
> larger manifests (Java EE, OSGi, ...). A possible implementation may look
> like this
> https://github.com/marschall/jdk/commit/c50880ffb18607077c4da3456b27957d1df8edb7.
>
> In either case since we're calling #readAllBytes I don't see why we are
> wrapping in a BufferedInputStream rather than calling #readAllBytes directly.
Usage of `BufferedInputStream` removed
-
PR: https://git.openjdk.java.net/jdk/pull/1853
Re: RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy [v9]
On Mon, 8 Feb 2021 21:18:44 GMT, Philippe Marschall wrote: >> Andrey Turbanov has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8080272: Refactor I/O stream copying to use java.io.InputStream.transferTo >> fix review comments > > src/java.base/share/classes/sun/security/provider/certpath/X509CertPath.java > line 230: > >> 228: // Copy the entire input stream into an InputStream >> that does >> 229: // support mark >> 230: is = new ByteArrayInputStream(is.readAllBytes()); > > I don't understand why the check for #markSupported is done there. The > InputStream constructor of PKCS7 creates a DataInputStream on the InputStream > only to then call #readFully. I can't find a place where a call to #mark > happens. Since the InputStream constructor reads all bytes anyway I wonder > whether we could remove this if and unconditionally do: > > PKCS7 pkcs7 = new PKCS7(is.readAllBytes()); Good idea. Will improve. By the way, code in `sun.security.pkcs.PKCS7#PKCS7(java.io.InputStream)` looks suspicious: it reads only `InputStream.available()` bytes, which doesn't make much sense to me. - PR: https://git.openjdk.java.net/jdk/pull/1853
Re: RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy [v11]
> 8080272 Refactor I/O stream copying to use > InputStream.transferTo/readAllBytes and Files.copy Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision: 8080272: Refactor I/O stream copying to use java.io.InputStream.transferTo remove unnecessary file.exists() check - Changes: - all: https://git.openjdk.java.net/jdk/pull/1853/files - new: https://git.openjdk.java.net/jdk/pull/1853/files/6614a10f..6e71e961 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1853&range=10 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1853&range=09-10 Stats: 3 lines in 1 file changed: 0 ins; 2 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/1853.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1853/head:pull/1853 PR: https://git.openjdk.java.net/jdk/pull/1853
Re: RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy [v10]
On Mon, 15 Feb 2021 19:23:16 GMT, Alan Bateman wrote:
>> Andrey Turbanov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8080272: Refactor I/O stream copying to use java.io.InputStream.transferTo
>> revert changes from MimeLauncher
>
> src/java.management/share/classes/javax/management/loading/MLet.java line
> 1149:
>
>> 1147: file.deleteOnExit();
>> 1148: Files.copy(is, file.toPath(),
>> StandardCopyOption.REPLACE_EXISTING);
>> 1149: if (file.exists()) {
>
> You might have missed the comment from a previous iteration. The
> files.exists() check goes away when Files.copy succeeds.
You are right. Fixed.
-
PR: https://git.openjdk.java.net/jdk/pull/1853
Re: RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy [v10]
> 8080272 Refactor I/O stream copying to use > InputStream.transferTo/readAllBytes and Files.copy Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision: 8080272: Refactor I/O stream copying to use java.io.InputStream.transferTo revert changes from MimeLauncher - Changes: - all: https://git.openjdk.java.net/jdk/pull/1853/files - new: https://git.openjdk.java.net/jdk/pull/1853/files/6a8a3ae6..6614a10f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1853&range=09 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1853&range=08-09 Stats: 19 lines in 1 file changed: 11 ins; 0 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/1853.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1853/head:pull/1853 PR: https://git.openjdk.java.net/jdk/pull/1853
Re: RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy [v8]
On Sat, 13 Feb 2021 10:20:29 GMT, Andrey Turbanov wrote: >> ## tools/jpackage/share/jdk/jpackage/tests/UnicodeArgsTest.java >> >> make test >> TEST="jtreg:tools/jpackage/share/jdk/jpackage/tests/UnicodeArgsTest.java" >> >> STDOUT: >> [00:56:54.598] Parsing [--jpt-run=jdk.jpackage.tests.UnicodeArgsTest]... >> [00:56:54.650] jdk.jpackage.tests.UnicodeArgsTest.test8246042 -> [public >> void jdk.jpackage.tests.UnicodeArgsTest.test8246042(boolean)] >> [00:56:54.682] Create: UnicodeArgsTest.test8246042(true) >> [00:56:54.684] Create: UnicodeArgsTest.test8246042(false) >> [00:56:54.688] [ RUN ] UnicodeArgsTest.test8246042(false) >> [00:56:54.693] TRACE: Test string code points: [0x00e9] >> [00:56:54.875] TRACE: exec: Execute tool provider [javac -d >> .\test8246042.9782d070\jar-workdir >> F:\Projects\official_openjdk\test\jdk\tools\jpackage\apps\image\Hello.java](4)... >> [00:56:55.996] TRACE: exec: Done. Exit code: 0 >> [00:56:55.997] TRACE: assertEquals(0): Check command tool provider >> [javac -d .\test8246042.9782d070\jar-workdir >> F:\Projects\official_openjdk\test\jdk\tools\jpackage\apps\image\Hello.java](4) >> exited with 0 code >> [00:56:56.014] TRACE: exec: Execute tool provider [jar -c -f >> .\test8246042.9782d070\input\hello.jar -C .\test8246042.9782d070\jar-workdir >> .](7)... >> [00:56:56.188] TRACE: exec: Done. Exit code: 0 >> [00:56:56.188] TRACE: assertEquals(0): Check command tool provider [jar >> -c -f .\test8246042.9782d070\input\hello.jar -C >> .\test8246042.9782d070\jar-workdir .](7) exited with 0 code >> [00:56:56.196] TRACE: exec: Execute tool provider [jpackage --input >> .\test8246042.9782d070\input --dest .\test8246042.9782d070\output --name >> 8246042UnicodeArgsTest --type app-image --main-jar hello.jar --main-class >> Hello --win-console --arguments ? --verbose](17)... >> [00:56:56.225] Creating app package: 8246042UnicodeArgsTest in >> F:\Projects\official_openjdk\build\windows-x86_64-server-release\test-support\jtreg_test_jdk_tools_jpackage_share_jdk_jpackage_tests_UnicodeArgsTest_java\scratch\0.\test8246042.9782d070\output >> [00:57:07.680] Command: >> jlink --output >> .\test8246042.9782d070\output\8246042UnicodeArgsTest\runtime --module-path >> f:\\projects\\official_openjdk\\build\\windows-x86_64-server-release\\images\\jdk\\jmods >> --add-modules >> jdk.management.jfr,java.rmi,jdk.jdi,jdk.charsets,java.xml,jdk.xml.dom,java.datatransfer,jdk.jstatd,jdk.httpserver,java.desktop,java.security.sasl,jdk.zipfs,java.base,jdk.crypto.ec,jdk.javadoc,jdk.management.agent,jdk.jshell,jdk.editpad,java.sql.rowset,jdk.jsobject,jdk.sctp,java.smartcardio,jdk.jlink,jdk.unsupported,java.security.jgss,java.compiler,jdk.nio.mapmode,jdk.dynalink,jdk.unsupported.desktop,jdk.accessibility,jdk.security.jgss,java.sql,jdk.incubator.vector,java.xml.crypto,java.logging,java.transaction.xa,jdk.jfr,jdk.crypto.cryptoki,jdk.net,java.naming,jdk.internal.ed,java.prefs,java.net.http,jdk.compiler,jdk.naming.rmi,jdk.internal.opt,jdk.jconsole,jdk.attach,jdk.crypto.mscapi,jdk.internal.le,java.management,jdk.jdwp.agent,jdk.internal.jvmstat,jdk.incubator.foreign,java.instr ument,jdk.management,jdk.security.auth,java.scripting,jdk.jdeps,jdk.jartool,java.management.rmi,jdk.jpackage,jdk.naming.dns,jdk.localedata --strip-native-commands --strip-debug --no-man-pages --no-header-files >> [00:57:07.680] Output: >> WARNING: Using incubator modules: jdk.incubator.vector, >> jdk.incubator.foreign >> >> [00:57:07.681] Returned: 0 >> >> [00:57:07.685] Using default package resource java48.ico [icon] (add >> 8246042UnicodeArgsTest.ico to the resource-dir to customize). >> [00:57:07.707] Warning: Windows Defender may prevent jpackage from >> functioning. If there is an issue, it can be addressed by either disabling >> realtime monitoring, or adding an exclusion for the directory >> "f:\projects\official_openjdk\build\windows-x86_64-server-release\test-support\jtreg_test_jdk_tools_jpackage_share_jdk_jpackage_tests_UnicodeArgsTest_java\tmp\jdk.jpackage16485063537873697224". >> [00:57:07.712] Using default package resource WinLauncher.template >> [Template for creating executable properties file] (add >> 8246042UnicodeArgsTest.properties to the resource-dir to customize). >> [00:57:07.746] Succeeded in building Windows Application Image package >> [00:57:07.748] TRACE: exec: Done. Exit code: 0 >> [00:57:07.748] TRACE: assertEquals(0): Check command tool provider >> [jpackage --input .\test8246042.9782d070\input -
Re: RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy [v8]
On Fri, 12 Feb 2021 22:12:29 GMT, Andrey Turbanov wrote: >> ## java/security/AccessController/DoPrivAccompliceTest.java >> >> make test >> TEST="jtreg:java/security/AccessController/DoPrivAccompliceTest.java" >> >> STDOUT: >> Adding DoPrivAccomplice.class to >> F:\Projects\official_openjdk\build\windows-x86_64-server-release\test-support\jtreg_test_jdk_java_security_AccessController_DoPrivAccompliceTest_java\scratch\0.\DoPrivAccomplice.jar >> >> Created jar file >> F:\Projects\official_openjdk\build\windows-x86_64-server-release\test-support\jtreg_test_jdk_java_security_AccessController_DoPrivAccompliceTest_java\scratch\0.\DoPrivAccomplice.jar >> Adding DoPrivTest.class to >> F:\Projects\official_openjdk\build\windows-x86_64-server-release\test-support\jtreg_test_jdk_java_security_AccessController_DoPrivAccompliceTest_java\scratch\0.\DoPrivTest.jar >> >> Created jar file >> F:\Projects\official_openjdk\build\windows-x86_64-server-release\test-support\jtreg_test_jdk_java_security_AccessController_DoPrivAccompliceTest_java\scratch\0.\DoPrivTest.jar >> Created policy for >> F:\Projects\official_openjdk\build\windows-x86_64-server-release\test-support\jtreg_test_jdk_java_security_AccessController_DoPrivAccompliceTest_java\scratch\0.\DoPrivAccomplice.jar >> Command line: >> [f:\projects\official_openjdk\build\windows-x86_64-server-release\images\jdk\bin\java.exe >> -cp >> F:\Projects\official_openjdk\build\windows-x86_64-server-release\test-support\jtreg_test_jdk_java_security_AccessController_DoPrivAccompliceTest_java\classes\0\java\security\AccessController\DoPrivAccompliceTest.d;F:\Projects\official_openjdk\test\jdk\java\security\AccessController;F:\Projects\official_openjdk\build\windows-x86_64-server-release\test-support\jtreg_test_jdk_java_security_AccessController_DoPrivAccompliceTest_java\classes\0\test\lib;F:\Projects\official_openjdk\test\lib;C:\Programs\jtreg-4.2.0-tip\jtreg\lib\javatest.jar;C:\Programs\jtreg-4.2.0-tip\jtreg\lib\jtreg.jar >> -Xmx512m -XX:MaxRAMPercentage=6 >> -Djava.io.tmpdir=f:\projects\official_openjdk\build\windows-x86_64-server-release\test-support\jtreg_test_jdk_java_security_AccessController_DoPrivAccompliceTest_java\tmp >> -ea -esa -Djava.security.manager >> -Djava.security.policy=F:\Projects\official_openjdk\build\ windows-x86_64-server-release\test-support\jtreg_test_jdk_java_security_AccessController_DoPrivAccompliceTest_java\scratch\0.\java.policy -classpath F:\Projects\official_openjdk\build\windows-x86_64-server-release\test-support\jtreg_test_jdk_java_security_AccessController_DoPrivAccompliceTest_java\scratch\0.\DoPrivAccomplice.jar;F:\Projects\official_openjdk\build\windows-x86_64-server-release\test-support\jtreg_test_jdk_java_security_AccessController_DoPrivAccompliceTest_java\scratch\0.\DoPrivTest.jar DoPrivTest ] >> [2021-02-12T21:42:29.297091800Z] Gathering output for process 12712 >> [2021-02-12T21:42:29.544092Z] Waiting for completion for process 12712 >> [2021-02-12T21:42:29.544092Z] Waiting for completion finished for >> process 12712 >> Output and diagnostic info for process 12712 was saved into >> 'pid-12712-output.log' >> [2021-02-12T21:42:29.547092500Z] Waiting for completion for process 12712 >> [2021-02-12T21:42:29.547092500Z] Waiting for completion finished for >> process 12712 >> Created policy for >> F:\Projects\official_openjdk\build\windows-x86_64-server-release\test-support\jtreg_test_jdk_java_security_AccessController_DoPrivAccompliceTest_java\scratch\0.\DoPrivTest.jar >> Command line: >> [f:\projects\official_openjdk\build\windows-x86_64-server-release\images\jdk\bin\java.exe >> -cp >> F:\Projects\official_openjdk\build\windows-x86_64-server-release\test-support\jtreg_test_jdk_java_security_AccessController_DoPrivAccompliceTest_java\classes\0\java\security\AccessController\DoPrivAccompliceTest.d;F:\Projects\official_openjdk\test\jdk\java\security\AccessController;F:\Projects\official_openjdk\build\windows-x86_64-server-release\test-support\jtreg_test_jdk_java_security_AccessController_DoPrivAccompliceTest_java\classes\0\test\lib;F:\Projects\official_openjdk\test\lib;C:\Programs\jtreg-4.2.0-tip\jtreg\lib\javatest.jar;C:\Programs\jtreg-4.2.0-tip\jtreg\lib\jtreg.jar >> -Xmx512m -XX:MaxRAMPercentage=6 >> -Djava.io.tmpdir=f:\projects\official_openjdk\build\windows-x86_64-server-release\test-support\jtreg_test_jdk_java_security_AccessController_DoPrivAccompliceTest_java\tmp >> -ea -esa -Djava.security.manager >> -Djava.security.policy=F:\Projects\official_openjdk\build\ windows-x86_64-server-release\test-support\jtreg_test_jdk_java_security_AccessController_DoP
Re: RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy [v8]
On Fri, 12 Feb 2021 21:53:13 GMT, Andrey Turbanov
wrote:
>> ## java/nio/file/Files/CopyAndMove.java
>>
>> make test TEST="jtreg:java/nio/file/Files/CopyAndMove.java"
>>
>> STDOUT:
>> Seed from RandomFactory = 704532001916725417L
>> STDERR:
>> dir1:
>> f:\projects\official_openjdk\build\windows-x86_64-server-release\test-support\jtreg_test_jdk_java_nio_file_Files_CopyAndMove_java\tmp\name9678927043623070601
>> (NTFS)
>> dir2: .\name1900089232270637553 (NTFS)
>> java.lang.RuntimeException: AtomicMoveNotSupportedException expected
>> at CopyAndMove.testMove(CopyAndMove.java:369)
>> at CopyAndMove.main(CopyAndMove.java:74)
>> at
>> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
>> Method)
>> at
>> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
>> at
>> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>> at java.base/java.lang.reflect.Method.invoke(Method.java:566)
>> at
>> com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127)
>> at java.base/java.lang.Thread.run(Thread.java:831)
>>
>> JavaTest Message: Test threw exception: java.lang.RuntimeException:
>> AtomicMoveNotSupportedException expected
>> JavaTest Message: shutting down test
>>
>> STATUS:Failed.`main' threw exception: java.lang.RuntimeException:
>> AtomicMoveNotSupportedException expected
>>
>> Checked in debugger:
>>
>> Files.getFileStore(dir1) = {WindowsFileStore@1211} "ssd (f:)"
>> Files.getFileStore(dir2) = {WindowsFileStore@1213} "ssd (F:)"
>> sameDevice = false
>>
>> https://bugs.openjdk.java.net/browse/JDK-8219644 looks like there is already
>> known bug this test.
>
> ## java/security/AccessController/DoPrivAccompliceTest.java
>
> make test
> TEST="jtreg:java/security/AccessController/DoPrivAccompliceTest.java"
>
> STDOUT:
> Adding DoPrivAccomplice.class to
> F:\Projects\official_openjdk\build\windows-x86_64-server-release\test-support\jtreg_test_jdk_java_security_AccessController_DoPrivAccompliceTest_java\scratch\0.\DoPrivAccomplice.jar
>
> Created jar file
> F:\Projects\official_openjdk\build\windows-x86_64-server-release\test-support\jtreg_test_jdk_java_security_AccessController_DoPrivAccompliceTest_java\scratch\0.\DoPrivAccomplice.jar
> Adding DoPrivTest.class to
> F:\Projects\official_openjdk\build\windows-x86_64-server-release\test-support\jtreg_test_jdk_java_security_AccessController_DoPrivAccompliceTest_java\scratch\0.\DoPrivTest.jar
>
> Created jar file
> F:\Projects\official_openjdk\build\windows-x86_64-server-release\test-support\jtreg_test_jdk_java_security_AccessController_DoPrivAccompliceTest_java\scratch\0.\DoPrivTest.jar
> Created policy for
> F:\Projects\official_openjdk\build\windows-x86_64-server-release\test-support\jtreg_test_jdk_java_security_AccessController_DoPrivAccompliceTest_java\scratch\0.\DoPrivAccomplice.jar
> Command line:
> [f:\projects\official_openjdk\build\windows-x86_64-server-release\images\jdk\bin\java.exe
> -cp
> F:\Projects\official_openjdk\build\windows-x86_64-server-release\test-support\jtreg_test_jdk_java_security_AccessController_DoPrivAccompliceTest_java\classes\0\java\security\AccessController\DoPrivAccompliceTest.d;F:\Projects\official_openjdk\test\jdk\java\security\AccessController;F:\Projects\official_openjdk\build\windows-x86_64-server-release\test-support\jtreg_test_jdk_java_security_AccessController_DoPrivAccompliceTest_java\classes\0\test\lib;F:\Projects\official_openjdk\test\lib;C:\Programs\jtreg-4.2.0-tip\jtreg\lib\javatest.jar;C:\Programs\jtreg-4.2.0-tip\jtreg\lib\jtreg.jar
> -Xmx512m -XX:MaxRAMPercentage=6
> -Djava.io.tmpdir=f:\projects\official_openjdk\build\windows-x86_64-server-release\test-support\jtreg_test_jdk_java_security_AccessController_DoPrivAccompliceTest_java\tmp
> -ea -esa -Djava.security.manager
> -Djava.security.policy=F:\Projects\official_openjdk\build\w
indows-x86_64-server-release\test-support\jtreg_test_jdk_java_security_AccessController_DoPrivAccompliceTest_java\scratch\0.\java.policy
-classpath
F:\Projects\official_openjdk\build\windows-x86_64-server-release\test-support\jtreg_test_jdk_java_security_AccessController_DoPrivAccompliceTest_java\scratch\0.\DoPrivAccomplice.jar;F:\Projects\official_openjdk\build\windows-x86_64-server-release\test-support\jtreg_test_jdk_java_security_AccessController_DoPrivAccompliceTest_jav
Re: RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy [v8]
On Fri, 12 Feb 2021 21:32:04 GMT, Andrey Turbanov
wrote:
>> ## java/net/MulticastSocket/SetLoopbackMode.java
>>
>> make test
>> TEST="jtreg:test/jdk/java/net/MulticastSocket/SetLoopbackMode.java"
>>
>>
>> STDOUT:
>> IPv6 can be used
>> Default network interface: null
>>
>> Test will use multicast group: /ff01:0:0:0:0:0:0:1
>> NetworkInterface.getByInetAddress(grp): null
>> STDERR:
>> java.net.NoRouteToHostException: No route to host: no further information
>> at java.base/sun.nio.ch.Net.joinOrDrop6(Native Method)
>> at java.base/sun.nio.ch.Net.join6(Net.java:734)
>> at
>> java.base/sun.nio.ch.DatagramChannelImpl.innerJoin(DatagramChannelImpl.java:1515)
>> at
>> java.base/sun.nio.ch.DatagramChannelImpl.join(DatagramChannelImpl.java:1551)
>> at
>> java.base/sun.nio.ch.DatagramSocketAdaptor.joinGroup(DatagramSocketAdaptor.java:532)
>> at
>> java.base/sun.nio.ch.DatagramSocketAdaptor.joinGroup(DatagramSocketAdaptor.java:479)
>> at
>> java.base/java.net.MulticastSocket.joinGroup(MulticastSocket.java:318)
>> at SetLoopbackMode.main(SetLoopbackMode.java:132)
>> at
>> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
>> Method)
>> at
>> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
>> at
>> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>> at java.base/java.lang.reflect.Method.invoke(Method.java:566)
>> at
>> com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127)
>> at java.base/java.lang.Thread.run(Thread.java:831)
>>
>> JavaTest Message: Test threw exception: java.net.NoRouteToHostException:
>> No route to host: no further information
>> JavaTest Message: shutting down test
>>
>> STATUS:Failed.`main' threw exception: java.net.NoRouteToHostException:
>> No route to host: no further information
>>
>> Cause looks similar to `MulticastAddresses`: virtualbox network interface:
>> Test: /ff01:0:0:0:0:0:0:1 ni: name:eth10 (VirtualBox Host-Only Ethernet
>> Adapter)
>> joinGroup(InetAddress) Failed: No route to host: no further information
>> Will investigate futher.
>
> ## java/nio/file/Files/CopyAndMove.java
>
> make test TEST="jtreg:java/nio/file/Files/CopyAndMove.java"
>
> STDOUT:
> Seed from RandomFactory = 704532001916725417L
> STDERR:
> dir1:
> f:\projects\official_openjdk\build\windows-x86_64-server-release\test-support\jtreg_test_jdk_java_nio_file_Files_CopyAndMove_java\tmp\name9678927043623070601
> (NTFS)
> dir2: .\name1900089232270637553 (NTFS)
> java.lang.RuntimeException: AtomicMoveNotSupportedException expected
> at CopyAndMove.testMove(CopyAndMove.java:369)
> at CopyAndMove.main(CopyAndMove.java:74)
> at
> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> at
> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
> at
> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> at java.base/java.lang.reflect.Method.invoke(Method.java:566)
> at
> com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127)
> at java.base/java.lang.Thread.run(Thread.java:831)
>
> JavaTest Message: Test threw exception: java.lang.RuntimeException:
> AtomicMoveNotSupportedException expected
> JavaTest Message: shutting down test
>
> STATUS:Failed.`main' threw exception: java.lang.RuntimeException:
> AtomicMoveNotSupportedException expected
>
> Checked in debugger:
>
> Files.getFileStore(dir1) = {WindowsFileStore@1211} "ssd (f:)"
> Files.getFileStore(dir2) = {WindowsFileStore@1213} "ssd (F:)"
> sameDevice = false
>
> https://bugs.openjdk.java.net/browse/JDK-8219644 looks like there is already
> known bug this test.
## java/security/AccessController/DoPrivAccompliceTest.java
make test
TEST="jtreg:java/security/AccessController/DoPrivAccompliceTest.java"
STDOUT:
Adding DoPrivAccomplice.class to
F:\Projects\official_openjdk\build\windows-x86_64-server-release\test-support\jtreg_test_jdk_java_security_AccessController_DoP
Re: RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy [v8]
On Fri, 12 Feb 2021 21:12:14 GMT, Andrey Turbanov wrote: >> ## java/net/MulticastSocket/MulticastAddresses.java >> >> make test >> TEST="jtreg:test/jdk/java/net/MulticastSocket/MulticastAddresses.java" >> >> STDOUT: >> Test: /224.80.80.80 ni: name:eth1 (PANGP Virtual Ethernet Adapter) >> joinGroup(InetAddress) Passed. >> joinGroup(InetAddress,NetworkInterface) Passed. >> Test: /129.1.1.1 >> joinGroup(InetAddress) >> Passed: Not a multicast address >> Test: /ff01:0:0:0:0:0:0:1 ni: name:eth10 (VirtualBox Host-Only Ethernet >> Adapter) >> joinGroup(InetAddress) Failed: No route to host: no further >> information >> Test: /ff02:0:0:0:0:0:0:1234 ni: name:eth10 (VirtualBox Host-Only >> Ethernet Adapter) >> joinGroup(InetAddress) Passed. >> joinGroup(InetAddress,NetworkInterface) Passed. >> Test: /ff05:0:0:0:0:0:0:a ni: name:eth10 (VirtualBox Host-Only Ethernet >> Adapter) >> joinGroup(InetAddress) Passed. >> joinGroup(InetAddress,NetworkInterface) Passed. >> Test: /ff0e:0:0:0:0:0:1234:a ni: name:eth10 (VirtualBox Host-Only >> Ethernet Adapter) >> joinGroup(InetAddress) Passed. >> joinGroup(InetAddress,NetworkInterface) Passed. >> Test: /0:0:0:0:0:0:0:1 >> joinGroup(InetAddress) >> Passed: Not a multicast address >> Test: /0:0:0:0:0:0:8101:101 >> joinGroup(InetAddress) >> Passed: Not a multicast address >> Test: /fe80:0:0:0:a00:20ff:fee5:bc02 >> joinGroup(InetAddress) >> Passed: Not a multicast address >> STDERR: >> java.lang.Exception: 1 test(s) failed - see log file. >> at MulticastAddresses.runTest(MulticastAddresses.java:93) >> at MulticastAddresses.main(MulticastAddresses.java:138) >> at >> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native >> Method) >> at >> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78) >> at >> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) >> at java.base/java.lang.reflect.Method.invoke(Method.java:566) >> at >> com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:312) >> at java.base/java.lang.Thread.run(Thread.java:831) >> >> JavaTest Message: Test threw exception: java.lang.Exception >> JavaTest Message: shutting down test >> >> >> TEST RESULT: Failed. Execution failed: `main' threw exception: >> java.lang.Exception: 1 test(s) failed - see log file. >> >> >> I connected debbuger and got this stack trace: >> >> java.net.NoRouteToHostException: No route to host: no further information >> at java.base/sun.nio.ch.Net.joinOrDrop6(Native Method) >> at java.base/sun.nio.ch.Net.join6(Net.java:734) >> at >> java.base/sun.nio.ch.DatagramChannelImpl.innerJoin(DatagramChannelImpl.java:1515) >> at >> java.base/sun.nio.ch.DatagramChannelImpl.join(DatagramChannelImpl.java:1551) >> at >> java.base/sun.nio.ch.DatagramSocketAdaptor.joinGroup(DatagramSocketAdaptor.java:532) >> at >> java.base/sun.nio.ch.DatagramSocketAdaptor.joinGroup(DatagramSocketAdaptor.java:479) >> at >> java.base/java.net.MulticastSocket.joinGroup(MulticastSocket.java:318) >> at MulticastAddresses.runTest(MulticastAddresses.java:56) >> at MulticastAddresses.main(MulticastAddresses.java:138) >> at >> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native >> Method) >> at >> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78) >> at >> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) >> at java.base/java.lang.reflect.Method.invoke(Method.java:566) >> at >> com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:312) >> at java.base/java.lang.Thread.run(Thread.java:831) >> >> Not sure what actual cause. Will investigate further. > > ## java/net/MulticastSocket/SetLoopbackMode.java > > make test > TEST="jtreg:test/jdk/java/net/MulticastSock
Re: RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy [v8]
On Fri, 12 Feb 2021 21:06:24 GMT, Andrey Turbanov wrote: >> Then I tried to run tests separately: >> ## java/io/File/GetXSpace.java >> >> >> make test TEST="jtreg:test/jdk/java/io/File/GetXSpace.java" >> >> STDERR: >> java.nio.file.InvalidPathException: Illegal char <:> at index 0: : >> at >> java.base/sun.nio.fs.WindowsPathParser.normalize(WindowsPathParser.java:182) >> at >> java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:153) >> at >> java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:77) >> at java.base/sun.nio.fs.WindowsPath.parse(WindowsPath.java:92) >> at >> java.base/sun.nio.fs.WindowsFileSystem.getPath(WindowsFileSystem.java:230) >> at java.base/java.io.File.toPath(File.java:2316) >> at GetXSpace.compare(GetXSpace.java:219) >> at GetXSpace.testDF(GetXSpace.java:394) >> at GetXSpace.main(GetXSpace.java:428) >> at >> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native >> Method) >> at >> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78) >> at >> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) >> at java.base/java.lang.reflect.Method.invoke(Method.java:566) >> at >> com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:312) >> at java.base/java.lang.Thread.run(Thread.java:831) >> >> JavaTest Message: Test threw exception: >> java.nio.file.InvalidPathException >> JavaTest Message: shutting down test >> >> STDOUT: >> --- Testing df >> C:/Programs/cygwin64 350809332 172573816 178235516 50% / >> D:3702215676 2812548988 88988 76% >> /cygdrive/d >> E:3906885628 3544182676 362702952 91% >> /cygdrive/e >> F: 250057724 240917056 9140668 97% >> /cygdrive/f >> >> >> SecurityManager = null >> C:/Programs/cygwin64: >> df total= 359228755968 free =0 usable = 182513168384 >> getX total= 359228755968 free = 182513168384 usable = 182513168384 >> :: >> df total= 3791068852224 free =0 usable = 911018688512 >> getX total=0 free =0 usable =0 >> >> TEST RESULT: Failed. Execution failed: `main' threw exception: >> java.nio.file.InvalidPathException: Illegal char <:> at index 0: : >> -- >> >> >> https://bugs.openjdk.java.net/browse/JDK-8251466 looks like there is already >> known bug for similar cygwin output. > > ## java/net/MulticastSocket/MulticastAddresses.java > > make test > TEST="jtreg:test/jdk/java/net/MulticastSocket/MulticastAddresses.java" > > STDOUT: > Test: /224.80.80.80 ni: name:eth1 (PANGP Virtual Ethernet Adapter) > joinGroup(InetAddress) Passed. > joinGroup(InetAddress,NetworkInterface) Passed. > Test: /129.1.1.1 > joinGroup(InetAddress) > Passed: Not a multicast address > Test: /ff01:0:0:0:0:0:0:1 ni: name:eth10 (VirtualBox Host-Only Ethernet > Adapter) > joinGroup(InetAddress) Failed: No route to host: no further > information > Test: /ff02:0:0:0:0:0:0:1234 ni: name:eth10 (VirtualBox Host-Only > Ethernet Adapter) > joinGroup(InetAddress) Passed. > joinGroup(InetAddress,NetworkInterface) Passed. > Test: /ff05:0:0:0:0:0:0:a ni: name:eth10 (VirtualBox Host-Only Ethernet > Adapter) > joinGroup(InetAddress) Passed. > joinGroup(InetAddress,NetworkInterface) Passed. > Test: /ff0e:0:0:0:0:0:1234:a ni: name:eth10 (VirtualBox Host-Only > Ethernet Adapter) > joinGroup(InetAddress) Passed. > joinGroup(InetAddress,NetworkInterface) Passed. > Test: /0:0:0:0:0:0:0:1 > joinGroup(InetAddress) > Passed: Not a multicast address > Test: /0:0:0:0:0:0:8101:101 > joinGroup(InetAddress) > Passed: Not a multicast address > Test: /fe80:0:0:0:a00:20ff:fee5:bc02 > joinGroup(InetAddress) > Passed: Not a multicast address > STDERR: > java.lang.Exception: 1 test(s) failed - see log file. >
Re: RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy [v8]
On Mon, 8 Feb 2021 16:39:55 GMT, Julia Boes wrote: >> The other security-related code changes look good to me. > > I've updated the issue summary to better reflect the changes, the PR summary > should be renamed accordingly. > As mentioned earlier, have you run the tests for the affected areas? Here's > some information on how to do that: > http://openjdk.java.net/guide/#testing-the-jdk I rebased my changes onto master. (commit 837bd8930d0a010110f1318b947c036609d3aa33) and checked tier2 and tier3. What I got: == Test summary == TEST TOTAL PASS FAIL ERROR >> jtreg:test/jdk:tier2 3698 3690 6 2 << >> jtreg:test/langtools:tier2 1211 1 0 << jtreg:test/jaxp:tier2 450 450 0 0 == TEST FAILURE == Test summary == TEST TOTAL PASS FAIL ERROR >> jtreg:test/jdk:tier3 1190 1188 2 0 << jtreg:test/langtools:tier30 0 0 0 jtreg:test/jaxp:tier3 0 0 0 0 == TEST FAILURE Failed tests: tier2: java/io/File/GetXSpace.java Failed. Execution failed: `main' threw exception: java.nio.file.InvalidPathException: Illegal char <:> at index 0: : java/net/MulticastSocket/MulticastAddresses.java Failed. Execution failed: `main' threw exception: java.lang.Exception: 1 test(s) failed - see log file. java/net/MulticastSocket/SetLoopbackMode.java Failed. Execution failed: `main' threw exception: java.net.NoRouteToHostException: No route to host: no further information java/nio/file/Files/CopyAndMove.java Failed. Execution failed: `main' threw exception: java.lang.RuntimeException: AtomicMoveNotSupportedException expected java/security/AccessController/DoPrivAccompliceTest.java Failed. Execution failed: `main' threw exception: java.lang.RuntimeException: 'user' found in stderr tools/jpackage/share/jdk/jpackage/tests/UnicodeArgsTest.java Failed. Execution failed: `main' threw exception: jdk.jpackage.test.Functional$ExceptionBox: java.lang.RuntimeException: 2 FAILED TESTS sun/security/tools/jarsigner/TimestampCheck.java Error. Agent error: java.lang.Exception: Agent 72 timed out with a timeout of 2400 seconds; check console log for any additional details sun/security/tools/keytool/DefaultOptions.java Error. Agent error: java.lang.Exception: Agent 77 timed out with a timeout of 480 seconds; check console log for any additional details jdk/jshell/ToolBasicTest.java Failed. Execution failed: `main' threw exception: java.lang.Exception: failures: 1 tier3: sanity/client/SwingSet/src/SwingSet2DemoTest.java Failed. Execution failed: `main' threw exception: java.lang.Exception: failures: 1 sanity/client/SwingSet/src/ColorChooserDemoTest.java Failed. Execution failed: `main' threw exception: java.lang.Exception: failures: 1 - PR: https://git.openjdk.java.net/jdk/pull/1853
Re: RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy [v8]
On Fri, 12 Feb 2021 21:03:04 GMT, Andrey Turbanov wrote: >> I've updated the issue summary to better reflect the changes, the PR summary >> should be renamed accordingly. >> As mentioned earlier, have you run the tests for the affected areas? Here's >> some information on how to do that: >> http://openjdk.java.net/guide/#testing-the-jdk > > I rebased my changes onto master. (commit > 837bd8930d0a010110f1318b947c036609d3aa33) > and checked tier2 and tier3. > What I got: > > == > Test summary > == >TEST TOTAL PASS FAIL > ERROR > >> jtreg:test/jdk:tier2 3698 3690 6 > 2 << > >> jtreg:test/langtools:tier2 1211 1 > 0 << >jtreg:test/jaxp:tier2 450 450 0 > 0 > == > TEST FAILURE > > > > > == > Test summary > == >TEST TOTAL PASS FAIL > ERROR > >> jtreg:test/jdk:tier3 1190 1188 2 > 0 << >jtreg:test/langtools:tier30 0 0 > 0 >jtreg:test/jaxp:tier3 0 0 0 > 0 > == > TEST FAILURE > > > Failed tests: > > tier2: > java/io/File/GetXSpace.java >Failed. Execution failed: `main' threw exception: > java.nio.file.InvalidPathException: Illegal char <:> at index 0: : > java/net/MulticastSocket/MulticastAddresses.java >Failed. Execution failed: `main' threw exception: > java.lang.Exception: 1 test(s) failed - see log file. > java/net/MulticastSocket/SetLoopbackMode.java >Failed. Execution failed: `main' threw exception: > java.net.NoRouteToHostException: No route to host: no further information > java/nio/file/Files/CopyAndMove.java >Failed. Execution failed: `main' threw exception: > java.lang.RuntimeException: AtomicMoveNotSupportedException expected > java/security/AccessController/DoPrivAccompliceTest.java >Failed. Execution failed: `main' threw exception: > java.lang.RuntimeException: 'user' found in stderr > tools/jpackage/share/jdk/jpackage/tests/UnicodeArgsTest.java >Failed. Execution failed: `main' threw exception: > jdk.jpackage.test.Functional$ExceptionBox: java.lang.RuntimeException: 2 > FAILED TESTS > > sun/security/tools/jarsigner/TimestampCheck.java >Error. Agent error: java.lang.Exception: Agent 72 > timed out with a timeout of 2400 seconds; check console log for any > additional details > sun/security/tools/keytool/DefaultOptions.java >Error. Agent error: java.lang.Exception: Agent 77 > timed out with a timeout of 480 seconds; check console log for any additional > details > > jdk/jshell/ToolBasicTest.java Failed. > Execution failed: `main' threw exception: java.lang.Exception: failures: 1 > > tier3: > sanity/client/SwingSet/src/SwingSet2DemoTest.java > Failed. Execution failed: `main' > threw exception: java.lang.Exception: failures: 1 > sanity/client/SwingSet/src/ColorChooserDemoTest.java > Failed. Execution failed: `main' > threw exception: java.lang.Exception: failures: 1 Then I tried to run tests separately: ## java/io/File/GetXSpace.java make test TEST="jtreg:test/jdk/java/io/File/GetXSpace.java" STDERR: java.nio.file.InvalidPathException: Illegal char <:> at index 0: : at java.base/sun.nio.fs.WindowsPathParser.normalize(WindowsPathParser.java:182) at java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:153) at java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:77) at java.base/sun.nio.fs.WindowsPath.parse(WindowsPath.ja
Re: RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy [v8]
On Fri, 12 Feb 2021 21:04:54 GMT, Andrey Turbanov wrote: >> I rebased my changes onto master. (commit >> 837bd8930d0a010110f1318b947c036609d3aa33) >> and checked tier2 and tier3. >> What I got: >> >> == >> Test summary >> == >>TEST TOTAL PASS FAIL >> ERROR >> >> jtreg:test/jdk:tier2 3698 3690 6 >> 2 << >> >> jtreg:test/langtools:tier2 1211 1 >> 0 << >>jtreg:test/jaxp:tier2 450 450 0 >> 0 >> == >> TEST FAILURE >> >> >> >> >> == >> Test summary >> == >>TEST TOTAL PASS FAIL >> ERROR >> >> jtreg:test/jdk:tier3 1190 1188 2 >> 0 << >>jtreg:test/langtools:tier30 0 0 >> 0 >>jtreg:test/jaxp:tier3 0 0 0 >> 0 >> == >> TEST FAILURE >> >> >> Failed tests: >> >> tier2: >> java/io/File/GetXSpace.java >> Failed. Execution failed: `main' threw >> exception: java.nio.file.InvalidPathException: Illegal char <:> at index 0: : >> java/net/MulticastSocket/MulticastAddresses.java >> Failed. Execution failed: `main' threw >> exception: java.lang.Exception: 1 test(s) failed - see log file. >> java/net/MulticastSocket/SetLoopbackMode.java >> Failed. Execution failed: `main' threw >> exception: java.net.NoRouteToHostException: No route to host: no further >> information >> java/nio/file/Files/CopyAndMove.java >> Failed. Execution failed: `main' threw >> exception: java.lang.RuntimeException: AtomicMoveNotSupportedException >> expected >> java/security/AccessController/DoPrivAccompliceTest.java >> Failed. Execution failed: `main' threw >> exception: java.lang.RuntimeException: 'user' found in stderr >> tools/jpackage/share/jdk/jpackage/tests/UnicodeArgsTest.java >> Failed. Execution failed: `main' threw >> exception: jdk.jpackage.test.Functional$ExceptionBox: >> java.lang.RuntimeException: 2 FAILED TESTS >> >> sun/security/tools/jarsigner/TimestampCheck.java >> Error. Agent error: java.lang.Exception: Agent >> 72 timed out with a timeout of 2400 seconds; check console log for any >> additional details >> sun/security/tools/keytool/DefaultOptions.java >> Error. Agent error: java.lang.Exception: Agent >> 77 timed out with a timeout of 480 seconds; check console log for any >> additional details >> >> jdk/jshell/ToolBasicTest.java Failed. >> Execution failed: `main' threw exception: java.lang.Exception: failures: 1 >> >> tier3: >> sanity/client/SwingSet/src/SwingSet2DemoTest.java >>Failed. Execution failed: `main' >> threw exception: java.lang.Exception: failures: 1 >> sanity/client/SwingSet/src/ColorChooserDemoTest.java >>Failed. Execution failed: `main' >> threw exception: java.lang.Exception: failures: 1 > > Then I tried to run tests separately: > ## java/io/File/GetXSpace.java > > > make test TEST="jtreg:test/jdk/java/io/File/GetXSpace.java" > > STDERR: > java.nio.file.InvalidPathException: Illegal char <:> at index 0: : > at > java.base/sun.nio.fs.WindowsPathParser.normalize(WindowsPathParser.java:182) > at > java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:153) > at > java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPath
Re: RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy [v9]
> 8080272 Refactor I/O stream copying to use > InputStream.transferTo/readAllBytes and Files.copy Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision: 8080272: Refactor I/O stream copying to use java.io.InputStream.transferTo fix review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/1853/files - new: https://git.openjdk.java.net/jdk/pull/1853/files/94e99817..6a8a3ae6 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1853&range=08 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1853&range=07-08 Stats: 29 lines in 10 files changed: 16 ins; 0 del; 13 mod Patch: https://git.openjdk.java.net/jdk/pull/1853.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1853/head:pull/1853 PR: https://git.openjdk.java.net/jdk/pull/1853
Re: RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy [v8]
On Mon, 8 Feb 2021 14:38:52 GMT, Weijun Wang wrote:
>> Andrey Turbanov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8080272: Refactor I/O stream copying to use java.io.InputStream.transferTo
>> revert changes in Apache Santuario
>
> src/java.xml.crypto/share/classes/org/jcp/xml/dsig/internal/dom/Utils.java
> line 49:
>
>> 47: throws IOException
>> 48: {
>> 49: return is.readAllBytes();
>
> This is also from Apache Santuario. It's better to keep it unchanged.
reverted
-
PR: https://git.openjdk.java.net/jdk/pull/1853
Re: RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy [v7]
On Mon, 8 Feb 2021 16:19:17 GMT, Julia Boes wrote: >> Andrey Turbanov 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/sun/security/provider/certpath/X509CertPath.java > line 29: > >> 27: >> 28: import java.io.ByteArrayInputStream; >> 29: import java.io.IOException; > > The copyright year needs to be updated for this file and changed to 2021 in > the other files where applicable. done - PR: https://git.openjdk.java.net/jdk/pull/1853
Re: RFR: 8080272 Refactor I/O stream copying to use java.io.InputStream.transferTo [v8]
On Mon, 8 Feb 2021 14:01:34 GMT, Alan Bateman wrote: >> Andrey Turbanov has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8080272: Refactor I/O stream copying to use java.io.InputStream.transferTo >> revert changes in Apache Santuario > > src/java.management/share/classes/javax/management/loading/MLet.java line > 1147: > >> 1145: .toFile(); >> 1146: file.deleteOnExit(); >> 1147: Files.copy(is, file.toPath()); > > One thing to check here is the existing behavior when the file already exists > (as Files.copy will fail if the file exists, need to specify REPLACE_EXISTING > to get the semantics of the existing code). > > The code that follows checks if the file exists, this will always be true > when Files.copy succeeds. fixed - PR: https://git.openjdk.java.net/jdk/pull/1853
