Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v17]

[Brian Burkhalter]

On Fri, 27 May 2022 18:40:32 GMT, XenoAmess  wrote:

>> as title.
>
> XenoAmess has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   do it as naotoj said

`java.io` and `java.nio` look all right.

-

Marked as reviewed by bpb (Reviewer).

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

Re: RFR: 8287237: (fs) Files.probeContentType returns null if filename contains hash mark on Linux [v4]

[Brian Burkhalter]

On Wed, 1 Jun 2022 07:28:11 GMT, Vyom Tewari  wrote:

> Looks ok, i tested on centos 7 and it is working as expected.

To really verify it you would have to suppress `~/.mime.types` and 
`/etc/mime.types` before running the test.

-

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

Integrated: 8287237: (fs) Files.probeContentType returns null if filename contains hash mark on Linux

[Brian Burkhalter]

On Thu, 26 May 2022 23:03:05 GMT, Brian Burkhalter  wrote:

> Modify `sun.net.www.MimeTable.findByFileName(String)` to attempt to find the 
> file extension in the entire file name if it is not found in the portion of 
> the name preceding the optional fragment beginning with a hash (`#`).

This pull request has now been integrated.

Changeset: 8071b231
Author:    Brian Burkhalter 
URL:   
https://git.openjdk.java.net/jdk/commit/8071b2311caaacd714d74f12aee6cb7c2fe700fa
Stats: 79 lines in 2 files changed: 53 ins; 15 del; 11 mod

8287237: (fs) Files.probeContentType returns null if filename contains hash 
mark on Linux

Reviewed-by: rriggs, jpai, vtewari

-

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

Re: RFR: 8287237: (fs) Files.probeContentType returns null if filename contains hash mark on Linux [v4]

[Brian Burkhalter]

> Modify `sun.net.www.MimeTable.findByFileName(String)` to attempt to find the 
> file extension in the entire file name if it is not found in the portion of 
> the name preceding the optional fragment beginning with a hash (`#`).

Brian Burkhalter has updated the pull request incrementally with one additional 
commit since the last revision:

  8287237: Simplify code a bit

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8909/files
  - new: https://git.openjdk.java.net/jdk/pull/8909/files/7c877f9e..b9eb7bbb

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=8909=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8909=02-03

  Stats: 12 lines in 1 file changed: 0 ins; 4 del; 8 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8909.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8909/head:pull/8909

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

Re: RFR: 8287237: (fs) Files.probeContentType returns null if filename contains hash mark on Linux [v3]

[Brian Burkhalter]

> Modify `sun.net.www.MimeTable.findByFileName(String)` to attempt to find the 
> file extension in the entire file name if it is not found in the portion of 
> the name preceding the optional fragment beginning with a hash (`#`).

Brian Burkhalter has updated the pull request incrementally with one additional 
commit since the last revision:

  8287237: Correct off-by-one endIndex passed to String.substring()

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8909/files
  - new: https://git.openjdk.java.net/jdk/pull/8909/files/eab33e80..7c877f9e

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=8909=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8909=01-02

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8909.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8909/head:pull/8909

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

Re: RFR: 8287237: (fs) Files.probeContentType returns null if filename contains hash mark on Linux [v2]

[Brian Burkhalter]

On Tue, 31 May 2022 16:41:57 GMT, Brian Burkhalter  wrote:

