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
Re: HttpClient Send Method Guaranteed Completion
Hi Eliot,
Unless the server keeps the connection open, and fails to send all the
body bytes it has promised to send, the send operation should eventually
terminate. The behavior you describe looks indeed like a bug.
Could you please log a ticket with:
https://bugreport.java.com/bugreport/
That said - there is very little information here to work with.
It would be helpful to understand with which version of the protocol
this happened: HTTP or HTTPS? Version 1.1 or Version 2? Was it
with an upgrade request (h2c)?
If you manage to reproduce, it would be helpful if HTTP traces
could be enabled - I'd suggest:
`-Djdk.httpclient.HttpClient.log=headers,requests,errors`
(see https://docs.oracle.com/en/java/javase/17/core/java-networking.html)
Another issue here is that the request timeout set through
`HttpRequestBuilder::timeout` only runs until the response
headers are received. The reception of the body bytes is not
covered by this timeout. This is not well documented, and
we should probably do a better job there.
We have an enhancement request related to this:
https://bugs.openjdk.java.net/browse/JDK-8258397
However - setting up a global timeout for the request
is already possible:
If you wish to set a timeout for the reception of the body
bytes you can do so by making use of the timeout facility
provided by CompletableFuture:
https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/concurrent/CompletableFuture.html#get(long,java.util.concurrent.TimeUnit)
Instead of:
```
String html = httpClient.send(request,
HttpResponse.BodyHandlers.ofString()).body();
```
Use:
```
long timeout = 5000; // ms
String html;
CompletableFuture> cf;
try {
cf = httpClient.sendAsync(request,
HttpResponse.BodyHandlers.ofString());
html = cf.get(timeout, TimedUnit.MILLISECONDS)
.body();
} catch (TimeoutException x) {
// timeout: cancel request to free up
// any related resources
cf.cancel();
} catch (CancellationException x) {
// cancelled
} catch (InterruptedException x) {
// interrupted
} catch (ExecutionException x) {
// IO etc..
// unwrap to get exception and rethrow...
}
```
This offers the possibility to set up a global timeout
You can also cancel the request asynchronously at any
time by calling cf.cancel() (since Java 15 IIRC).
best regards,
-- daniel
On 05/10/2021 23:05, Elliot Barlas wrote:
Hello net-dev!
I'm emailing about a surprising observation in the HttpClient send
method related to guaranteed completion.
Despite explicit timeout configurations, a call to send can block
indefinitely. The stacktrace below was obtained from a thread dump on a
running OpenJDK 16.0.2 JVM. The thread was stuck in that state for over
a week. An application restart was required to recover. The related Java
source code is also included below.
My sense is that this is due to a missed notification from the async I/O
subsystem. Unfortunately, I'm not an expert on HttpClient internals, and
the asynchronous nature of the code makes it challenging to debug.
Is this the intended behavior? That is, are there scenarios in which
send should never return? Did I miss a socket timeout option?
-
HttpClient httpClient = HttpClient.newBuilder()
.version(HttpClient.Version.HTTP_1_1)
.connectTimeout(Duration.ofSeconds(5))
.build();
...
HttpRequest request = HttpRequest.newBuilder()
.timeout(Duration.ofSeconds(5))
.uri(URI.create("http://"; + ip + ":" + port + "/frontend"))
.build();
...
String html = httpClient.send(request,
HttpResponse.BodyHandlers.ofString()).body();
-
"pool-1-thread-70" #164 prio=5 os_prio=0 cpu=100038.29ms
elapsed=1891832.58s tid=0x7fe0211d4050 nid=0x29e waiting on
condition [0x7fdf025ac000]
java.lang.Thread.State: WAITING (parking)
at jdk.internal.misc.Unsafe.park(java.base@16.0.2/Native Method)
- parking to wait for <0xe7fc9be0> (a
java.util.concurrent.CompletableFuture$Signaller)
at
java.util.concurrent.locks.LockSupport.park(java.base@16.0.2/LockSupport.java:211)
at
java.util.concurrent.CompletableFuture$Signaller.block(java.base@16.0.2/CompletableFuture.java:1860)
at
java.util.concurrent.ForkJoinPool.managedBlock(java.base@16.0.2/ForkJoinPool.java:3137)
at
java.util.concurrent.CompletableFuture.waitingGet(java.base@16.0.2/CompletableFuture.java:1894)
at
java.util.concurrent.CompletableFuture.get(java.base@16.0.2/CompletableFuture.java:2068)
at
jdk.internal.net.http.HttpClientImpl.send(java.net.http@16.0.2/HttpClientImpl.java:535)
at
jdk.internal.net.http.HttpClientFacade.send(java.net.http@16.0.2/HttpClientFacade.java:119)
at
com.logmein.haproxy.ProxyFrontendConnectionCounter.countCurrentConnections(ProxyFrontendConnectionCounter.java:89)
at
com.logmein.haproxy.ProxyFrontendConnectionC
Re: RFR: 8233674: JarURLConnection.getJarFile throws Exception if some process is holding the file
On Fri, 10 Sep 2021 09:23:52 GMT, Masanori Yano wrote: > Could you please review the 8233674 bug fixes? > This problem is caused by the antivirus software opening the file for a short > time, so CreateFile() should be retried. I inquired of the Microsoft Technical Support about this problem. They said that using a simple retry mechanism is the effective workaround for this problem as in the article. But, they are not sure which is wrong the application or the virus checker because it depends on the situation. Which method do you think is better, to fix the JDK or to change the configuration of the virus checker? - PR: https://git.openjdk.java.net/jdk/pull/5460
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
Re: RFR: 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. The security related files look fine. - Marked as reviewed by mullan (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5454
Re: RFR: 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 The security related files look fine. - Marked as reviewed by mullan (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5818
Re: RFR: 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
Reviewed the crypto/security files.
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.
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.
-
Marked as reviewed by wetmore (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/5818
Re: RFR: 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
I checked the rest. The one BufferedInputStream change is puzzling. Please
explain or address.
Files like HttpTimestamper need the copyright dates updated to 2021.
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?
src/java.base/share/classes/sun/security/util/PolicyUtil.java line 156:
> 154: }
> 155:
> 156: try (InputStream inStream = new
> BufferedInputStream(getInputStream(keyStoreUrl))) {
Same comment as above.
-
Marked as reviewed by wetmore (Reviewer).
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
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:00:41 GMT, Bradford Wetmore wrote:
>> 8274809: Update java.base classes to use try-with-resources
>
> 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`.
-
PR: https://git.openjdk.java.net/jdk/pull/5818
Re: RFR: 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. Calendar-related changes look good to me. - Marked as reviewed by naoto (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5454
Re: RFR: 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. Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5454
Re: RFR: 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. Curious. The JDK build is done with javac -Xlint:cast warning enabled (JDK-8032734) which is intended to catch issues like this. Perhaps IntelliJ is using a different (or sharper) analysis. - PR: https://git.openjdk.java.net/jdk/pull/5454
Re: HttpClient Send Method Guaranteed Completion
Thanks for the reply, Daniel.
This was an HTTP 1.1 connection. The code snippet for building the URL and
configuring the HttpClient was included in my previous email for reference.
I can't make any claims about what was happening with packet transmissions at
the network level. It's entirely possible that the destination maintained the
TCP connection over an extended period without fulfilling its obligation to
send the body. But there must be a way to guarantee completion for such
scenarios. This was the function of socket timeouts in the world of synchronous
network APIs. In this new asynchronous world, it looks like that might have
been forgotten.
The alternative timeout code you mentioned addresses the problem in a narrow
sense, but it results in a resource leak. If the HTTP session is abandoned at
the application level via CompletableFuture timed wait, there is still a
footprint within HttpClient. Over time, socket handles, callback objects, and
other resources can accumulate with no visibility to application code.
From: Daniel Fuchs
Sent: Wednesday, October 6, 2021 2:42 AM
To: Elliot Barlas ; net-dev@openjdk.java.net
Subject: Re: HttpClient Send Method Guaranteed Completion
Hi Eliot,
Unless the server keeps the connection open, and fails to send all the
body bytes it has promised to send, the send operation should eventually
terminate. The behavior you describe looks indeed like a bug.
Could you please log a ticket with:
https://urldefense.com/v3/__https://bugreport.java.com/bugreport/__;!!OA8L0MA-!sPeWGu915xnLugfAz60R8t6hJfzTFK_1ZMX7cI0b-SgYHEDvWjnOSVCVmjsgoCLBZNA$
That said - there is very little information here to work with.
It would be helpful to understand with which version of the protocol
this happened: HTTP or HTTPS? Version 1.1 or Version 2? Was it
with an upgrade request (h2c)?
If you manage to reproduce, it would be helpful if HTTP traces
could be enabled - I'd suggest:
`-Djdk.httpclient.HttpClient.log=headers,requests,errors`
(see
https://urldefense.com/v3/__https://docs.oracle.com/en/java/javase/17/core/java-networking.html__;!!OA8L0MA-!sPeWGu915xnLugfAz60R8t6hJfzTFK_1ZMX7cI0b-SgYHEDvWjnOSVCVmjsgmoTmRXo$
)
Another issue here is that the request timeout set through
`HttpRequestBuilder::timeout` only runs until the response
headers are received. The reception of the body bytes is not
covered by this timeout. This is not well documented, and
we should probably do a better job there.
We have an enhancement request related to this:
https://urldefense.com/v3/__https://bugs.openjdk.java.net/browse/JDK-8258397__;!!OA8L0MA-!sPeWGu915xnLugfAz60R8t6hJfzTFK_1ZMX7cI0b-SgYHEDvWjnOSVCVmjsg46sP2Vg$
However - setting up a global timeout for the request
is already possible:
If you wish to set a timeout for the reception of the body
bytes you can do so by making use of the timeout facility
provided by CompletableFuture:
https://urldefense.com/v3/__https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/concurrent/CompletableFuture.html*get(long,java.util.concurrent.TimeUnit)__;Iw!!OA8L0MA-!sPeWGu915xnLugfAz60R8t6hJfzTFK_1ZMX7cI0b-SgYHEDvWjnOSVCVmjsgABT3nlI$
Instead of:
```
String html = httpClient.send(request,
HttpResponse.BodyHandlers.ofString()).body();
```
Use:
```
long timeout = 5000; // ms
String html;
CompletableFuture> cf;
try {
cf = httpClient.sendAsync(request,
HttpResponse.BodyHandlers.ofString());
html = cf.get(timeout, TimedUnit.MILLISECONDS)
.body();
} catch (TimeoutException x) {
// timeout: cancel request to free up
// any related resources
cf.cancel();
} catch (CancellationException x) {
// cancelled
} catch (InterruptedException x) {
// interrupted
} catch (ExecutionException x) {
// IO etc..
// unwrap to get exception and rethrow...
}
```
This offers the possibility to set up a global timeout
You can also cancel the request asynchronously at any
time by calling cf.cancel() (since Java 15 IIRC).
best regards,
-- daniel
On 05/10/2021 23:05, Elliot Barlas wrote:
> Hello net-dev!
>
> I'm emailing about a surprising observation in the HttpClient send
> method related to guaranteed completion.
>
> Despite explicit timeout configurations, a call to send can block
> indefinitely. The stacktrace below was obtained from a thread dump on a
> running OpenJDK 16.0.2 JVM. The thread was stuck in that state for over
> a week. An application restart was required to recover. The related Java
> source code is also included below.
>
> My sense is that this is due to a missed notification from the async I/O
> subsystem. Unfortunately, I'm not an expert on HttpClient internals, and
> the asynchronous nature of the code makes it challenging to debug.
>
> Is this the intended behavior? That is, are there scenarios in which
> send should never return? Did I miss a socket timeout option?
>
> -
>
>
Re: RFR: 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; I have sent this patch to our test system. I will come back with the results when they are available. - PR: https://git.openjdk.java.net/jdk/pull/5190
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 [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: Re: HttpClient Send Method Guaranteed Completion
Hi Eliot, On 06/10/2021 18:22, Elliot Barlas wrote: Thanks for the reply, Daniel. This was an HTTP 1.1 connection. The code snippet for building the URL and configuring the HttpClient was included in my previous email for reference. I can't make any claims about what was happening with packet transmissions at the network level. It's entirely possible that the destination maintained the TCP connection over an extended period without fulfilling its obligation to send the body. But there must be a way to guarantee completion for such scenarios. This was the function of socket timeouts in the world of synchronous network APIs. In this new asynchronous world, it looks like that might have been forgotten. The alternative timeout code you mentioned addresses the problem in a narrow sense, but it results in a resource leak. If the HTTP session is abandoned at the application level via CompletableFuture timed wait, there is still a footprint within HttpClient. Over time, socket handles, callback objects, and other resources can accumulate with no visibility to application code. There should be no leak if you call CompletableFuture::cancel when catching the TimeoutException as shown in my suggestion, and provided you're on a recent JDK (16+). That should take care of reclaiming all the resources linked to the exchange. (that was taken care of by https://bugs.openjdk.java.net/browse/JDK-8245462) If we had to implement a global timeout that's how we would do it internally. best regards, -- daniel
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 [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: HttpClient Send Method Guaranteed Completion
On 06/10/2021 10:42, Daniel Fuchs wrote:
} catch (TimeoutException x) {
// timeout: cancel request to free up
// any related resources
cf.cancel();
Correction: that should be `cf.cancel(true)`
Re: RFR: 8274809: Update java.base classes to use try-with-resources [v3]
On Wed, 6 Oct 2021 16:07:12 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
>> update copyright year
>
> src/java.base/share/classes/sun/security/util/PolicyUtil.java line 156:
>
>> 154: }
>> 155:
>> 156: try (InputStream inStream = new
>> BufferedInputStream(getInputStream(keyStoreUrl))) {
>
> Same comment as above.
Looks like it's still > 80 chars
-
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 18:47:26 GMT, Andrey Turbanov wrote: >> 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 Greater than 80 chars comment missing in latest changeset. - 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 18:47:26 GMT, Andrey Turbanov
wrote:
>> 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
src/java.base/share/classes/sun/net/NetProperties.java line 72:
> 70: fname = f.getCanonicalPath();
> 71: try (FileInputStream in = new FileInputStream(fname)) {
> 72: BufferedInputStream bin = new BufferedInputStream(in);
Shouldn't this have a multiple resource block in the try-with block here:
try (FileInputStream in = new FileInputStream(fname);
BufferedInputStream bin = new BufferedInputStream(in)) {
props.land(bin);
}
-
PR: https://git.openjdk.java.net/jdk/pull/5818
Re: HttpClient Send Method Guaranteed Completion
Thank you for pointing that out! That addresses my immediate concern. This
ought to work just fine.
From: Daniel Fuchs
Sent: Wednesday, October 6, 2021 12:08 PM
To: Elliot Barlas ; net-dev@openjdk.java.net
Subject: Re: HttpClient Send Method Guaranteed Completion
On 06/10/2021 10:42, Daniel Fuchs wrote:
>} catch (TimeoutException x) {
>// timeout: cancel request to free up
>// any related resources
>cf.cancel();
Correction: that should be `cf.cancel(true)`
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 [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: 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; The updated logic looks correct, and my tests (tier1,tier2,tier3) came back with no failure. Reviewed. - Marked as reviewed by dfuchs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5190
Re: RFR: 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. `java.io` change looks all right. - Marked as reviewed by bpb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5454
