Re: RFR 15 8243099: SO_INCOMING_NAPI_ID support

[Daniel Fuchs]

Hi Vladimir,

On 06/05/2020 00:36, Ivanov, Vladimir A wrote:

I was failed to get non-zero value for the ServerSocket and according to 
clarification from the network experts:
" NAPI IDs is not marked on listening sockets. A listening socket can accept 
multiple connections coming on different queues. So even if we do it, it can keep 
changing and we don't see a use case for it. Once a connection is accepted, accepted or 
the child socket will return a valid NAPI id."

So seems the listener sockets will have this as zero.


Thanks for this clarifications! Could you also clarify how
this is supposed to work with UDP sockets?

I mean - does the socket needs to be connected to make
the best use of this feature?

If not connected, will a listening UDP socket always be
assigned to the same device queue regardless of the source
of the datagrams it receives or might the device queue id
change with each incoming packet depending on its source?

I am especially intrigued by the case where a DatagramSocket
is bound to the wildcard address and as such potentially
able to receive datagrams comming through any active NIC.

best regards,

-- daniel

Re: 8244205: HTTP/2 tunnel connections through proxy may be reused regardless of which proxy is selected

[Chris Hegarty]

> On 1 May 2020, at 15:38, Daniel Fuchs  wrote:
> 
> Hi,
> 
> Please find below a fix for:
> 8244205: HTTP/2 tunnel connections through proxy may be
> reused regardless of which proxy is selected
> https://bugs.openjdk.java.net/browse/JDK-8244205
> 
> webrev:
> http://cr.openjdk.java.net/~dfuchs/webrev_8244205/webrev.00/index.html

Thank you Daniel. This change looks good to me.

-Chris.

Re: RFC: 8132359: JarURLConnection.getJarFile() resource leak when file is not found

[Michael McMahon]

Hi,

Yes, we've had some discussion about it internally, and while others may
yet have an opinion, I think this approach is a reasonable one, with a 
spec change

that captures the behavior.

The main thing we need to capture is that while connect() to a non 
existing resource
may fail, getJarFile() can still return a reference to the containing 
JAR file if that is accessible.


There could be other subtleties like what happens if the JAR file is not 
accessible,
is null a reasonable return value in that case? I think that could 
happen and might

need to be specified.

Michael.

On 01/05/2020 10:32, Alex Kashchenko wrote:

Hi,

Pinging politely about this issue.

I wonder whether it would be appropriate to file a CSR about 
getJarFile() behaviour change at this point?


Concise description of the change is:

"So should the getJarFile return be a reference to JarFile for an URL 
specifying an non existent entry  i.e. the resource doesn't exist?"



On 04/02/2020 12:14 AM, mark sheppard wrote:

  p.s.

I had also meant to say in the response below, that the proposed 
getJaFile solution is
perfectly reasonable and would allow a recoverable strategy in an 
related
scenario where a URLConnection:: connect,  for the same type of entry 
URL,

throws a FNFException resulting in the same type of "resource leak".
But, in this case, with the proposed change the JarFile can be 
retrieved and closed.


regards
Mark




From: net-dev  on behalf of mark 
sheppard 

Sent: Wednesday 1 April 2020 16:03
To: Michael McMahon ; Alex Kashchenko 

Cc: Mark Sheppard ; 
net-dev@openjdk.java.net >> OpenJDK Network Dev list 

Subject: Re: RFC: 8132359: JarURLConnection.getJarFile() resource 
leak when file is not found


Hi Michael et al.,
 just looking at the webrev ... the change in URLClassPath seems 
reasonable.
The change in JarURLConnection  has implications and would change the 
semantics of

the getJarFile method.

using the example accompanying this JBS item to demonstrate an issue 
caused by the caching mechanism
within the URLConnection framework, it should be noted that the jar 
URL is referencing
an non existent entry in the jar file entry. Thus some form of 
exception would be anticipated in this scenario.


With the proposed change, this will return a JarFile regardless of 
whether a referenced resource (URL) exists or not.


Examining  the call flows it can be observed that
the getJarFile invokes connect. This will retrieve a jar file via 
JarFileFactory
  and then, because the URL references an entry in the jar file, 