>> src/java.base/share/classes/sun/net/www/MimeTable.java line 188:
>> 
>>> 186: int hashIndex = fname.lastIndexOf(HASH_MARK);
>>> 187: if (hashIndex > 0) {
>>> 188: String ext = getFileExtension(fname.substring(0, hashIndex 
>>> - 1));
>> 
>> Hello Brian, I think there's a bug here. Not introduced by this PR but even 
>> in the current master. I think that `fname.substring(0, hashIndex -1)` call 
>> should actually be `fname.substring(0, hashIndex)`. For example, in its 
>> current form, if `fname` is `a.png#foo` then `fname.substring(...)` here 
>> will return `a.pn` instead of `a.png`. It looks like we don't have a test 
>> for that case.
>
> I agree. I saw that myself. As nothing seems broken I did not want to touch 
> it but I will investigate.

This was probably never caught because all our Linux machines seem to have 
`/etc/mime.types`.

Fixed by 7c877f9ee2a0788849bac4d64a4ab4c89cbc9e0c.

-

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

Re: RFR: 8287237: (fs) Files.probeContentType returns null if filename contains hash mark on Linux [v2]

[Brian Burkhalter]

On Sat, 28 May 2022 14:39:16 GMT, Jaikiran Pai  wrote:

>> Brian Burkhalter has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8287237: Refactor and clean up
>
> src/java.base/share/classes/sun/net/www/MimeTable.java line 188:
> 
>> 186: int hashIndex = fname.lastIndexOf(HASH_MARK);
>> 187: if (hashIndex > 0) {
>> 188: String ext = getFileExtension(fname.substring(0, hashIndex 
>> - 1));
> 
> Hello Brian, I think there's a bug here. Not introduced by this PR but even 
> in the current master. I think that `fname.substring(0, hashIndex -1)` call 
> should actually be `fname.substring(0, hashIndex)`. For example, in its 
> current form, if `fname` is `a.png#foo` then `fname.substring(...)` here will 
> return `a.pn` instead of `a.png`. It looks like we don't have a test for that 
> case.

I agree. I saw that myself. As nothing seems broken I did not want to touch it 
but I will investigate.

> src/java.base/share/classes/sun/net/www/MimeTable.java line 196:
> 
>> 194: 
>> 195: String ext = "";
>> 196: if (i != -1 && fname.charAt(i) == '.')
> 
> Nit - the existing method currently uses a `{` even for single line 
> conditionals. Should we do the same for the new `if` blocks introduced in 
> this PR and enclose them within  `{` `}`?

Fixed in eab33e8000ea730c57c918dbf291af23bacbd059.

-

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

Re: RFR: 8287237: (fs) Files.probeContentType returns null if filename contains hash mark on Linux [v2]

[Brian Burkhalter]

On Fri, 27 May 2022 12:15:07 GMT, Jaikiran Pai  wrote:

>> Brian Burkhalter has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8287237: Refactor and clean up
>
> src/java.base/share/classes/sun/net/www/MimeTable.java line 164:
> 
>> 162: public MimeEntry findByFileName(String fname) {
>> 163: // attempt to find the entry with the fragment component removed
>> 164: MimeEntry entry = findByFileName(fname, true);
> 
> I think we might need a check here first to see if the `fname` contains `#` 
> and if it does, only then call the `findByFileName` with `true`. Without that 
> check, with the change in this PR, it's now possible that the 
> `findByFileName` will get called twice (once with `true` and once with 
> `false`) for the case where the filename doesn't have a `#` and whose 
> extension isn't in the MimeTable.

Methods refactored in eab33e8000ea730c57c918dbf291af23bacbd059.

> src/java.base/share/classes/sun/net/www/MimeTable.java line 183:
> 
>> 181:  * @return the MIME entry associated with the file name
>> 182:  */
>> 183: public MimeEntry findByFileName(String fname, boolean 
>> removeFragment) {
> 
> Hello Brian,
> 
> Perhaps this new method can be made `private` (and maybe even `static`)?

Private yes, static no. Refactored in eab33e8000ea730c57c918dbf291af23bacbd059.

> test/jdk/java/nio/file/Files/probeContentType/Basic.java line 197:
> 
>> 195: String contentType = Files.probeContentType(pathWithFragement);
>> 196: if (contentType == null || !contentType.equals("image/png")) {
>> 197: System.out.printf("For %s expected \"png\" but got %s%n",
> 
> Should this instead use `System.err`, since the rest of the test uses that 
> for printing these failures when incrementing the failure count.

Yes, it should have used `System.err`; fixed in 
eab33e8000ea730c57c918dbf291af23bacbd059.

-

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

RFR: 8287237: (fs) Files.probeContentType returns null if filename contains hash mark on Linux

[Brian Burkhalter]

Modify `sun.net.www.MimeTable.findByFileName(String)` to attempt to find the 
file extension in the entire file name if it is not found in the portion of the 
name preceding the optional fragment beginning with a hash (`#`).

-

Commit messages:
 - 8287237: (fs) Files.probeContentType returns null if filename contains hash 
mark on Linux

Changes: https://git.openjdk.java.net/jdk/pull/8909/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8909=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8287237
  Stats: 51 lines in 2 files changed: 37 ins; 2 del; 12 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8909.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8909/head:pull/8909

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

Re: RFR: 8286378: Address possibly lossy conversions in java.base [v3]

[Brian Burkhalter]

On Wed, 11 May 2022 16:30:41 GMT, Roger Riggs  wrote:

>> PR#8599 8244681: proposes to add compiler warnings for possible lossy 
>> conversions
>> From the CSR:
>> 
>> "If the type of the right-hand operand of a compound assignment is not 
>> assignment compatible with the type of the variable, a cast is implied and 
>> possible lossy conversion may silently occur. While similar situations are 
>> resolved as compilation errors for primitive assignments, there are no 
>> similar rules defined for compound assignments."
>> 
>> This PR anticipates the warnings and adds explicit casts to replace the 
>> implicit casts.
>> In most cases, the cast matches the type of the right-hand side to type of 
>> the compound assignment.
>> Since these casts are truncating the value, there might be a different 
>> refactoring that avoids the cast
>> but a direct replacement was chosen here.
>> 
>> (Advise and suggestions will inform similar updates to other OpenJDK 
>> modules).
>
> Roger Riggs has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Updated copyrights
>   Fixed cast style to add a space after cast, (where consistent with file 
> style)
>   Improved code per review comments in PollSelectors.

Marked as reviewed by bpb (Reviewer).

-

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

Re: RFR: 8286378: Address possibly lossy conversions in java.base

[Brian Burkhalter]

On Tue, 10 May 2022 21:32:10 GMT, Roger Riggs  wrote:

> PR#8599 8244681: proposes to add compiler warnings for possible lossy 
> conversions
> From the CSR:
> 
> "If the type of the right-hand operand of a compound assignment is not 
> assignment compatible with the type of the variable, a cast is implied and 
> possible lossy conversion may silently occur. While similar situations are 
> resolved as compilation errors for primitive assignments, there are no 
> similar rules defined for compound assignments."
> 
> This PR anticipates the warnings and adds explicit casts to replace the 
> implicit casts.
> In most cases, the cast matches the type of the right-hand side to type of 
> the compound assignment.
> Since these casts are truncating the value, there might be a different 
> refactoring that avoids the cast
> but a direct replacement was chosen here.
> 
> (Advise and suggestions will inform similar updates to other OpenJDK modules).

IO, math, and NIO changes look fine.

-

Marked as reviewed by bpb (Reviewer).

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

Re: RFR: 8274835: Remove unnecessary castings in java.base

[Brian Burkhalter]

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

Re: RFR: 8273655: content-types.properties files are missing some common types [v2]

[Brian Burkhalter]

On Thu, 16 Sep 2021 15:28:36 GMT, Julia Boes  wrote:

>> This change adds some common types to the content-type.properties files, 
>> notably .js, .css, and .jar, as well as some others. 
>> 
>> The duplicated entry for .zip is removed from the Windows properties file.
>
> Julia Boes has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   address PR comments and refactor test

Marked as reviewed by bpb (Reviewer).

-

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

Re: RFR: 8273243: Fix indentations in java.net.InetAddress methods

[Brian Burkhalter]

On Wed, 1 Sep 2021 16:53:50 GMT, Aleksei Efimov  wrote:

> Hi,
> 
> The fix changes indentations in the following `java.net.InetAddress` methods:
> - getAddressesFromNameService
> - getHostFromNameService
> 
> It helps to improve code readability. Can't say the same about 
> `getHostFromNameService` diffs shown in this PR.

Marked as reviewed by bpb (Reviewer).

src/java.base/share/classes/java/net/InetAddress.java line 689:

> 687: }
> 688: 
> 689: //XXX: if it looks a spoof just return the address?

looks _like_ a spoof

-

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

Integrated: 8269481: SctpMultiChannel never releases own file descriptor

[Brian Burkhalter]

On Tue, 29 Jun 2021 00:44:25 GMT, Brian Burkhalter  wrote:

> Please review this change to the Unix implementations of 
> `sun.nio.ch.sctp.Sctp*ChannelImpl#kill()` to close the socket if `state == 
> ChannelState.UNINITIALIZED`.

This pull request has now been integrated.

Changeset: d1cecaaa
Author:    Brian Burkhalter 
URL:   
https://git.openjdk.java.net/jdk/commit/d1cecaaa22d551b93074c33209dac7354f4b6932
Stats: 145 lines in 4 files changed: 142 ins; 3 del; 0 mod

8269481: SctpMultiChannel never releases own file descriptor

Reviewed-by: alanb, chegar

-

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

Re: RFR: 8269481: SctpMultiChannel never releases own file descriptor [v6]

[Brian Burkhalter]

> Please review this change to the Unix implementations of 
> `sun.nio.ch.sctp.Sctp*ChannelImpl#kill()` to close the socket if `state == 
> ChannelState.UNINITIALIZED`.

Brian Burkhalter has updated the pull request incrementally with one additional 
commit since the last revision:

  8269481: Clean up MAX_DESC in test

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4621/files
  - new: https://git.openjdk.java.net/jdk/pull/4621/files/8ca5b5c0..65ec32a0

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4621=05
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4621=04-05

  Stats: 4 lines in 1 file changed: 0 ins; 1 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4621.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4621/head:pull/4621

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

Re: RFR: 8269481: SctpMultiChannel never releases own file descriptor [v5]

[Brian Burkhalter]

On Fri, 2 Jul 2021 16:14:25 GMT, Brian Burkhalter  wrote:

>> Please review this change to the Unix implementations of 
>> `sun.nio.ch.sctp.Sctp*ChannelImpl#kill()` to close the socket if `state == 
>> ChannelState.UNINITIALIZED`.
>
> Brian Burkhalter has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8269481: In test use othervm mode and an automatically allocated port

Sure, no problem.

-

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

Re: RFR: 8269481: SctpMultiChannel never releases own file descriptor [v5]

[Brian Burkhalter]

> Please review this change to the Unix implementations of 
> `sun.nio.ch.sctp.Sctp*ChannelImpl#kill()` to close the socket if `state == 
> ChannelState.UNINITIALIZED`.

Brian Burkhalter has updated the pull request incrementally with one additional 
commit since the last revision:

  8269481: In test use othervm mode and an automatically allocated port

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4621/files
  - new: https://git.openjdk.java.net/jdk/pull/4621/files/676c0f0a..8ca5b5c0

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4621=04
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4621=03-04

  Stats: 23 lines in 1 file changed: 5 ins; 8 del; 10 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4621.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4621/head:pull/4621

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

Re: RFR: 8269481: SctpMultiChannel never releases own file descriptor [v4]

[Brian Burkhalter]

> Please review this change to the Unix implementations of 
> `sun.nio.ch.sctp.Sctp*ChannelImpl#kill()` to close the socket if `state == 
> ChannelState.UNINITIALIZED`.

Brian Burkhalter has updated the pull request incrementally with one additional 
commit since the last revision:

  8269481: Fix incorrect order of setting state to KILLED and closing the socket

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4621/files
  - new: https://git.openjdk.java.net/jdk/pull/4621/files/c8865603..676c0f0a

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4621=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4621=02-03

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

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

Re: RFR: 8269481: SctpMultiChannel never releases own file descriptor [v3]

[Brian Burkhalter]

Oh I see now. Thanks.

On Jul 1, 2021, at 8:56 AM, Alan Bateman 
mailto:alan.bate...@oracle.com>> wrote:

I should have been clearer. Look at SctpServerChannelImpl.java L284 and 
SctpMultiChannelImpl.java L376.  SctpNet.close(fdVal) added with the patch 
added them them before setting the state. The suggestion in the previous 
comments was to insert the close after setting the state to KILLED in case the 
close fail (in some corner case).

Re: RFR: 8269481: SctpMultiChannel never releases own file descriptor [v3]

[Brian Burkhalter]

On Jul 1, 2021, at 1:17 AM, Alan Bateman 
mailto:al...@openjdk.java.net>> wrote:

It looks like you've changed some but not all cases. I realize 
ChannelState.UNINITIALIZED is not too interesting but further maintainers may 
wonder about it.

I’m not seeing that I missed any cases. I don’t really want to get into 
modifying the states in this PR.

Re: RFR: 8269481: SctpMultiChannel never releases own file descriptor [v3]

[Brian Burkhalter]

On Tue, 29 Jun 2021 17:50:48 GMT, Brian Burkhalter  wrote:

>> Please review this change to the Unix implementations of 
>> `sun.nio.ch.sctp.Sctp*ChannelImpl#kill()` to close the socket if `state == 
>> ChannelState.UNINITIALIZED`.
>
> Brian Burkhalter has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8269481: Change test to look for lsof in more than one location

If this looks all right could I get an approval please?

-

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

Re: RFR: 8269481: SctpMultiChannel never releases own file descriptor [v3]

[Brian Burkhalter]

> Please review this change to the Unix implementations of 
> `sun.nio.ch.sctp.Sctp*ChannelImpl#kill()` to close the socket if `state == 
> ChannelState.UNINITIALIZED`.

Brian Burkhalter has updated the pull request incrementally with one additional 
commit since the last revision:

  8269481: Change test to look for lsof in more than one location

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4621/files
  - new: https://git.openjdk.java.net/jdk/pull/4621/files/cc18da47..c8865603

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4621=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4621=01-02

  Stats: 17 lines in 1 file changed: 15 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4621.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4621/head:pull/4621

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

Re: RFR: 8269481: SctpMultiChannel never releases own file descriptor [v2]

[Brian Burkhalter]

On Tue, 29 Jun 2021 16:58:53 GMT, Chris Hegarty  wrote:

>> Brian Burkhalter has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8269481: Set state to KILLED *before* closing socket
>
> test/jdk/com/sun/nio/sctp/SctpMultiChannel/CloseDescriptors.java line 71:
> 
>> 69: ProcessBuilder pb = new ProcessBuilder(
>> 70: "lsof", "-U", "-a", "-p", Long.toString(myPid));
>> 71: Process p = pb.start();
> 
> Is this test stable? Do we do similar `losf` in other areas ?

`test/jdk/java/net/DatagramSocket/UnreferencedDatagramSockets.java
`
`test/jdk/java/net/MulticastSocket/UnreferencedMulticastSockets.java
`

Might want to change this test to be robust as to the location of the `lsof` 
executable.

-

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

Re: RFR: 8269481: SctpMultiChannel never releases own file descriptor [v2]

[Brian Burkhalter]

On Tue, 29 Jun 2021 16:55:33 GMT, Chris Hegarty  wrote:

>> Brian Burkhalter has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8269481: Set state to KILLED *before* closing socket
>
> src/jdk.sctp/unix/classes/sun/nio/ch/sctp/SctpMultiChannelImpl.java line 377:
> 
>> 375: if (state == ChannelState.UNINITIALIZED) {
>> 376: SctpNet.close(fdVal);
>> 377: state = ChannelState.KILLED;
> 
> Good catch. It seems that there was a missing state here, but not worth 
> adding at this point.

I also thought that it looks like there is a missing state.

-

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

Re: RFR: 8269481: SctpMultiChannel never releases own file descriptor [v2]

[Brian Burkhalter]

> Please review this change to the Unix implementations of 
> `sun.nio.ch.sctp.Sctp*ChannelImpl#kill()` to close the socket if `state == 
> ChannelState.UNINITIALIZED`.

Brian Burkhalter has updated the pull request incrementally with one additional 
commit since the last revision:

  8269481: Set state to KILLED *before* closing socket

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4621/files
  - new: https://git.openjdk.java.net/jdk/pull/4621/files/23cdf146..cc18da47

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4621=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4621=00-01

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

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

Re: RFR: 8269481: SctpMultiChannel never releases own file descriptor [v2]

[Brian Burkhalter]

On Tue, 29 Jun 2021 06:52:47 GMT, Alan Bateman  wrote:

>> Brian Burkhalter has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8269481: Set state to KILLED *before* closing socket
>
> src/jdk.sctp/unix/classes/sun/nio/ch/sctp/SctpChannelImpl.java line 643:
> 
>> 641: if (state == ChannelState.UNINITIALIZED) {
>> 642: SctpNet.close(fdVal);
>> 643: state = ChannelState.KILLED;
> 
> It might be better to invert these, meaning set the state to KILLED before 
> close(fdVal), just in case the close throws.

So modified.

-

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

Re: RFR: 8269481: SctpMultiChannel never releases own file descriptor

[Brian Burkhalter]

On Tue, 29 Jun 2021 00:44:25 GMT, Brian Burkhalter  wrote:

> Please review this change to the Unix implementations of 
> `sun.nio.ch.sctp.Sctp*ChannelImpl#kill()` to close the socket if `state == 
> ChannelState.UNINITIALIZED`.

It would be good if someone more familiar with the SCTP channels could take a 
look at this one.

-

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

RFR: 8269481: SctpMultiChannel never releases own file descriptor

[Brian Burkhalter]

Please review this change to the Unix implementations of 
`sun.nio.ch.sctp.Sctp*ChannelImpl#kill()` to close the socket if `state == 
ChannelState.UNINITIALIZED`.

-

Commit messages:
 - 8269481: SctpMultiChannel never releases own file descriptor

Changes: https://git.openjdk.java.net/jdk/pull/4621/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4621=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8269481
  Stats: 128 lines in 4 files changed: 128 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4621.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4621/head:pull/4621

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

Re: RFR: 8266155: Convert java.base to use Stream.toList()

[Brian Burkhalter]

On Tue, 27 Apr 2021 21:34:02 GMT, Ian Graves  wrote:

> 8266155: Convert java.base to use Stream.toList()

Looks all right.

-

Marked as reviewed by bpb (Reviewer).

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

Re: RFR: 8263233: Update java.net and java.nio to use instanceof pattern variable [v2]

[Brian Burkhalter]

On Tue, 9 Mar 2021 17:59:27 GMT, Patrick Concannon  
wrote:

>> Hi,
>> 
>> Could someone please review my code for updating the code in the `java.net` 
>> and `java.nio` packages to make use of the `instanceof` pattern variable?
>> 
>> Kind regards,
>> Patrick
>
> Patrick Concannon has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8263233: Small refactor of equals method in java/net/InterfaceAddress; 
> removed superfluous whitespace in java/net/Inet6Address

Looks fine.

-

Marked as reviewed by bpb (Reviewer).

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

Re: RFR: 8262296 Fix remaining doclint warnings in jdk.httpserver [v2]

[Brian Burkhalter]

On Wed, 24 Feb 2021 16:21:59 GMT, Chris Hegarty  wrote:

>> Trivial clean up of remaining doclint warnings in jdk.httpserver. 
>> 
>> The minor spec clarifications do not amount to a normative change, so no CSR 
>> is required (they're documented the obvious and only possible behaviour).
>
> Chris Hegarty has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Update external links to use https

Marked as reviewed by bpb (Reviewer).

-

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

Re: RFR: JDK-8261601: free memory in early return in Java_sun_nio_ch_sctp_SctpChannelImpl_receive0

[Brian Burkhalter]

On Fri, 12 Feb 2021 08:50:14 GMT, Matthias Baesken  wrote:

> There seems to be an early return in 
> Java_sun_nio_ch_sctp_SctpChannelImpl_receive0 that misses freeing memory.
> 
> Sonar reports :
> https://sonarcloud.io/project/issues?id=shipilev_jdk=c=AXck8Cl0BBG2CXpcnjFu=false=BLOCKER=BUG
> 
> Potential leak of memory pointed to by 'newBuf'
> I adjusted  the early return and added freeing memory .
> 
> 
> Btw. while  adjusting  Java_sun_nio_ch_sctp_SctpChannelImpl_receive0  , I 
> started  to wonder what happens to the allocated memory in  the same file in 
> handleSendFailed  (  if ((addressP = malloc(dataLength)) == NULL)   )   in 
> early return cases  incl. the CHECK_NULL , is there some deallocation missing 
> there too ?

This change looks fine. I don't know about handleSendFailed().

-

Marked as reviewed by bpb (Reviewer).

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

Re: RFR: 8250564: Remove terminally deprecated constructor in GSSUtil

[Brian Burkhalter]

On Tue, 5 Jan 2021 21:02:21 GMT, Joe Darcy  wrote:

> Back in JDK 16, two unintended default constructors were identified and 
> deprecated for removal. The time has come to remove them.
> 
> Please also review the corresponding CSRs:
> 
> JDK-8258521 Remove terminally deprecated constructor in GSSUtil 
> https://bugs.openjdk.java.net/browse/JDK-8258521
> 
>  JDK-8258522 Remove terminally deprecated constructor in java.net.URLDecoder 
> https://bugs.openjdk.java.net/browse/JDK-8258522
> 
> Calling a static method using an instance of a class, as done in the test 
> B6463990.java, is a coding anti-pattern that generates a lint warning; that 
> warning in enabled in the JDK build.
> 
> Thanks,

Looks good. CSRs also reviewed.

-

Marked as reviewed by bpb (Reviewer).

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

Re: RFR: 8258696: Temporarily revert use of pattern match instanceof until docs-reference is fixed

[Brian Burkhalter]

On Fri, 18 Dec 2020 16:54:59 GMT, Chris Hegarty  wrote:

> Temporarily revert use of pattern match instanceof construct until 
> docs-reference is fixed, see JDK-8258657. 
> 
> ... 
> Generating REFERENCE_API javadoc for 21 modules 
> 
> if (delegate instanceof ExecutorService service) { 
> ^ 
>   (use --enable-preview to enable pattern matching in instanceof) 
> 1 error 
> make[3]: *** 
> [/Users/chhegar/git/open/build/macosx-x64/support/docs/_javadoc_REFERENCE_API_exec.marker]
>  Error 1 
> Docs.gmk:472: recipe for target 
> '/Users/chhegar/git/open/build/macosx-x64/support/docs/_javadoc_REFERENCE_API_exec.marker'
>  failed 
> make/Main.gmk:485: recipe for target 'docs-reference-api-javadoc' failed 
> make[2]: *** [docs-reference-api-javadoc] Error 2 
> make[2]: *** Waiting for unfinished jobs

Marked as reviewed by bpb (Reviewer).

-

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

Re: RFR: 8255555: Bad copyright headers in SocketChannelCompare.java SocketChannelConnectionSetup.java UnixSocketChannelReadWrite.java

[Brian Burkhalter]

On Wed, 28 Oct 2020 18:48:51 GMT, Michael McMahon  wrote:

> …nnelConnectionSetup.java UnixSocketChannelReadWrite.java

Marked as reviewed by bpb (Reviewer).

-

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

Re: RFR: 8227721: NetworkInterfaceRetrievalTests.java should open the java.net package

[Brian Burkhalter]

Hi Patrick,

Based on [1] this looks OK to me.

Brian

[1] https://openjdk.java.net/jtreg/tag-spec.html

> On Jul 17, 2019, at 9:52 AM, Patrick Concannon  
> wrote:
> 
> Would it be possible to have my fix for JDK-8227721 reviewed?
> 
> test/jdk/java/net/NetworkInterface/NetworkInterfaceRetrievalTests.java has 
> been recently modified to perform a deep reflection ( setAccessible(true) ) 
> on a non-public member java.net.NetworkInterface::isBoundInetAddress. 
> However, an illegal access warning is generated by JTReg as a result.
> 
> This fix sets the test to run in 'othervm' mode, and gives 'open' access to 
> 'java.net ' to the unnamed module (where the test lives).
> 
> Further information on this fix can be found here: 
> https://bugs.openjdk.java.net/browse/JDK-8227721 
> 
> 
> Webrev for fix: http://cr.openjdk.java.net/~aefimov/pconcann/8227721/00/ 
> 

Re: [teststabilization] RFR: 8226825: Replace wildcard address with loopback or local host in tests - part 19

[Brian Burkhalter]

Hi Julia,

(part 19 (!))

In AsyncDisconnect line 57 could be replaced with ioe.printStackTrace() to be 
consistent with lines 85 and 88.
Same thing for B6641309 line 58, and B6660405 line 108.

I’ll leave comments on the more substantive changes to the net-dev experts.

Thanks,

Brian

> On Jun 26, 2019, at 10:33 AM, Julia Boes  wrote:
> 
> webrev:
> 
> http://cr.openjdk.java.net/~dfuchs/jboes/webrev_8226825/webrev/ 
> 
> 
> - Replaced wildcard address with loopback or localhost in all tests
> 
> - Replaced URL constructor with URIBuilder
> 
> - Set proxy to null per default for all tests but SetSoLinger
> 
> - Test B6660405 now checks the length of the input stream and throws an 
> AssertionError if the length is not as expected.

Re: RFR [13] 8226730: Missing `@` in code tags

[Brian Burkhalter]

Hi Patrick,

Looks OK to me except the 2017 copyright year in URLStreamHandler needs to be 
2019.

Thanks,

Brian

> On Jun 25, 2019, at 7:36 AM, Patrick  wrote:
> 
> Hi,
> 
> Would it be possible to have my fix for JDK-8226730 reviewed?
> 
> Fix inserts missing '@' symbols in code tags.
> 
> Webrev for fix: http://cr.openjdk.java.net/~aefimov/pconcann/8226730/00/
> 
> Kind regards,
> Patrick
> 

Re: 8223813: (aio) Iocp.getErrorMessage should drop trailing \r\n

[Brian Burkhalter]

Hi Ivan,

> On Jun 12, 2019, at 4:03 PM, Ivan Gerasimov  wrote:
> 
> On 6/12/19 10:02 AM, Brian Burkhalter wrote:
>> Actually, never mind, I am being completely lame here: both NET_ThrowNew() 
>> and the Windows function LocalFree() are robust to a NULL-valued buf so I 
>> think we can just remove the n > 0 or buf == NULL check altogether.
>> 
> That's true, assuming that you initialize buf = NULL and hopping that 
> FormatMessage won't change buf upon failure.

True.

> I wish MSDN were a little bit more specific here.

Likewise.

> I am fine with
> 
> 1)
> 362 TCHAR *buf = NULL;
> 
> 2)
> unconditional
>  395NET_ThrowNew(env, err, buf);
>  396LocalFree(buf);

Barring objections I will check it in with the above code after the JDK 13 fork.

Thanks,

Brian

Re: 8223813: (aio) Iocp.getErrorMessage should drop trailing \r\n

[Brian Burkhalter]

Actually, never mind, I am being completely lame here: both NET_ThrowNew() and 
the Windows function LocalFree() are robust to a NULL-valued buf so I think we 
can just remove the n > 0 or buf == NULL check altogether.

Sorry for the noise: I should have checked this first.

Thanks,

Brian

> On Jun 12, 2019, at 9:51 AM, Brian Burkhalter  
> wrote:
> 
> I am perhaps beating a dead horse here, but how about this instead?
> 
> if (n > 0) {
> NET_ThrowNew(env, err, buf);
> LocalFree(buf);
> } else {
> NET_ThrowNew(env, err, "FormatMessage failed");
> }
> 
> After all, an error *did* occur.

Re: 8223813: (aio) Iocp.getErrorMessage should drop trailing \r\n

[Brian Burkhalter]

Hi Ivan,

I am perhaps beating a dead horse here, but how about this instead?

if (n > 0) {
NET_ThrowNew(env, err, buf);
LocalFree(buf);
} else {
NET_ThrowNew(env, err, "FormatMessage failed");
}

After all, an error *did* occur.

Thanks,

Brian

> On Jun 11, 2019, at 7:11 PM, Ivan Gerasimov  wrote:
> 
> The webrev looks fine to me.
> 
> I think that *most likely* the check if (buf != NULL) will work as expected:  
> buf will only be set to non-NULL value upon success.
> 
> However, the documentation for the function FormatMessage states:
> """
> If the function fails, the return value is zero.
> """
> 
> So, my preference would be if (n > 0).

Re: 8223813: (aio) Iocp.getErrorMessage should drop trailing \r\n

[Brian Burkhalter]

Hi Roger,

Thanks for the $0.02.

> On Jun 11, 2019, at 12:14 PM, Roger Riggs  wrote:
> 
> Having the extraneous suffix consistently removed seems like a good thing.
> Though I'm not sure what the best form of the utility function is:
>  1) return the count of characters to remove
>  2) just truncate the buffer.

I am not sure either nor am I convinced that it would even be worth the effort. 
As mentioned there are already getLastErrorString() and getErrorString() for 
chars so I suppose the equivalent for WCHARs would need to be added for at 
least one of these.

> There may be ways to force some of the errors that would lead to the 
> truncation, but testing all uses will not be practical.

That’s what I thought.

Thanks,

Brian

8218882: NET_Writev is declared, NET_WriteV is defined

[Brian Burkhalter]

Removing NET_Write{V,v} does not break the Unix build, at least for Linux, 
macOS, and Solaris, so perhaps these can in fact be removed [1] as suggested in 
the issue description.

Thanks,

Brian

[1] http://cr.openjdk.java.net/~bpb/8218882/webrev.00/ 

[2] https://bugs.openjdk.java.net/browse/JDK-8218882 

Re: RFR (testbug): 8217882: java/net/httpclient/MaxStreams.java failed once

[Brian Burkhalter]

Hi Daniel,

Perhaps consistently capitalize the first word in each message? Otherwise OK.

No need to refresh.

Thanks,

Brian

> On Jan 28, 2019, at 9:42 AM, Daniel Fuchs  wrote:
> 
> Please find below a patch for:
> 
> 8217882: java/net/httpclient/MaxStreams.java failed once
> https://bugs.openjdk.java.net/browse/JDK-8217882 
> 
> 
> webrev:
> http://cr.openjdk.java.net/~dfuchs/webrev_8217882/webrev.00/ 
> 
> 
> The patch simply instruments the test with
> some more traces so that we might have a better
> diagnosis next time it fails.

Re: RFR 8217740: SocksIPv6Test.java compilation error

[Brian Burkhalter]

Verified against current repo.

+1

Brian

> On Jan 24, 2019, at 10:58 AM, Roger Riggs  wrote:
> 
> Please review a quick fix for a test compilation error, as a side effect of 
> 8216986.
> 
> diff --git a/test/jdk/java/net/Socks/SocksIPv6Test.java 
> b/test/jdk/java/net/Socks/SocksIPv6Test.java
> --- a/test/jdk/java/net/Socks/SocksIPv6Test.java
> +++ b/test/jdk/java/net/Socks/SocksIPv6Test.java
> @@ -173,7 +173,7 @@ public class SocksIPv6Test {
>  server.stop(1);
>  }
>  if (socks != null) {
> -socks.terminate();
> +socks.close();
>  }
>  }
>  }

Re: 8213210: Change ServerSocket(SocketImpl impl) constructor to protected access

[Brian Burkhalter]

> On Oct 31, 2018, at 12:20 PM, Brian Burkhalter  
> wrote:
> 
> -ServerSocket(SocketImpl impl) {
> +protected ServerSocket(SocketImpl impl) {
>  this.impl = impl;
>  impl.setServerSocket(this); // <- NPE if impl == null
>  }

Oops, it can throw null as indicated above. Revised patch below.

Brian

--- a/src/java.base/share/classes/java/net/ServerSocket.java
+++ b/src/java.base/share/classes/java/net/ServerSocket.java
@@ -76,10 +76,16 @@
 private boolean oldImpl = false;
 
 /**
- * Package-private constructor to create a ServerSocket associated with
- * the given SocketImpl.
+ * Creates a server socket with a user-specified {@code SocketImpl}.
+ *
+ * @param  impl an instance of a SocketImpl the subclass
+ * wishes to use on the ServerSocket.
+ *
+ * @throws NullPointerException if impl is {@code null}
+ *
+ * @since 12
  */
-ServerSocket(SocketImpl impl) {
+protected ServerSocket(SocketImpl impl) {
 this.impl = impl;
 impl.setServerSocket(this);
 }

Re: 8213210: Change ServerSocket(SocketImpl impl) constructor to protected access

[Brian Burkhalter]

> On Oct 31, 2018, at 12:16 PM, Alan Bateman  wrote:
> 
> On 31/10/2018 19:13, Brian Burkhalter wrote:
>> https://bugs.openjdk.java.net/browse/JDK-8213210 
>> <https://bugs.openjdk.java.net/browse/JDK-8213210>
>> 
>> Please see diff included below. CSR to follow.
> One thing to check is whether ServerSocket specifies null handling anywhere. 
> I don't think it does so you might have to add an @throws NPE.

SocketImpl.serServerSocket() cannot throw NPE so I don’t see that the updated 
ctor can throw it either:

void setServerSocket(ServerSocket soc) {
this.serverSocket = soc;
}

-ServerSocket(SocketImpl impl) {
+protected ServerSocket(SocketImpl impl) {
 this.impl = impl;
 impl.setServerSocket(this);
 }

Brian

8213210: Change ServerSocket(SocketImpl impl) constructor to protected access

[Brian Burkhalter]

https://bugs.openjdk.java.net/browse/JDK-8213210

Please see diff included below. CSR to follow.

Thanks,

Brian

--- a/src/java.base/share/classes/java/net/ServerSocket.java
+++ b/src/java.base/share/classes/java/net/ServerSocket.java
@@ -76,10 +76,13 @@
 private boolean oldImpl = false;
 
 /**
- * Package-private constructor to create a ServerSocket associated with
- * the given SocketImpl.
+ * Creates a server socket with a user-specified {@code SocketImpl}.
+ *
+ * @param  impl an instance of a SocketImpl the subclass
+ * wishes to use on the ServerSocket.
+ * @since 12
  */
-ServerSocket(SocketImpl impl) {
+protected ServerSocket(SocketImpl impl) {
 this.impl = impl;
 impl.setServerSocket(this);
 }

Re: [XS] RFR: 8209994: windows: Java_java_net_NetworkInterface_getAll misses releasing interface-list

[Brian Burkhalter]

+3

Thanks,

Brian

On Aug 28, 2018, at 12:19 AM, Baesken, Matthias  
wrote:

> looking at the coding, your sceanario ***should*** not happen ;  however 
> to be on the safe side it is for sure better to do the initialization 
> you propose.
> 
> Looking a bit more at the coding,  there is  a 
> Java_java_net_NetworkInterface_getAll_XP  that has similar issues (missing  
> free_netif  calls in case of  "early" returns ).
> I adjusted this as well  in the second  webrev :
> 
> http://cr.openjdk.java.net/~mbaesken/webrevs/8209994.1/

Re: [XS] RFR: 8209994: windows: Java_java_net_NetworkInterface_getAll misses releasing interface-list

[Brian Burkhalter]

Hi Matthias,

I neglected to mention that an appropriate noreg-* label [1], perhaps 
“noreg-cleanup," will be needed on the issue as it seems likely untestable.

Thanks,

Brian

[1] http://openjdk.java.net/guide/changePlanning.html#bug, section 6.

On Aug 27, 2018, at 8:12 AM, Baesken, Matthias  wrote:

> Bug :
>  
> https://bugs.openjdk.java.net/browse/JDK-8209994

Re: [XS] RFR: 8209994: windows: Java_java_net_NetworkInterface_getAll misses releasing interface-list

[Brian Burkhalter]

Hi Matthias,

This looks fine to me.

Thanks,

Brian

On Aug 27, 2018, at 8:12 AM, Baesken, Matthias  wrote:

> Hello,  please  review this small fix ;
>  
> When returning from  Java_java_net_NetworkInterface_getAll   (windows 
> version), we have to  free resources to avoid leaks.
> In some special  cases this is not done .
>  
>  
> Bug :
>  
> https://bugs.openjdk.java.net/browse/JDK-8209994
>  
> change :
>  
> http://cr.openjdk.java.net/~mbaesken/webrevs/8209994/

Re: [12] 8194899: Remove unused sun.net classes

[Brian Burkhalter]

On Jul 2, 2018, at 10:23 AM, Alan Bateman  wrote:

> On 02/07/2018 18:21, Brian Burkhalter wrote:
>> https://bugs.openjdk.java.net/browse/JDK-8194899
>> 
>> The proposed change is to remove these classes which are unused in the JDK:
>> 
>> sun.net.NetworkServer
>> sun.net.URLCanonicalizer
>> 
> Should be okay, assuming the JDK builds and there aren't any tests using it.

It was fine before the 12 fork but I am going to rebuild and re-run the tests 
again to be sure.

Thanks,

Brian

[12] 8194899: Remove unused sun.net classes

[Brian Burkhalter]

https://bugs.openjdk.java.net/browse/JDK-8194899

The proposed change is to remove these classes which are unused in the JDK:

sun.net.NetworkServer
sun.net.URLCanonicalizer

Thanks,

Brian

Re: RFR(XS): 8202181: Correctly specify size of hostname buffer in Unix Inet*AddressImpl_getLocalHostName implementations

[Brian Burkhalter]

Hi Christoph,

This looks OK to me but probably a net-dev engineer should also comment. The 
bug needs a noreg label, e.g., noreg-cleanup.

Brian

On Apr 26, 2018, at 11:56 AM, Langer, Christoph  
wrote:

> Ping, can some reviewer please have a look at this small fix?
>  […]
>  
> Bug: https://bugs.openjdk.java.net/browse/JDK-8202181
> Webrev:http://cr.openjdk.java.net/~clanger/webrevs/8202181.0/

RFR 8198302: VS2017 (C4477) java.base/windows/native/libnet/NetworkInterface_winXP.c incorrect printf format strings

[Brian Burkhalter]

https://bugs.openjdk.java.net/browse/JDK-8198302

Changes are in the diff below.

Thanks,

Brian

--- a/src/java.base/windows/native/libnet/NetworkInterface_winXP.c
+++ b/src/java.base/windows/native/libnet/NetworkInterface_winXP.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -39,14 +39,15 @@
 #ifdef DEBUG
 void printnif (netif *nif) {
 #ifdef _WIN64
-printf ("nif:0x%I64x name:%s\n", nif,nif->name);
+printf ("nif:0x%I64x name:%s\n", (UINT_PTR)nif, nif->name);
 #else
-printf ("nif:0x%x name:%s\n", nif,nif->name);
+printf ("nif:0x%x name:%s\n", nif, nif->name);
 #endif
 if (nif->dNameIsUnicode) {
-printf ("dName:%S index:%d ", nif->displayName,nif->index);
+printf ("dName:%S index:%d ", (unsigned short *)nif->displayName,
+nif->index);
 } else {
-printf ("dName:%s index:%d ", nif->displayName,nif->index);
+printf ("dName:%s index:%d ", nif->displayName, nif->index);
 }
 printf ("naddrs:%d\n", nif->naddrs);
 }

Re: Fix typo in InetSocketAddress.getAddress() documentation

[Brian Burkhalter]

Hi Chris,

Sure, no problem: http://cr.openjdk.java.net/~bpb/8193861/webrev.01/index.html

Brian

On Jan 4, 2018, at 1:13 PM, Chris Hegarty  wrote:

> Would you mind adding the below typo to another you are fixing under 
> JDK-8193861 ( to reduce the overhead ).

Re: RFR: JDK-7155591 - [macosx] regression test java/net/MulticastSocket/SetOutgoingIf.java fail

[Brian Burkhalter]

Hi Mark,

Not exactly my area but I did look over similar code that Chris wrote for [1] 
and your code here looks OK (aside from the more recent copyright year).

Thanks,

Brian

[1] https://bugs.openjdk.java.net/browse/JDK-8158270

On May 4, 2017, at 1:56 PM, Mark Sheppard  wrote:

> Hi,
>please oblige and review the following change
> http://cr.openjdk.java.net/~msheppar/7155591/webrev/
> 
> to address the issue raised in
> https://bugs.openjdk.java.net/browse/JDK-7155591
> 
> this identifies that the awdl interface causes issues for the SetOutgoingIf 
> multicast socket test, and
> the proposed solution is exclude it from test scenarios. As such, the change
> adds a check to ignore this network interface during interface selection.
> 
> regards
> Mark

Re: RFR [9] 8178161: Default multicast interface on Mac

[Brian Burkhalter]

On Apr 6, 2017, at 11:07 AM, Chris Hegarty  wrote:

>> On 6 Apr 2017, at 17:50, Michael McMahon  
>> wrote:
>> 
>> Looks fine to me Chris. Stylistically, the boolean tests
>> in line 104 could remove the == true obviously, but not a big deal.
> 
> Thanks, I’ll make this change before pushing.

+1

Thanks,

Brian

Re: RFR 8175305: Typos in net.properties

[Brian Burkhalter]

+1

On Feb 21, 2017, at 8:01 AM, Pavel Rappo  wrote:

> Hello,
> 
> Could you please review the following trivial change for [1]?
> 
>   http://cr.openjdk.java.net/~prappo/8175305/webrev.00/
> 
> Thanks,
> -Pavel
> 
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8175305
> 

Re: RFR JDK-8161091 Incorrect Stream.FlowControl implementation allows to send DataFrame even when window size was exhausted

[Brian Burkhalter]

Hi Sergey,

This is not my area of expertise so probably a second Reviewer would be in 
order here but the changes appear straightforward.

On Jul 8, 2016, at 2:40 PM, Sergey Kuksenko  wrote:

> Could you please review the following fix for JDK-8161091?
> 
> http://cr.openjdk.java.net/~skuksenko/jep110/8161091/webrev.00/
> https://bugs.openjdk.java.net/browse/JDK-8161091
> 
> Existing Stream.FlowControl implementation doesn't decrease amount of permits 
> if requested amount is less than permits. That means that client may send any 
> amount of data if data frame size less than window size.

This looks correct.

Also per the HTTP2 specification [1]

"All implementations MUST be capable of receiving and minimally processing 
frames up to 214 octets in length, *plus* the 9-octet frame header”

so the removal of the subtraction of 9 from the payload length looks correct.

Thanks,

Brian

[1] http://httpwg.org/specs/rfc7540.html#FrameSize

RFC on 8146041: java.net.URLConnection.guessContentTypeFromStream() does not recognize TIFF streams

[Brian Burkhalter]

This core-libs-dev RFR

http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-December/037847.html

was already approved on core-libs-dev but I would like to make sure there is no 
objection from the net-dev community.

Thanks,

Brian

[9] RFR of 8129499: Structure of java/rmi/activation/rmidViaInheritedChannel tests masks exception

[Brian Burkhalter]

Please review at your convenience.

Issue:  https://bugs.openjdk.java.net/browse/JDK-8129499
Patch:  http://cr.openjdk.java.net/~bpb/8129499/webrev.00/

Summary: The instance variable ‘rmid’ is never initialized due to some error 
which occurs before the statement which would initialize it but it is 
dereferenced in the finally block which causes any exception within the try 
block to be suppressed.

This issue blocks obtaining further information from tests runs with respect to 
https://bugs.openjdk.java.net/browse/JDK-8077668.

Thanks,

Brian

Fwd: [9] RFR of 8129499: Structure of java/rmi/activation/rmidViaInheritedChannel tests masks exception

[Brian Burkhalter]

Sorry: posted to wrong list.

Re-direct this thread to: 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-June/034223.html

Begin forwarded message:

 From: Brian Burkhalter brian.burkhal...@oracle.com
 Subject: [9] RFR of 8129499: Structure of 
 java/rmi/activation/rmidViaInheritedChannel tests masks exception
 Date: June 22, 2015 at 1:03:07 PM PDT
 To: net-dev@openjdk.java.net
 
 Please review at your convenience.
 
 Issue:https://bugs.openjdk.java.net/browse/JDK-8129499
 Patch:http://cr.openjdk.java.net/~bpb/8129499/webrev.00/
 
 Summary: The instance variable ‘rmid’ is never initialized due to some error 
 which occurs before the statement which would initialize it but it is 
 dereferenced in the finally block which causes any exception within the try 
 block to be suppressed.
 
 This issue blocks obtaining further information from tests runs with respect 
 to https://bugs.openjdk.java.net/browse/JDK-8077668.
 
 Thanks,
 
 Brian

Re: [9] RFR of 8074937: ServerSocket.accept() API Descriptions should include past tense

[Brian Burkhalter]

Identical verbiage is in fact also present in

http://docs.oracle.com/javase/8/docs/api/java/net/ServerSocket.html#ServerSocket-int-int-

and

http://docs.oracle.com/javase/8/docs/api/java/net/ServerSocket.html#ServerSocket-int-int-java.net.InetAddress-

To wit:

The backlog argument is the requested maximum number of pending connections on 
the socket. Its exact semantics are implementation specific. In particular, an 
implementation may impose a maximum length or may choose to ignore the 
parameter altogther. The value provided should be greater than 0. If it is less 
than or equal to 0, then an implementation specific default will be used.

Brian

On Mar 19, 2015, at 7:47 AM, Alan Bateman alan.bate...@oracle.com wrote:

 In ServerSocketChannel.bind then it is documented as:
 
  * p The {@code backlog} parameter is the maximum number of pending
  * connections on the socket. Its exact semantics are implementation 
 specific.
  * In particular, an implementation may impose a maximum length or may 
 choose
  * to ignore the parameter altogether.

[9] RFR of 8074937: ServerSocket.accept() API Descriptions should include past tense

[Brian Burkhalter]

Please review at your convenience. This a doc-only change.

Issue:  https://bugs.openjdk.java.net/browse/JDK-8074937
Patch:  http://cr.openjdk.java.net/~bpb/8074937/webrev.00/

This is in effect an amplification of extant javadoc suggested as an 
enhancement by an external user. I performed some testing to verify the 
accuracy of the suggested documentation change prior to crafting the verbiage, 
and the criticism does seem to be well-founded in the behavior of the APIs in 
question.

It would be good however if someone with a more profound knowledge of this area 
were to review the proposed documentation update for accuracy, especially as it 
is not unlikely that there might be platform-specific behavior with which these 
statements conflict.

Thanks,

Brian

hg: jdk8/tl/jdk: 2 new changesets

[brian . burkhalter]

Changeset: d3c4e8fe98c3
Author:bpb
Date:  2013-12-05 07:44 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/d3c4e8fe98c3

8029514: java/math/BigInteger/BigIntegerTest.java failing since thresholds 
adjusted in 8022181
Summary: Ensure the value returned by getLower() is unsigned.
Reviewed-by: darcy

! src/share/classes/java/math/BigInteger.java
! test/java/math/BigInteger/BigIntegerTest.java

Changeset: 303f4bccfca2
Author:bpb
Date:  2013-12-05 07:45 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/303f4bccfca2

8029501: BigInteger division algorithm selection heuristic is incorrect
Summary: Change Burnikel-Ziegler division heuristic to require that the 
dividend int-length exceed that of the divisor by a minimum amount.
Reviewed-by: darcy

! src/share/classes/java/math/BigInteger.java
! src/share/classes/java/math/MutableBigInteger.java

hg: jdk8/tl/jdk: 8022181: Tune algorithm crossover thresholds in BigInteger

[brian . burkhalter]

Changeset: c138b0d33980
Author:bpb
Date:  2013-12-03 12:25 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/c138b0d33980

8022181: Tune algorithm crossover thresholds in BigInteger
Summary: Change multiplication, squaring, division, and base conversion 
thresholds to values which retain performance improvement in most cases but 
with a a lower overall risk of regression.
Reviewed-by: darcy

! src/share/classes/java/math/BigInteger.java

hg: jdk8/tl/jdk: 8027625: test/java/math/BigInteger/ExtremeShiftingTests.java needs @run tag to specify heap size

[brian . burkhalter]

Changeset: 92fb6baaebc4
Author:bpb
Date:  2013-11-04 08:05 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/92fb6baaebc4

8027625: test/java/math/BigInteger/ExtremeShiftingTests.java needs @run tag to 
specify heap size
Summary: Add @run tag to specify heap size
Reviewed-by: alanb, dxu

! test/java/math/BigInteger/ExtremeShiftingTests.java

hg: jdk8/tl/jdk: 6910473: java.math.BigInteger.bitLength() may return negative int on large numbers; ...

[brian . burkhalter]

Changeset: 0734e1584d9d
Author:bpb
Date:  2013-10-30 17:45 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/0734e1584d9d

6910473: java.math.BigInteger.bitLength() may return negative int on large 
numbers
8021203: BigInteger.doubleValue/floatValue returns 0.0 instead of Infinity
8021204: Constructor BigInteger(String val, int radix) doesn't detect overflow
8022780: Incorrect BigInteger division because of MutableBigInteger.bitLength() 
overflow
Summary: Prevent construction of overflowed BigIntegers.
Reviewed-by: bpb, darcy, psandoz
Contributed-by: Dmitry Nadezhin dmitry.nadez...@oracle.com

! src/share/classes/java/math/BigInteger.java
! src/share/classes/java/math/MutableBigInteger.java
+ test/java/math/BigInteger/BitLengthOverflow.java
+ test/java/math/BigInteger/DivisionOverflow.java
+ test/java/math/BigInteger/DoubleValueOverflow.java
! test/java/math/BigInteger/ExtremeShiftingTests.java
+ test/java/math/BigInteger/StringConstructorOverflow.java
+ test/java/math/BigInteger/SymmetricRangeTests.java

hg: jdk8/tl/jdk: 8026806: Incomplete test of getaddrinfo() return value could lead to incorrect exception for Windows Inet 6

[brian . burkhalter]

Changeset: 9758edb6976f
Author:bpb
Date:  2013-10-22 10:44 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/9758edb6976f

8026806: Incomplete test of getaddrinfo() return value could lead to incorrect 
exception for Windows Inet 6
Summary: Check getaddrinfo return value before calling WSAGetLastError.
Reviewed-by: alanb, dsamersoff

! src/windows/native/java/net/Inet6AddressImpl.c

hg: jdk8/tl/jdk: 7179567: JCK8 tests: api/java_net/URLClassLoader/index.html#Ctor3 failed with NPE; ...

[brian . burkhalter]

Changeset: 2be08cdd1ced
Author:bpb
Date:  2013-10-22 11:25 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/2be08cdd1ced

7179567: JCK8 tests: api/java_net/URLClassLoader/index.html#Ctor3 failed with 
NPE
6445180: URLClassLoader does not describe the behavior of several methods with 
respect to null arguments
Summary: Document when a NPE will be thrown by URLClassLoader constructors, 
newInstance(), findClass(), and getPermissions().
Reviewed-by: alanb, mduigou, chegar, dholmes, jrose

! src/share/classes/java/net/URLClassLoader.java
! src/share/classes/javax/management/remote/rmi/NoCallStackClassLoader.java
! src/share/classes/sun/applet/AppletClassLoader.java
+ test/java/net/URLClassLoader/NullURLTest.java

Re: JDK 8 RFR 8010371: getaddrinfo can fail with EAI_SYSTEM/EAGAIN, causes UnknownHostException to be thrown

[Brian Burkhalter]

Sorry about that. Obviously I still need to learn some things about the 
process, e.g., review the prescribed workflow.

Thanks,

Brian

On Oct 16, 2013, at 7:48 PM, David Holmes wrote:

 Brian,
 
 On 17/10/2013 9:21 AM, Brian Burkhalter wrote:
 Dmitry,
 
 I think you are correct: that slipped by both me and the reviewers. I have 
 reopened the issue and posted an amendment to the original webrev here:
 
 You can not reopen a bug once it has been fixed! You need to create a new bug 
 for this issue and re-close the original ensuring that you restore all the 
 correct field values ie Fixed in build.
 
 David
 
 http://cr.openjdk.java.net/~bpb/8010371/webrev.4-amendment/
 
 Thanks for catching this.
 
 Brian
 
 On Oct 16, 2013, at 3:06 PM, Dmitry Samersoff wrote:
 
 You have to check error != 0 before call to WSAGetLastError() at
 ll. 134 Inet6AddressImpl.c
 
 Besides that - the fix looks good for me.
 
 Here's the patch updated for this option:
 
 http://cr.openjdk.java.net/~bpb/8010371/webrev.4/
 

Re: JDK 8 RFR 8010371: getaddrinfo can fail with EAI_SYSTEM/EAGAIN, causes UnknownHostException to be thrown

[Brian Burkhalter]

On Oct 17, 2013, at 5:37 AM, Alan Bateman wrote:

 On 17/10/2013 00:21, Brian Burkhalter wrote:
 Dmitry,
 
 I think you are correct: that slipped by both me and the reviewers. I have 
 reopened the issue and posted an amendment to the original webrev here:
 
 http://cr.openjdk.java.net/~bpb/8010371/webrev.4-amendment/
 
 I've restored the bug fields and I assume you'll create a new bug for the 
 follow-up. Sorry this was missed in the original review (probably because it 
 went through so many iterations).
 
 -Alan.

Yes I will create a new one, thanks.

Brian

JDK 8 RFR 8026806: Incomplete test of getaddrinfo() return value could lead to incorrect exception for Windows Inet 6

[Brian Burkhalter]

Continuing the discussion from 
http://mail.openjdk.java.net/pipermail/net-dev/2013-October/007574.html under 
new issue ID:

Issue:  https://bugs.openjdk.java.net/browse/JDK-8026806
Webrev: http://cr.openjdk.java.net/~bpb/8026806/webrev/

Thanks,

Brian

On Oct 17, 2013, at 8:06 AM, Brian Burkhalter wrote:

 
 On Oct 17, 2013, at 5:37 AM, Alan Bateman wrote:
 
 On 17/10/2013 00:21, Brian Burkhalter wrote:
 Dmitry,
 
 I think you are correct: that slipped by both me and the reviewers. I have 
 reopened the issue and posted an amendment to the original webrev here:
 
 http://cr.openjdk.java.net/~bpb/8010371/webrev.4-amendment/
 
 I've restored the bug fields and I assume you'll create a new bug for the 
 follow-up. Sorry this was missed in the original review (probably because it 
 went through so many iterations).
 
 -Alan.
 
 Yes I will create a new one, thanks.
 
 Brian

hg: jdk8/tl/jdk: 8026832: Clean up straggling doclint warnings in java.math

[brian . burkhalter]

Changeset: e76bb2436b04
Author:bpb
Date:  2013-10-17 15:05 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e76bb2436b04

8026832: Clean up straggling doclint warnings in java.math
Summary: Fix empty paragraph tag warnings.
Reviewed-by: lancea

! src/share/classes/java/math/BigDecimal.java
! src/share/classes/java/math/RoundingMode.java

Re: JDK 8 RFR 8010371: getaddrinfo can fail with EAI_SYSTEM/EAGAIN, causes UnknownHostException to be thrown

[Brian Burkhalter]

Dmitry,

I think you are correct: that slipped by both me and the reviewers. I have 
reopened the issue and posted an amendment to the original webrev here:

http://cr.openjdk.java.net/~bpb/8010371/webrev.4-amendment/

Thanks for catching this.

Brian

On Oct 16, 2013, at 3:06 PM, Dmitry Samersoff wrote:

 You have to check error != 0 before call to WSAGetLastError() at
 ll. 134 Inet6AddressImpl.c
 
 Besides that - the fix looks good for me.
 
 Here's the patch updated for this option:
 
 http://cr.openjdk.java.net/~bpb/8010371/webrev.4/

Re: JDK 8 RFR 8010371: getaddrinfo can fail with EAI_SYSTEM/EAGAIN, causes UnknownHostException to be thrown

[Brian Burkhalter]

On Oct 15, 2013, at 6:21 AM, Alan Bateman wrote:

 Here's the patch updated for this option:
 
 http://cr.openjdk.java.net/~bpb/8010371/webrev.4/http://cr.openjdk.java.net/%7Ebpb/8010371/webrev.4/
 
 I think this is okay, the only concern is that the host name is no longer 
 guaranteed to be in the exception detail (but for WSATRY_AGAIN then it might 
 not matter because the problem is unlikely to be specific to the host name 
 that is being looked up).

The hostname is guaranteed to be in the exception detail:

void
NET_ThrowByNameWithLastError(JNIEnv *env, const char *name,
   const char *defaultDetail) {
char errmsg[255];
sprintf(errmsg, errno: %d, error: %s\n, WSAGetLastError(), defaultDetail);
JNU_ThrowByNameWithLastError(env, name, errmsg);
}

The hostname string is passed to the third parameter in the above method.

Brian

hg: jdk8/tl/jdk: 8010371: getaddrinfo can fail with EAI_SYSTEM/EAGAIN, causes UnknownHostException to be thrown

[brian . burkhalter]

Changeset: 3676f04e6553
Author:bpb
Date:  2013-10-15 16:45 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3676f04e6553

8010371: getaddrinfo can fail with EAI_SYSTEM/EAGAIN, causes 
UnknownHostException to be thrown
Summary: Modify UHE exception message for EAI_AGAIN failures.
Reviewed-by: alanb, chegar, michaelm, dsamersoff
Contributed-by: Brian Burkhalter brian.burkhal...@oracle.com

! src/solaris/native/java/net/Inet4AddressImpl.c
! src/windows/native/java/net/Inet4AddressImpl.c
! src/windows/native/java/net/Inet6AddressImpl.c

Re: JDK 8 RFR 8010371: getaddrinfo can fail with EAI_SYSTEM/EAGAIN, causes UnknownHostException to be thrown

[Brian Burkhalter]

On Oct 14, 2013, at 1:58 AM, Alan Bateman wrote:

 2) In Inet4AddressImpl.c and Inet6AddressImpl.c replace 
 NET_ThrowUnknownHostExceptionWithGaiError with NET_ThrowByNameWithLastError 
 (see net_md_util.c).
 
 […]
 
 If the con of option 2 is acceptable then I think that would be the best 
 way to go, otherwise option 1.
 
 Option #2 seems reasonable, the exception messages for similar network 
 conditions are rarely the same on Windows and Unix anyway.

Here's the patch updated for this option:

http://cr.openjdk.java.net/~bpb/8010371/webrev.4/

 However I think it's important to have verified it with one or two errors to 
 be confident that the errors translate as expected.

I can do this if we are actually going with this change for JDK 8.

 One other thing to add is that winsock_errors dates from early versions of 
 Windows whether there wasn't a means to translate Windows Sockets errors. We 
 should look at eliminating it (not for JDK 8 of course, it's too late) so 
 that all errors are handle translated consistently.

See https://bugs.openjdk.java.net/browse/JDK-4842142.

Brian

Re: JDK 8 RFR 8010371: getaddrinfo can fail with EAI_SYSTEM/EAGAIN, causes UnknownHostException to be thrown

[Brian Burkhalter]

On Oct 10, 2013, at 6:52 AM, Alan Bateman wrote:

 Thanks for persevering with this, we are on the right road now.
 
 I just looked at JDK-8010371 and the OS field is set to linux. I don't 
 know if this is right but as IPv6 is usually enabled by default these days 
 then I have to guess that the person that submitted the bug has IPv6 disabled 
 or is running with -Djava.net.preferIPv4Stack=true, otherwise it would be a 
 non-issue (as you have discovered).
 
 The other thing about your observation is that 
 ThrowUnknownHostExceptionWithGaiError is creating the UHE with the both the 
 hostname and the error message. This resolves to the concern in one or two of 
 the mails that the UHE names the exception message host and that someone 
 might assume that the exception detail is the hostname.
 
 So thumbs up on the update to src/solaris/native/java/net/Inet4Address.c.

Thanks.
 For Windows I added a similar NET_ ThrowUnknownHostExceptionWithGaiError and 
 modified Inet{4,6} to mimic the Unix case. Note that the Windows code has 
 not yet been compiled pending comments.
 
 Inet4AddressImpl.lookupAllHostAddr uses gethostbyname and when I look at the 
 MSDN documentation then I don't see EAI_AGAIN as possible error. It does list 
 WSATRY_AGAIN and I'm wondering if they have the same value and whether the 
 WSA* error is only returned by WSAGetLastError?

This symbolic name definition

#define EAI_AGAIN   WSATRY_AGAIN

is at line 99 of net_util_mh. which is included by net_util.h hence the two 
aforementioned values are the same.

 However I think we have a problem with using gai_strerror here as the MSDN 
 documentation says that it is not thread safe. This means we may have to look 
 for a WSA equivalent or maybe drop the Windows part of the fix (using a mutex 
 of some synchronization might be possible but the scope would require 
 research and of course it wouldn't work with 3rd party native code that is 
 also using it).


Yes, I read about the thread safety. There are other ways to do this but I 
wanted first to air the general idea using gai_strerror() before doing any 
further work.

 getaddrinfo could return EAI_AGAIN[1]
 
 But recomended way is to just check return value of getaddrinfo and then
 use WSA functions especially, FormatMessage[2] instead of gai_strerror.

Thanks, I did read about FormatMessage() but forewent the usage thereof for the 
just stated reason.

 Also we may consider rewriting this code using GetAddrInfoW sometimes in
 a future to support IDN.
 
 PS:
 Brian, please notice mixing WSA and EAI at ll. 137.

Agreed, that indeed should be changed as EAI_AGAIN - WSATRY_AGAIN.

Thanks,

Brian

Re: JDK 8 RFR 8010371: getaddrinfo can fail with EAI_SYSTEM/EAGAIN, causes UnknownHostException to be thrown

[Brian Burkhalter]

On Oct 10, 2013, at 12:37 PM, Brian Burkhalter wrote:

 Yes, I read about the thread safety. There are other ways to do this but I 
 wanted first to air the general idea using gai_strerror() before doing any 
 further work.
 
 […]
 
 Thanks, I did read about FormatMessage() but forewent the usage thereof for 
 the just stated reason.

Assuming that the general idea is correct, there are a few possible 
alternatives for a final version for Windows. I am not concerned in this 
discussion about picky details including naming which can be fine tuned at the 
end.

1) In net_util_mh.c/NET_ThrowUnknownHostExceptionWithGaiError replace 
gai_strerror() with FormatMessage().

2) In Inet4AddressImpl.c and Inet6AddressImpl.c replace 
NET_ThrowUnknownHostExceptionWithGaiError with NET_ThrowByNameWithLastError 
(see net_md_util.c).

3) In net_md_util.c change (line 97 in 
http://cr.openjdk.java.net/~bpb/8010371/webrev.3/ version)

{ WSATRY_AGAIN, 0,  Nonauthoritative host not found },

to

{ WSATRY_AGAIN, UnknownHostException,  Nonauthoritative 
host not found },

and in Inet4AddressImpl.c and Inet6AddressImpl.c replace 
NET_ThrowUnknownHostExceptionWithGaiError with NET_ThrowNew().

4) In NET_ThrowUnknownHostExceptionWithGaiError instead of using gai_strerror() 
use the hard-coded error message Nonauthoritative host not found.

The pros and/or cons for each, respectively, are

1) Most closely matches the Unix version and avoids hard-coded messages.

2) Simple and obviates the need for 
NET_ThrowUnknownHostExceptionWithGaiError(), but the exception message has a 
different format from the Unix version and contains the error code instead of 
an error string.

3) Simple and obviates the need for 
NET_ThrowUnknownHostExceptionWithGaiError(), but the exception message format 
is slightly different from the Unix version and the change could have the 
undesirable (and unpredictable) side effect of an UnknownHostException being 
thrown somewhere else where a SocketException used to be thrown.

4) Simplest option but has distasteful hard-coding (as do option 3 indirectly 
and NET_ThrowNew() for that matter).

