Re: RFR: 8270553: Tests should not use (real, in-use, routable) 1.1.1.1 as dummy IP value [v2]

2021-09-13 Thread Aleksey Shipilev 
On Fri, 6 Aug 2021 19:50:48 GMT, Jonathan Dowland  wrote:

>> The tests `test/jdk/java/net/HttpURLConnection/HttpURLConWithProxy.java` 
>> uses the IP address "1.1.1.1" as a value. I think at the time the address 
>> was picked, the assumption was the address was not valid / not routable. 
>> Since April 2018 the address is part of CloudFlare's "Free" DNS product: 
>> . (this test was originally written 
>> in 2016, before the service was launched)
>> 
>> I've verified using local packet captures that running the test does result 
>> in IP traffic being sent to 1.1.1.1. (Several other tests in JDK use 1.1.1.1 
>> as a placeholder IP. I've checked them all and none of the others connect 
>> out to the IP like this one)
>>  
>> This PR substitutes that IP address value (and two others) for ones from a 
>> reserved IP range (240.0.0.0/4 according to RFC 6761) which will not result 
>> in runners of the test suit inadvertently sending IP packets to the 
>> CloudFlare service. 
>> 
>> This could be invalidated again if that address range is allocated at some 
>> point in the future. A more future-proof fix would be to bind to random 
>> ports on localhost for each dummy proxy (as done for the target HTTP server 
>> in the test already). I can do that if preferred.
>> 
>> 
>
> Jonathan Dowland has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains one commit:
> 
>   8270553: Tests should not use (real, in-use, routable) 1.1.1.1 as dummy IP 
> value