attempts
  to access the entry resulting in a  null return,  which generates a 
FNF exception to be thrown.


Also note that an explicit invocation of connect on the 
JarURLConnection instance will result in the FNFException.


the change in the JarURLConnection will return a jar file in this 
test scenario and not the FNF exception. Based on the methods

spec is that acceptable behaviour change?


public abstract JarFile getJarFile throws IOException

Return the JAR file for this connection.

Returns:
the JAR file for this connection. If the connection is a connection 
to an entry of a JAR file, the JAR file object is returned

Throws:
IOException 
- if an IOException occurs while trying to connect to the JAR file 
for this connection.

See Also:
URLConnection.connect() 


and connect says
"Opens a communications link to the resource referenced by this URL, 
if such a connection has not already been established."


So should the getJarFile return be a reference to JarFile for an URL 
specifying an non existent entry  i.e. the resource doesn't exist?
Is the resource associated with a JarURLConnection the encapsulated 
JarFile? Or the target in the specified  in the URL ?


There are a series of similar behaviour anomalies in this area 
URL/URLConnection/JarURLConnection (on Windows) and in general they
can be attributed to the internal caching  mechanism, which is 
enabled OOTB, and its impact on the closing of file resource in 
Exception conditions scenarios.


Disabling caching for jar protocol , which will allow consistent and 
correct behaviour is one possibility.


As such an alternative or workaround  is to disable caching for the 
jar protocol
via the URLConnection::setDefaultUseCaches() on Windows where an 
application

may want to delete a jar file resource, for example:

URLConnection.setDefaultUseCaches("jar", false);

best regards
Mark


From: net-dev  on behalf of Michael 
McMahon 

Sent: Monday 16 March 2020 12:39
To: Alex Kashchenko 
Cc: net-dev@openjdk.java.net >> OpenJDK Network Dev list 

Subject: Re: RFC: 8132359: JarURLConnection.getJarFile() resource 
leak when file is not found


Hi Alex,

(and redirecting the thread to net-dev)

It looks like a straight forward sol

RFR 8240666: Websocket client’s OpeningHandshake discards the HTTP response body

[Rahul]

  Hello,

 

  Request to have my fix reviewed for the issue:

  

  JDK-8240666:  Websocket client’s OpeningHandshake discards the HTTP response 
body.

   

  The fix updates jdk.internal.net.http.websocket.OpeningHandshake.send()

  to process the response body received from server instead of discarding it,

  when the handshake is rejected by the websocket server.

  

   

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

 Webrev:  
http://cr.openjdk.java.net/~ryadav/webrev_8240666/webrev.00/index.html 

 

   --rahul

Re: RFR 8240666: Websocket client’s OpeningHandshake discards the HTTP response body

[Pavel Rappo]

An assertion of the form

assertEquals(true, ((String)wse.getResponse().body()).contains("404"));

looks odd. I'd suggest using any of

assertTrue(boolean condition)
assertTrue(boolean condition, String message)

-Pavel

> On 6 May 2020, at 14:12, Rahul  wrote:
> 
> http://cr.openjdk.java.net/~ryadav/webrev_8240666/webrev.00/index.html

Re: RFR 8240666: Websocket client’s OpeningHandshake discards the HTTP response body

[Rahul]

Hi Pavel,

Thank you for the comment, the webrev has been updated.

webrev : http://cr.openjdk.java.net/~ryadav/webrev_8240666/webrev.00/index.html

- rahul

On 06/05/2020, 14:26, "Pavel Rappo"  wrote:

An assertion of the form

assertEquals(true, ((String)wse.getResponse().body()).contains("404"));

looks odd. I'd suggest using any of

assertTrue(boolean condition)
assertTrue(boolean condition, String message)

-Pavel

> On 6 May 2020, at 14:12, Rahul  wrote:
> 
> http://cr.openjdk.java.net/~ryadav/webrev_8240666/webrev.00/index.html

Re: RFR 8240666: Websocket client’s OpeningHandshake discards the HTTP response body