If the con of option 2 is acceptable then I think that would be the best way 
to go, otherwise option 1.

Thanks,

Brian

hg: jdk8/tl/jdk: 8016252: More defensive HashSet.readObject

[brian . burkhalter]

Changeset: b86e6700266e
Author:bpb
Date:  2013-10-09 11:47 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/b86e6700266e

8016252: More defensive HashSet.readObject
Summary: Add data validation checks in readObject().
Reviewed-by: alanb, mduigou, chegar
Contributed-by: Brian Burkhalter brian.burkhal...@oracle.com

! src/share/classes/java/util/HashSet.java
+ test/java/util/HashSet/Serialization.java

Re: JDK 8 RFR 8010371: getaddrinfo can fail with EAI_SYSTEM/EAGAIN, causes UnknownHostException to be thrown

[Brian Burkhalter]

On Oct 9, 2013, at 11:16 AM, Brian Burkhalter wrote:

 For Windows I added a similar NET_ ThrowUnknownHostExceptionWithGaiError and 
 modified Inet{4,6} to mimic the Unix case. Note that the Windows code has not 
 yet been compiled pending comments.

Since compiled successfully on Windows but holding off on JPRT until there are 
comments.

Thanks,

Brian

hg: jdk8/tl/jdk: 7189139: BigInteger's staticRandom field can be a source of bottlenecks.