Marked as reviewed by shade (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/4806

Re: RFR: 8273616: Fix trivial doc typos in the java.base module [v2]

2021-09-13 Thread Pavel Rappo 
On Fri, 10 Sep 2021 23:20:11 GMT, Pavel Rappo  wrote:

>> 8273616: Fix trivial doc typos in the java.base module
>
> Pavel Rappo has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Revert two fixes

Thanks to those who reviewed this PR. Since I posted it, I've found three more 
occurrences of _insure_ used in the sense of _ensure_: two in the 
`java.io.Object*Stream` area and one in the `java.util.Currency` class. I 
decided to fix those in this PR, which now needs to be (re)reviewed. Thanks!

There are more occurrences of _insure_, which I didn't touch. Some of them are 
in java.sql, java.sql.rowset and java.desktop. In the latter, _insure_ even 
crept into method names.

-

PR: https://git.openjdk.java.net/jdk/pull/5475

Re: RFR: 8273616: Fix trivial doc typos in the java.base module [v3]

2021-09-13 Thread Pavel Rappo 
> 8273616: Fix trivial doc typos in the java.base module

Pavel Rappo has updated the pull request incrementally with one additional 
commit since the last revision:

  Use "ensure" instead of "insure"

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5475/files
  - new: https://git.openjdk.java.net/jdk/pull/5475/files/9a9deee1..4b33fb94

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5475&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5475&range=01-02

  Stats: 3 lines in 2 files changed: 0 ins; 0 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5475.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5475/head:pull/5475

PR: https://git.openjdk.java.net/jdk/pull/5475

Re: RFR: 8238274: (sctp) JDK-7118373 is not fixed for SctpChannel

2021-09-13 Thread Masanori Yano 
On Thu, 2 Sep 2021 14:24:24 GMT, Daniel Fuchs  wrote:

>> Please review this change to the Unix implementations of 
>> sun.nio.ch.sctp.Sctp*ChannelImpl#implCloseSelectableChannel() 
>> to be same as SocketChannelImpl at JDK-7118373. (The preClose is missing a 
>> check for the ST_KILLED state.)
>
> I have run this change on one of our machines that support SCTP. I did get 
> some intermittent failures with the other SCTP tests - they don't seem much 
> stable - but the new proposed test was failing all the time. I suspect that 
> using `lsof` to figure out whether the file descriptor was closed is not 
> reliable/portable enough. I also tried to modify the test to use /othervm - 
> which is probably a good idea if you don't want your results to be polluted 
> by whatever other test might have run previously/concurrently in the agent VM 
> - but to no avail: the test was still failing 100% of the time.
> @masyano could you figure another way to detect whether the file descriptor 
> has been released?

@dfuch I tried to count `/proc//fd`, but there are recorded all fds in the 
process. I don't know how to pick up only active fds in `/proc//fd`. So, I 
will change to call lsof last one time only (like 
https://github.com/openjdk/jdk/pull/4621), and counts all unreleased fds.

Could you try this test case?

-

PR: https://git.openjdk.java.net/jdk/pull/5274

Re: RFR: 8238274: (sctp) JDK-7118373 is not fixed for SctpChannel [v2]

2021-09-13 Thread Masanori Yano 
> Please review this change to the Unix implementations of 
> sun.nio.ch.sctp.Sctp*ChannelImpl#implCloseSelectableChannel() 
> to be same as SocketChannelImpl at JDK-7118373. (The preClose is missing a 
> check for the ST_KILLED state.)

Masanori Yano 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 three additional 
commits since the last revision:

 - 8238274: (sctp) JDK-7118373 is not fixed for SctpChannel
 - Merge branch 'master' of https://github.com/masyano/jdk into 8238274
 - 8238274: (sctp) JDK-7118373 is not fixed for SctpChannel

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5274/files
  - new: https://git.openjdk.java.net/jdk/pull/5274/files/31a23e2f..cd67ff8f

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5274&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5274&range=00-01

  Stats: 19064 lines in 853 files changed: 10843 ins; 4119 del; 4102 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5274.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5274/head:pull/5274

PR: https://git.openjdk.java.net/jdk/pull/5274

Re: RFR: 8273616: Fix trivial doc typos in the java.base module [v3]

2021-09-13 Thread Daniel Fuchs 
On Mon, 13 Sep 2021 11:22:27 GMT, Pavel Rappo  wrote:

>> 8273616: Fix trivial doc typos in the java.base module
>
> Pavel Rappo has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Use "ensure" instead of "insure"

LGTM

-

Marked as reviewed by dfuchs (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/5475

Re: RFR: 8273616: Fix trivial doc typos in the java.base module [v3]

2021-09-13 Thread Roger Riggs 
On Mon, 13 Sep 2021 11:22:27 GMT, Pavel Rappo  wrote:

>> 8273616: Fix trivial doc typos in the java.base module
>
> Pavel Rappo has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Use "ensure" instead of "insure"

Marked as reviewed by rriggs (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/5475

Re: RFR: 8238274: (sctp) JDK-7118373 is not fixed for SctpChannel [v2]

2021-09-13 Thread Daniel Fuchs 
On Mon, 13 Sep 2021 11:48:22 GMT, Masanori Yano  wrote:

>> Please review this change to the Unix implementations of 
>> sun.nio.ch.sctp.Sctp*ChannelImpl#implCloseSelectableChannel() 
>> to be same as SocketChannelImpl at JDK-7118373. (The preClose is missing a 
>> check for the ST_KILLED state.)
>
> Masanori Yano 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 three additional 
> commits since the last revision:
> 
>  - 8238274: (sctp) JDK-7118373 is not fixed for SctpChannel
>  - Merge branch 'master' of https://github.com/masyano/jdk into 8238274
>  - 8238274: (sctp) JDK-7118373 is not fixed for SctpChannel

test/jdk/com/sun/nio/sctp/SctpChannel/CloseDescriptors.java line 29:

> 27:  * @summary Potential leak file descriptor for SCTP
> 28:  * @requires (os.family == "linux")
> 29:  * @run main CloseDescriptors

The test SctpMultiChannel/CloseDescriptors seems to work - at least it doesn't 
fail all the time - but it's using `@run main/othervm`. I believe you should 
make this test run in /othervm mode for more stability. I will test again with 
your later change and /othervm mode.

-

PR: https://git.openjdk.java.net/jdk/pull/5274

Re: RFR: 8273616: Fix trivial doc typos in the java.base module [v3]

2021-09-13 Thread Iris Clark 
On Mon, 13 Sep 2021 11:22:27 GMT, Pavel Rappo  wrote:

>> 8273616: Fix trivial doc typos in the java.base module
>
> Pavel Rappo has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Use "ensure" instead of "insure"

Marked as reviewed by iris (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/5475

Re: RFR: 8273616: Fix trivial doc typos in the java.base module [v3]

2021-09-13 Thread Lance Andersen 
On Mon, 13 Sep 2021 11:22:27 GMT, Pavel Rappo  wrote:

>> 8273616: Fix trivial doc typos in the java.base module
>
> Pavel Rappo has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Use "ensure" instead of "insure"

Marked as reviewed by lancea (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/5475

Re: RFR: 8273616: Fix trivial doc typos in the java.base module [v3]

2021-09-13 Thread John R Rose 
On Mon, 13 Sep 2021 11:22:27 GMT, Pavel Rappo  wrote:

>> 8273616: Fix trivial doc typos in the java.base module
>
> Pavel Rappo has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Use "ensure" instead of "insure"

Marked as reviewed by jrose (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/5475

Integrated: 8273616: Fix trivial doc typos in the java.base module

2021-09-13 Thread Pavel Rappo 
On Fri, 10 Sep 2021 21:16:19 GMT, Pavel Rappo  wrote:

> 8273616: Fix trivial doc typos in the java.base module

This pull request has now been integrated.

Changeset: b4b12101
Author:Pavel Rappo 
URL:   
https://git.openjdk.java.net/jdk/commit/b4b121018d16e531f3a51ff75ae37fdc374d530b
Stats: 55 lines in 34 files changed: 0 ins; 0 del; 55 mod

8273616: Fix trivial doc typos in the java.base module

Reviewed-by: jrose, iris, lancea, dfuchs, rriggs

-

PR: https://git.openjdk.java.net/jdk/pull/5475