[Daniel Fuchs]

Hi Rahul,

LGTM.

 111 WebSocketHandshakeException wse = 
(WebSocketHandshakeException) t;
 112 out.println("Status code is " + 
wse.getResponse().statusCode());
 113 out.println("Response is " + 
wse.getResponse().body());

 114 assertNotNull(wse.getResponse());


I'd suggest moving line 114 two lines up (just after line 111)
to avoid triggering a NPE if wse.getResponse() returns null.
No need for a new webrev if that's the only change.

best regards,

-- daniel

On 06/05/2020 16:04, Rahul wrote:

Hi Pavel,

Thank you for the comment, the webrev has been updated.

webrev : http://cr.openjdk.java.net/~ryadav/webrev_8240666/webrev.00/index.html

- rahul

On 06/05/2020, 14:26, "Pavel Rappo"  wrote:

 An assertion of the form
 
 assertEquals(true, ((String)wse.getResponse().body()).contains("404"));
 
 looks odd. I'd suggest using any of
 
 assertTrue(boolean condition)

 assertTrue(boolean condition, String message)
 
 -Pavel
 
 > On 6 May 2020, at 14:12, Rahul  wrote:

 >
 > http://cr.openjdk.java.net/~ryadav/webrev_8240666/webrev.00/index.html
 
 

Re: RFR 8240666: Websocket client’s OpeningHandshake discards the HTTP response body

[Chris Hegarty]

> On 6 May 2020, at 16:16, Daniel Fuchs  wrote:
> 
> Hi Rahul,
> 
> LGTM.

+1

-Chris.

Re: RFR 8240666: Websocket client’s OpeningHandshake discards the HTTP response body

[rahul . r . yadav]

Thanks Daniel , webrev updated.

- rahul

On 06/05/2020 16:16, Daniel Fuchs wrote:

Hi Rahul,

LGTM.

 111 WebSocketHandshakeException wse = 
(WebSocketHandshakeException) t;
 112 out.println("Status code is " + 
wse.getResponse().statusCode());
 113 out.println("Response is " + 
wse.getResponse().body());

 114 assertNotNull(wse.getResponse());


I'd suggest moving line 114 two lines up (just after line 111)
to avoid triggering a NPE if wse.getResponse() returns null.
No need for a new webrev if that's the only change.

best regards,

-- daniel

On 06/05/2020 16:04, Rahul wrote:

Hi Pavel,

Thank you for the comment, the webrev has been updated.

webrev : 
http://cr.openjdk.java.net/~ryadav/webrev_8240666/webrev.00/index.html


- rahul

On 06/05/2020, 14:26, "Pavel Rappo"  wrote:

 An assertion of the form
  assertEquals(true, 
((String)wse.getResponse().body()).contains("404"));

  looks odd. I'd suggest using any of
  assertTrue(boolean condition)
 assertTrue(boolean condition, String message)
  -Pavel
  > On 6 May 2020, at 14:12, Rahul  
wrote:

 >
 > 
http://cr.openjdk.java.net/~ryadav/webrev_8240666/webrev.00/index.html

Re: RFR 8240666: Websocket client’s OpeningHandshake discards the HTTP response body

[Daniel Fuchs]

On 06/05/2020 16:48, rahul.r.ya...@oracle.com wrote:

Thanks Daniel , webrev updated.


Looks good!

-- daniel

Re: RFR[8241072]: 'Reimplement Legacy DatagramSocket API'

[Patrick Concannon]

Hi Alan,

With regards to the SetGetSendBufferSize.java test: yes, it was created 
to increase the test coverage for the DatagramSocket class. However, it 
was decided that it should be handled separately and was pushed to the 
mainline in advance of this RFR - (JDK-8243488 
).



Please find the updated webrev below:

http://cr.openjdk.java.net/~pconcannon/8241072/webrevs/webrev.05/

Kind regards,

Patrick



On 17/04/2020 10:35, Alan Bateman wrote:

On 15/04/2020 20:22, Patrick Concannon wrote:

:

WRT the PDSI issue, I've created a bug to track this and have 
assigned it to myself. You can view the issue here: 
https://bugs.openjdk.java.net/browse/JDK-8242885