[brian . burkhalter]

Changeset: 673f8045311e
Author:bpb
Date:  2013-10-09 17:22 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/673f8045311e

7189139: BigInteger's staticRandom field can be a source of bottlenecks.
Summary: Use ThreadLocalRandom instead of SecureRandom.
Reviewed-by: shade, psandoz
Contributed-by: Brian Burkhalter brian.burkhal...@oracle.com

! src/share/classes/java/math/BigInteger.java

Re: JDK 8 RFR 8010371: getaddrinfo can fail with EAI_SYSTEM/EAGAIN, causes UnknownHostException to be thrown

[Brian Burkhalter]

Resuming this discussion …

Thanks for the previous comments. My feeling at this point is to do one of two 
things:

A) defer to something after JDK 8, or
B) on EAI_AGAIN do not retry but set the cause of the UAE to new 
SomeException(gai_strerror(error)) where SomeException could be for example 
Exception, RuntimeException, or IOException.

Comments?

Thanks,

Brian

Re: JDK 8 RFR 8010371: getaddrinfo can fail with EAI_SYSTEM/EAGAIN, causes UnknownHostException to be thrown

[Brian Burkhalter]

All,

Please see comments inline.

Thanks,

Brian

On Oct 1, 2013, at 8:44 PM, Alan Bateman wrote:

 On 01/10/2013 12:46, Brian Burkhalter wrote:
 :
 
 I updated the webrev
 
 http://cr.openjdk.java.net/~bpb/8010371/
 
 with changes in the test of the return value of getaddrinfo for Unix Inet 4 
 and 6 and Windows Inet 6. The usual testing is in progress.
 
 Brian
 This looks better, although I think I would reverse re-write the expressions 
 to if (error = ...).