Thanks. That should be trivial to fix and should allow SendBufCheck to 
be updated to avoid testing that the older implementation has the 
wrong limit. The JEP hasn't been proposed to target yet so I think 
there is time to get this sorted out in advance.


Is SetGetSendBufferSize.java about extending test coverage? If so then 
I think the range of input needs to be expanded a bit, e.g. attempting 
to set to 0 should throw IAE, it should at least be possible to set it 
to 64. I'm curious why the test checks getSoTimeout after the socket 
has been closed. Do you know if the test coverage for the receive 
buffer size is sufficient?


-Alan

RE: RFR 15 8243099: SO_INCOMING_NAPI_ID support

[Ivanov, Vladimir A]

The socket option SO_INCOMING_NAPI_ID returns the integer value that is 
associated with the hardware queue of the device the last packet is received 
on. This is true for UDP Datagram sockets as well. In case the socket is 
receiving the packets from multiple NIC Devices or the incoming packets of the 
socket are getting directed to different Hardware queues on same device, this 
return value will change as the queue for last packet will change.

Please note that the proposed change provides a method for application to query 
the queue id on which the last packet is received on. Not all applications need 
to use this option or need to change their existing behavior. It is expected 
that the applications that take advantage of this new option are getting the 
connections aligned to the particular HW queues. For example, for UDP 
application, a socket can be bound to an IP/Port and NIC can be configured to 
direct all packets for that IP/Port to a queue. In this case the value will 
remain constant and application can utilize this option. If UDP application is 
listening on all devices in the system, then it does not have to use this 
option and continue to function as it does today.

 Thanks, Vladimir

-Original Message-
From: Daniel Fuchs  
Sent: Wednesday, May 6, 2020 2:37 AM
To: Ivanov, Vladimir A ; Alan Bateman 
; OpenJDK Network Dev list 
Subject: Re: RFR 15 8243099: SO_INCOMING_NAPI_ID support

Hi Vladimir,

On 06/05/2020 00:36, Ivanov, Vladimir A wrote:
> I was failed to get non-zero value for the ServerSocket and according to 
> clarification from the network experts:
> " NAPI IDs is not marked on listening sockets. A listening socket can accept 
> multiple connections coming on different queues. So even if we do it, it can 
> keep changing and we don't see a use case for it. Once a connection is 
> accepted, accepted or the child socket will return a valid NAPI id."
> 
> So seems the listener sockets will have this as zero.

Thanks for this clarifications! Could you also clarify how this is supposed to 
work with UDP sockets?

I mean - does the socket needs to be connected to make the best use of this 
feature?

If not connected, will a listening UDP socket always be assigned to the same 
device queue regardless of the source of the datagrams it receives or might the 
device queue id change with each incoming packet depending on its source?

I am especially intrigued by the case where a DatagramSocket is bound to the 
wildcard address and as such potentially able to receive datagrams comming 
through any active NIC.

best regards,

-- daniel

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports - (core libraries)

[Mikael Vidstedt]

Alan, thank you for the review! New webrev coming. Meanwhile comments inline..

> On May 4, 2020, at 1:49 AM, Alan Bateman  wrote:
> 
> On 04/05/2020 06:12, Mikael Vidstedt wrote:
>> Please review this change which implements part of JEP 381:
>> 
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8244224
>> webrev: 
>> http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.00/corelibs/open/webrev/
>> JEP: https://bugs.openjdk.java.net/browse/JDK-8241787
>> 
>> 
>> Note: When reviewing this, please be aware that this exercise was 
>> *extremely* mind-numbing, so I appreciate your help reviewing all the 
>> individual changes carefully. You may want to get that coffee cup filled up 
>> (or whatever keeps you awake)!
>> 
> I took a pass over the changes. I agree its a bit tedious. I'm sure there 
> will be a few follow up issues as there are opportunities for cleanup in 
> several areas. Just a few comments/questions from a first pass.
> 
> ExtendedSocketOption.SO_FLOW_SLA is the Solaris specific socket option that 
> was terminally deprecated in 14. The patch removes the implementation but 
> leave the API (SO_FLOW_SA and jdk.net.SocketFlow). Do you want a someone to 
> take a follow-on issue to remove the API?

I would very much appreciate if somebody could take that on!

> ResolverConfigurationImpl.localDomain0 can be removed.

Fixed.

> The comment on mcast_join_leave in PlainDatagramSocketImpl.c has a residual 
> reference to Solaris.

Fixed.

> JISAutoDetect - might be simpler to just initialize EUCJPName to "EUC_JP”.

I was going back and forth on this one - in a way preserving the method was 
nicely symmetric with getSJISName(), but in the end I removed the method in 
favor of initializing the variable directly.

> Socket.setTrafficClass(int) swallows exceptions to workaround strange 
> behaviour on Solaris. Tracked as JDK-8221487 so okay to leave it to that 
> issue if you want. There is also cruft in the old plain SocketImpl that to 
> work around eagerness to report "connection reset errors - I think we should 
> just leave that because the old socket impl is not used by default and will 
> be removed at some point.

Anything I can do to keep the complexity of this patch down is appreciated. I 
have *not* changed anything, let me know if you want me to do something with 
this.

Cheers,
Mikael

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports - (core libraries)

[Mikael Vidstedt]

Thank you for reviewing, Max!

Cheers,
Mikael

> On May 4, 2020, at 7:22 AM, Weijun Wang  wrote:
> 
> There are several security-related files (name.contains("security")) and they 
> all look fine.
> 
> --Max
> 
> 
>> On May 4, 2020, at 1:12 PM, Mikael Vidstedt  
>> wrote:
>> 
>> 
>> Please review this change which implements part of JEP 381:
>> 
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8244224
>> webrev: 
>> http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.00/corelibs/open/webrev/
>> JEP: https://bugs.openjdk.java.net/browse/JDK-8241787
>> 
>> 
>> Note: When reviewing this, please be aware that this exercise was 
>> *extremely* mind-numbing, so I appreciate your help reviewing all the 
>> individual changes carefully. You may want to get that coffee cup filled up 
>> (or whatever keeps you awake)!
>> 
>> 
>> Background:
>> 
>> Because of the size of the total patch and wide range of areas touched, this 
>> patch is one out of in total six partial patches which together make up the 
>> necessary changes to remove the Solaris and SPARC ports. The other patches 
>> are being sent out for review to mailing lists appropriate for the 
>> respective areas the touch. An email will be sent to jdk-dev summarizing all 
>> the patches/reviews. To be clear: this patch is *not* in itself complete and 
>> stand-alone - all of the (six) patches are needed to form a complete patch. 
>> Some changes in this patch may look wrong or incomplete unless also looking 
>> at the corresponding changes in other areas.
>> 
>> For convenience, I’m including a link below[1] to the full webrev, but in 
>> case you have comments on changes in other areas, outside of the files 
>> included in this thread, please provide those comments directly in the 
>> thread on the appropriate mailing list for that area if possible.
>> 
>> In case it helps, the changes were effectively produced by searching for and 
>> updating any code mentioning “solaris", “sparc”, “solstudio”, “sunos”, etc. 
>> More information about the areas impacted can be found in the JEP itself.
>> 
>> 
>> Testing:
>> 
>> A slightly earlier version of this change successfully passed tier1-8, as 
>> well as client tier1-2. Additional testing will be done after the first 
>> round of reviews has been completed.
>> 
>> Cheers,
>> Mikael
>> 
>> [1] 
>> http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.00/all/open/webrev/
>> 
> 

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports - (core libraries)

[Mikael Vidstedt]