I agree that is an ugly style but it guarantees that mistakes such as

if (error = EAI_AGAIN) {}

are caught at compilation time.

 One thing to consider is whether this condition is really worth introducing 
 HostLookupException, particularly when it doesn't include additional 
 information (except to distinguish it from its supertype). If a new exception 
 is really needed then maybe it could add the error message obtained from 
 gai_strerror, alternatively maybe you could considered setting the cause of 
 the UHE to something like an IOException with the translation of the error.

On Oct 2, 2013, at 7:40 AM, Chris Hegarty wrote:

 On 02/10/2013 04:44, Alan Bateman wrote:
 
 One thing to consider is whether this condition is really worth
 introducing HostLookupException, particularly when it doesn't include
 
 I am also not convinced of the merits of adding a new public checked 
 Exception type for this situation. Do we expect all callers of API's that can 
 throw UKE to now have to catch this Exception and provide their own try 
 logic? Otherwise, what do we expect them to do with it.
 
 I cannot see the original webrev, so cannot comment on the retry logic that 
 was there. Do you still have it around? Can upload as a .05 version? The 
 reason I ask is that, IMHO, I would prefer this type of approach over the new 
 Exception type, but that could be just me.

No, I do not have it.

On Oct 2, 2013, at 7:58 AM, Michael McMahon wrote:

 On 02/10/13 15:40, Chris Hegarty wrote:
 
 I am also not convinced of the merits of adding a new public checked 
 Exception type for this situation. Do we expect all callers of API's that 
 can throw UKE to now have to catch this Exception and provide their own try 
 logic? Otherwise, what do we expect them to do with it.
 
 
 It's proposed as a subclass of UnknownHostException. So, nobody would have to 
 catch it.
 
 I suppose another approach, which is a variant of the first one suggested, 
 would be
 retry logic built in, which is switched off by default and configurable 
 somehow.