> On May 4, 2020, at 2:33 PM, Brent Christian  
> wrote:
> 
> Hi,
> 
> Looks fine to me.  I have just one minor observation:
> 
> src/java.base/share/native/libjli/emessages.h
> 
> *** 92,102 
>  #define JRE_ERROR5  "Error: Failed to start a %d-bit JVM process from a 
> %d-bit JVM."
> ! #define JRE_ERROR6  "Error: Verify all necessary Java SE components 
> have been installed.\n(Solaris SPARC 64-bit components must be installed 
> after 32-bit components.)"
>  #define JRE_ERROR7  "Error: Either 64-bit processes are not supported by 
> this platform\nor the 64-bit components have not been installed."
> --- 91,101 
>  #define JRE_ERROR5  "Error: Failed to start a %d-bit JVM process from a 
> %d-bit JVM."
> ! #define JRE_ERROR6  "Error: Verify all necessary Java SE components 
> have been installed.\n"
>  #define JRE_ERROR7  "Error: Either 64-bit processes are not supported by 
> this platform\nor the 64-bit components have not been installed."
> 
> 
> The other error messages do not include a trailing newline.

Nice catch! Fixed in the upcoming webrev.

Thank you for this review - and the pre-review before it! :)

Cheers,
Mikael

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports - (core libraries)

[Mikael Vidstedt]

> On May 4, 2020, at 9:27 AM, naoto.s...@oracle.com wrote:
> 
> Hi Mikael,
> 
> I took a look at i18n related files. It looks good overall.
> 
> One nit in java/nio/charset/Charset/DefaultCharsetTest.java: If the test is 
> only applicable to linux (@requires os.family == "linux" in jtreg tag after 
> the change), the condition isLinux() is no longer needed at line 73.

Good catch! Fixed in the upcoming webrev.

Thank you for the review!

Cheers,
Mikael

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports - (core libraries)

[Mikael Vidstedt]

I have always wondered what “solinux” is supposed to mean - though not enough 
to actually ask anybody :)

I’ll file a follow-up enhancement to cover renaming the files.

Thank you for the review!

Cheers,
Mikael

> On May 4, 2020, at 7:59 AM, Roger Riggs  wrote:
> 
> Hi Michael,
> 
> Looks good.
> 
> Maybe just a future cleanup to rename files, since the "...so..." is refering 
> to solaris.
> 
> src/java.base/unix/native/libjli/java_md_solinux.h
> src/java.base/unix/native/libjli/java_md_solinux.h
> 
> Regards, Roger
> 
> 
> On 5/4/20 4:49 AM, Alan Bateman wrote:
>> On 04/05/2020 06:12, Mikael Vidstedt wrote:
>>> Please review this change which implements part of JEP 381:
>>> 
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8244224
>>> webrev: 
>>> http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.00/corelibs/open/webrev/
>>> JEP: https://bugs.openjdk.java.net/browse/JDK-8241787
>>> 
>>> 
>>> Note: When reviewing this, please be aware that this exercise was 
>>> *extremely* mind-numbing, so I appreciate your help reviewing all the 
>>> individual changes carefully. You may want to get that coffee cup filled up 
>>> (or whatever keeps you awake)!
>>> 
>> I took a pass over the changes. I agree its a bit tedious. I'm sure there 
>> will be a few follow up issues as there are opportunities for cleanup in 
>> several areas. Just a few comments/questions from a first pass.
>> 
>> ExtendedSocketOption.SO_FLOW_SLA is the Solaris specific socket option that 
>> was terminally deprecated in 14. The patch removes the implementation but 
>> leave the API (SO_FLOW_SA and jdk.net.SocketFlow). Do you want a someone to 
>> take a follow-on issue to remove the API?
>> 
>> ResolverConfigurationImpl.localDomain0 can be removed.
>> 
>> The comment on mcast_join_leave in PlainDatagramSocketImpl.c has a residual 
>> reference to Solaris.
>> 
>> JISAutoDetect - might be simpler to just initialize EUCJPName to "EUC_JP".
>> 
>> Socket.setTrafficClass(int) swallows exceptions to workaround strange 
>> behaviour on Solaris. Tracked as JDK-8221487 so okay to leave it to that 
>> issue if you want. There is also cruft in the old plain SocketImpl that to 
>> work around eagerness to report "connection reset errors - I think we should 
>> just leave that because the old socket impl is not used by default and will 
>> be removed at some point.
>> 
>> -Alan.
> 

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports - (core libraries)

[Mikael Vidstedt]

> On May 5, 2020, at 5:42 AM, Daniel Fuchs  wrote:
> 
> Hi Mikael,
> 
> I spotted another place where a residual reference to Solaris
> remains in a comment:
> src/java.base/unix/native/libnet/PlainSocketImpl.c
> 
> 857 #if defined(_AIX)
> 858 if (errno == EINVAL) {
> 859 // On Solaris setsockopt will set errno to EINVAL if the 
> socket
> 860 // is closed. The default error message is then confusing

Fixed in the upcoming webrev!

> Otherwise I had a look at the networking related files and
> the changes seemed reasonable to me.

Thanks a lot for the review!

Cheers,
Mikael

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports - (core libraries)

[Mikael Vidstedt]

New webrev addressing the feedback/comments I have received:

webrev: 
http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.01/corelibs/open/webrev/
incremental: 
http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.01/corelibs.incr/open/webrev/

I hope I caught everything. Some outstanding items:

* File follow-up enhancement for the removal of SO_FLOW_SA and 
jdk.net.SocketFlow

Would appreciate if somebody could help file this for me.

* File follow-up enhancement for renaming the “solinux” files

* Get confirmation from Alan that the Socket.setTrafficClass(int) and 
SocketImpl stuff is good as-is

Cheers,
Mikael

> On May 3, 2020, at 10:12 PM, Mikael Vidstedt  
> wrote:
> 
> 
> Please review this change which implements part of JEP 381:
> 
> JBS: https://bugs.openjdk.java.net/browse/JDK-8244224
> webrev: 
> http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.00/corelibs/open/webrev/
> JEP: https://bugs.openjdk.java.net/browse/JDK-8241787
> 
> 
> Note: When reviewing this, please be aware that this exercise was *extremely* 
> mind-numbing, so I appreciate your help reviewing all the individual changes 
> carefully. You may want to get that coffee cup filled up (or whatever keeps 
> you awake)!
> 
> 
> Background:
> 
> Because of the size of the total patch and wide range of areas touched, this 
> patch is one out of in total six partial patches which together make up the 
> necessary changes to remove the Solaris and SPARC ports. The other patches 
> are being sent out for review to mailing lists appropriate for the respective 
> areas the touch. An email will be sent to jdk-dev summarizing all the 
> patches/reviews. To be clear: this patch is *not* in itself complete and 
> stand-alone - all of the (six) patches are needed to form a complete patch. 
> Some changes in this patch may look wrong or incomplete unless also looking 
> at the corresponding changes in other areas.
> 
> For convenience, I’m including a link below[1] to the full webrev, but in 
> case you have comments on changes in other areas, outside of the files 
> included in this thread, please provide those comments directly in the thread 
> on the appropriate mailing list for that area if possible.
> 
> In case it helps, the changes were effectively produced by searching for and 
> updating any code mentioning “solaris", “sparc”, “solstudio”, “sunos”, etc. 
> More information about the areas impacted can be found in the JEP itself.
> 
> 
> Testing:
> 
> A slightly earlier version of this change successfully passed tier1-8, as 
> well as client tier1-2. Additional testing will be done after the first round 
> of reviews has been completed.
> 
> Cheers,
> Mikael
> 
> [1] 
> http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.00/all/open/webrev/
> 

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports - (core libraries)

[Alan Bateman]

On 07/05/2020 05:56, Mikael Vidstedt wrote:

:

* File follow-up enhancement for the removal of SO_FLOW_SA and 
jdk.net.SocketFlow
I've created JDK-8244582 to track this, we should try to this in the 
same release as JEP 381.



:


* Get confirmation from Alan that the Socket.setTrafficClass(int) and 
SocketImpl stuff is good as-is

JDK-8221487 tracks the Socket.setTrafficClass issue. I didn't create an 
issue to mop up issues in the old SocketImpl as it is not used (at least 
not by default) since JDK 13. The intention is to completely remove the 
old SocketImpl implementation, don't know which release yet but it's 
hardly seems worth spending time doing cleanup on this code. I've no 
doubt there will be cleanup in many areas of the JDK code after JEP 381 
is pushed.


-Alan.