I personally do not like the idea of the configurable approach.

On Oct 2, 2013, at 8:12 AM, Chris Hegarty wrote:

 On 02/10/2013 15:58, Michael McMahon wrote:
 
 It's proposed as a subclass of UnknownHostException. So, nobody would
 have to catch it.
 
 Right, understood. But unless someone adds explicit handling code when there 
 is no change in existing behavior.

Well this is good in the sense that existing code would not be affected 
negatively but it would require that code be changed for the application 
explicitly to do a retry.

 I suppose another approach, which is a variant of the first one
 suggested, would be
 retry logic built in, which is switched off by default and configurable
 somehow.
 
 I did not see the retry webrev so cannot comment on the specifics, but I 
 don't see any issue with a single untimed retry, if that fails throw an UHE 
 with a specific cause.

This is close to what I original thought to do modulo the exception class.

So, how about this approach:

1) If the error is EAI_AGAIN / EIA_SYSTEM+EAGAIN / WSATRY_AGAIN then do one 
immediate native retry.
2) If the retry fails with the same error, then throw a UHE with a specific 
message or cause.

Questions:

A) Would it be better to do the retry in the Java layer, perhaps with a very 
short wait?
B) Should the message or cause in #2 be explicitly document in the javadoc?

JDK 8 RFR 8010371: getaddrinfo can fail with EAI_SYSTEM/EAGAIN, causes UnknownHostException to be thrown

[Brian Burkhalter]

Hello net-dev members,

Please review this proposed fix at your convenience.

Summary
When looking up a host and an EAGAIN error is encountered, throw an instance of 
the new HostLookupException subclass of UnknownHostException.

Issue
https://bugs.openjdk.java.net/browse/JDK-8010371

Webrev
http://cr.openjdk.java.net/~bpb/8010371

If this fix appears acceptable I will file a CCC request.

Thanks,

Brian

Re: JDK 8 RFR 8010371: getaddrinfo can fail with EAI_SYSTEM/EAGAIN, causes UnknownHostException to be thrown

[Brian Burkhalter]

On Oct 1, 2013, at 11:59 AM, Brian Burkhalter wrote:

 It seems a bit unclear to me and to depend on which system one is on. There 
 is also the possibility apparently of it returning EAI_AGAIN. It might be 
 best to test both the return value and if that is EAI_SYSTEM to test errno.

I updated the webrev

http://cr.openjdk.java.net/~bpb/8010371/

with changes in the test of the return value of getaddrinfo for Unix Inet 4 and 
6 and Windows Inet 6. The usual testing is in progress.

Brian