Re: Thread safety problem in java.net.ProxySelector

[David Lloyd]

The bug ID is https://bugs.openjdk.java.net/browse/JDK-8252996 .

On Wed, Sep 9, 2020 at 3:14 PM Chris Hegarty  wrote:
>
> Seems like a bug. Can you please file an issue for it.
>
> Thanks,
> -Chris.
>
> > On 2 Sep 2020, at 14:16, David Lloyd  wrote:
> >
> > The default proxy selector field in java.net.ProxySelector is
> > essentially a global variable with no thread safety measures taken to
> > ensure that accesses are valid.  Anecdotally, a symptom of this bug
> > *may* be that the field appears `null` after assignment (based on an
> > IRC discussion with a confused user).
> >
> > Here's the trivial fix:
> >
> > diff --git a/src/java.base/share/classes/java/net/ProxySelector.java
> > b/src/java.base/share/classes/java/net/ProxySelector.java
> > index c1e97ecc981..e52c888f755 100644
> > --- a/src/java.base/share/classes/java/net/ProxySelector.java
> > +++ b/src/java.base/share/classes/java/net/ProxySelector.java
> > @@ -65,7 +65,7 @@ public abstract class ProxySelector {
> >  *
> >  * @see #setDefault(ProxySelector)
> >  */
> > -private static ProxySelector theProxySelector;
> > +private static volatile ProxySelector theProxySelector;
> >
> > static {
> > try {
> >
> >
> > --
> > - DML • he/him
> >
>


-- 
- DML • he/him

Thread safety problem in java.net.ProxySelector

[David Lloyd]

The default proxy selector field in java.net.ProxySelector is
essentially a global variable with no thread safety measures taken to
ensure that accesses are valid.  Anecdotally, a symptom of this bug
*may* be that the field appears `null` after assignment (based on an
IRC discussion with a confused user).

Here's the trivial fix:

diff --git a/src/java.base/share/classes/java/net/ProxySelector.java
b/src/java.base/share/classes/java/net/ProxySelector.java
index c1e97ecc981..e52c888f755 100644
--- a/src/java.base/share/classes/java/net/ProxySelector.java
+++ b/src/java.base/share/classes/java/net/ProxySelector.java
@@ -65,7 +65,7 @@ public abstract class ProxySelector {
  *
  * @see #setDefault(ProxySelector)
  */
-private static ProxySelector theProxySelector;
+private static volatile ProxySelector theProxySelector;

 static {
 try {


-- 
- DML • he/him

Re: Reading from a closed socket: different behavior between Linux and other operating systems

[David Lloyd]

This is, AFAICT, expected based on the differences between the socket
layers of the various operating systems involved and their handling of
closed sockets.  If you write a similar test program in C using OS specific
APIs, I believe you will see similar results.  I don't think this is a
problem with the JDK, nor is it likely to be something that can be fixed in
the JDK (since the error reported by the OS is, as far as I know, unlikely
to be universally sufficient to extrapolate the exact cause of failure).

On Thu, Jan 2, 2020 at 9:14 AM Sean Mullan  wrote:

> Cross-posting to security-dev as SSL is involved.
>
> --Sean
>
> On 12/29/19 4:01 PM, Dawid Weiss wrote:
> > Hello,
> >
> > I am a committer to the Apache Lucene project. We have been looking
> > into a problem in which SSL connections were handled differently in
> > tests on different operating systems and narrowed it down to
> > essentially the following scenario (full repro code at [1]):
> >
> > Server side:
> >
> >try (ServerSocketChannel serverChannel = ServerSocketChannel.open()) {
> >  SocketChannel clientChannel = serverChannel.accept();
> >  clientChannel.close();
> >}
> >
> > Client side:
> >
> >Socket socket = new Socket();
> >socket.connect(target);
> >// ... server closes the socket here.
> >// Queue some data for writing to the closed socket. This succeeds.
> >socket.getOutputStream().write("will succeed?!".getBytes("UTF-8"));
> >// Try to read something from the closed socket.
> >socket.getInputStream().read(new byte[100]);
> >
> > The last line of the client results in different behavior between
> > operating systems.
> >
> > 1) Linux, JDK 11, 13, 14: succeeds with -1 (EOF).
> > 2) Windows, JDK 11: SocketException ("recv failed") is thrown
> > 3) Windows, JDK 13, 14: SocketException (localized message) is thrown
> > 4) FreeBSD: SocketException (connection reset) is thrown
> > 5) Mac OS X: SocketException (connection reset) is thrown
> >
> > I admit my original thinking on the Lucene issue (see full  discussion
> > at [2]) was that it was Windows that was off here (due to
> > WSAECONNRESET not being handled at all in SocketInputStream.c [3].
> > Since then (JDK11) the underlying socket implementation has changed
> > due to JEP 353 [4] (which Alan Bateman kindly pointed out to me).
> >
> > But the difference in runtime behavior between Linux and other
> > operating systems still exists on both the old and the new
> > implementation. I don't know whether it's something that should be
> > qualified as platform-specific but it causes additional problems when
> > it triggers somewhere deep inside the SSL handling layer -- then the
> > application-level code receives a different exception depending on
> > where it's run (an SSLException with a suppressed SocketException or a
> > SocketException directly).
> >
> > I don't have any ideas about what a "good" fix for this is but I'm
> > curious what others think.
> >
> > Dawid
> >
> > [1]
> https://issues.apache.org/jira/secure/attachment/12989538/RecvRepro.java
> > [2] https://issues.apache.org/jira/browse/SOLR-13778
> > [3]
> https://github.com/openjdk/jdk14/blob/f58a8cbed2ba984ceeb9a1ea59f917e3f9530f1e/src/java.base/windows/native/libnet/SocketInputStream.c#L120-L154
> > [4] https://openjdk.java.net/jeps/353
> >
>
>

-- 
- DML

Unused declaration in PlainDatagramSocketImpl.c

[David Lloyd]

While doing some GraalVM work, it was discovered that there's an
apparently unused declaration in PlainDatagramSocketImpl.c.  I've
attached a trivial patch to remove it; do with it what you will.

-- 
- DML


pdsi.patch
Description: Binary data

Re: RFR 8216978: Drop support for pre JDK 1.4 SocketImpl implementations

[David Lloyd]

On Mon, Apr 29, 2019 at 10:54 AM Michael McMahon
 wrote:
>
>
>
> On 29/04/2019, 13:08, Chris Hegarty wrote:
> >
> > On 29/04/2019 12:17, Alan Bateman wrote:
> >> ...
> >> Changing SIS.close and SOS.close to caller super.close raises a
> >> number of questions. These close should never be called
> >> Socket.getInputStream and getOutputStream don't leak these streams to
> >> user code (they used to but now in JDK 13). My concern is that if
> >> they were ever to be called then it will be calling the FIS/FOS close
> >> methods which brings along a several questions on it interacts with
> >> the cleaner mechanism used by those classes.
> >
> > Since 8220493, these socket in/out stream close methods are effectively
> > no longer in charge of closing the socket ( since that will happen from
> > the outer stream wrapper in Socket ). I wonder if they should simply
> > call impl.close()?
> >
>
> On that point. Since it might not be obvious that SIS.close and
> SOS.close are no longer used,
> it might be more useful to just replace the close implementations with a
> comment and an assertion
> which underlines that fact.

As someone who does a lot of socket programming (albeit mostly
non-blocking), I would be *shocked* if closing a socket input or
output stream didn't translate into shutting down input or output
(respectively).  Not close per se but socket shutdown.
-- 
- DML

Re: RFR: 8189366: SocketInputStream.available() should check for eof

[David Lloyd]

On Fri, Oct 12, 2018 at 6:01 AM Chris Hegarty  wrote:
> That is correct. While not intuitive, I don't propose
> that we change this. ( if this were a new implementation
> then I think it should throw IOE for this scenario, but
> we are where we are ).

I am glad then that it is not a new implementation.  Returning an EOF
signal is a very reasonable behavior for a closed input stream or
input channel.  In most cases, it is more difficult for I/O handling
code to distinguish the two states (open+eof versus closed), and in
every case I've ever known, they are handled in the same way, or
intended to be.  There is no useful way I know of to exploit a side
effect of exception throwing in the read-on-closed case.

Also, I think it's a lot more reasonable for programs to expect that
available() will return 0 when the stream is closed than for
available() to be, in effect, an openness check.  Granted that
available() is not really useful anyway, but were one to use it,
having an extra failure state to deal with probably isn't helpful.

-- 
- DML

Re: [openjdk-x] Bind to a multicast address fails

[David Lloyd]

Fantastic!  I'm glad to hear it.
On Fri, Sep 14, 2018 at 1:39 AM Andre Naujoks  wrote:
>
> On 9/10/18 10:36 AM, Andre Naujoks wrote:
> > On 9/8/18 11:49 AM, Andre Naujoks wrote:
> >> On 9/7/18 6:08 PM, David Lloyd wrote:
> >>> On Fri, Sep 7, 2018 at 6:56 AM Andre Naujoks  wrote:
> >>>> On 9/7/18 1:15 PM, Alan Bateman wrote:
> >>>>> On 07/09/2018 10:49, Decke, Hendrik (K-GERFA/A) wrote:
> >>>>>>
> >>>>>> Hello,
> >>>>>>
> >>>>>> it seems one of our external developers (Andre Naujoks, CC) found a
> >>>>>> bug while binding a IPv6 multicast UDP-socket for one of our projects.
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> Since this seems to be a fundamental bug (from our perspective), we
> >>>>>> address this directly to this mailing list.
> >>>>>>
> >>>>>> (reproducible in OpenJDK 8-11, Windows and Linux)
> >>>>>>
> >>>>> This bug was submitted this week on this issue:
> >>>>>   https://bugs.openjdk.java.net/browse/JDK-8210493
> >>>>>
> >>>>> Have you tried joining the mullticast group, specifying the network
> >>>>> interface, rather than binding to the multicast address?
> >>>>
> >>>> Hi Alan.
> >>>>
> >>>> First of all, thank you for the quick reply. I was not aware, that there
> >>>> was actually a bug opened for that issue.
> >>>>
> >>>> The join is not the problem at this point. We need to bind the socket to
> >>>> the address to avoid receiving traffic from all multicast groups, that
> >>>> are joined on the system. Since a join joins the system (not the socket)
> >>>> to the group, all sockets bound to a port, which receive multicast
> >>>> traffic will receive all of that traffic, no matter the destination
> >>>> address. The bind prevents that. IP_MULTICAST_ALL sadly only works for
> >>>> IPv4 and the patch I tried to get IPV6_MULTICAST_ALL upstream into the
> >>>> kernel was even more sadly (almost) ignored. see
> >>>> https://marc.info/?l=linux-netdev=152344460530252=2
> >>>
> >>> Hi Andre,
> >>>
> >>> I spoke with a colleague about this kernel patch.  They said first of
> >>> all that multicast filtering is pretty complex in the kernel with a
> >>> lot of subtle behaviors.  But, they also said that it may have been
> >>> ignored because of the format of the patch, perhaps even
> >>> accidentally/automatically.  The proposed patch has an "RFC" tag, and
> >>> such patches apparently need to be in "git-format-patch" mode.
> >>> Lastly, they said that since the time that the post was made,
> >>> IP_MULTICAST_ALL (for IPv4 only of course) has changed a little bit in
> >>> that "it only allows receipt of all multicast groups if a specific
> >>> list of multicast groups has not already been set", so it may need to
> >>> be updated accordingly.  FWIW they didn't mention seeing any actual
> >>> problems with the content of the patch, though I'm not sure how
> >>> thoroughly they reviewed it.
> >>>
> >>> So, while I myself am not a Linux kernel contributor, I do suspect
> >>> that if you reposted an updated version of the patch, in the correct
> >>> format, it will enter the patch queue and may be more actively
> >>> discussed.  For more information, see [1].
> >>>
> >>> [1] 
> >>> https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#the-canonical-patch-format
> >>>
> >>
> >> Hi David
> >>
> >> Thanks for the advice. The patch was actually generated with 'git
> >> format-patch' I just added some explanatory text, which I did not see as
> >> part of the commit message. The RFC tag was intentional because I am not
> >> that deeply familiar with the networking code in the kernel to "just
> >> send a patch", if you know what I mean. As far as I know, there is no
> >> automatic handling of those e-mails except for the patchwork stuff.
> >>
> >> I need to take a look at the IP_MULTICAST_ALL changes and see, if there
> >> is anything that needs to be changed in the IPV6_MULTICAST_ALL addition
> >> to accommodate for that. But, as you said, the multicast filtering is
> >> pretty complex.
> >>
> >> I'll try and re-post the patch on Monday, leave out all the extra stuff
> >> and see if it changes anything or at least if it gets any more replies.
> >
> > I just reposted the patch. Fully automated this time. I couldn't find
> > any changes in the IPv4 handling of the socket option, that was not
> > there, when I first posted it.
> >
> > Lets see, if this has more success.
> >
> > https://marc.info/?l=linux-netdev=153656806202701=2
>
> Hello all.
>
> I just wanted to thank you again for the advice.
>
> The patch just got accepted and is now in the net-next kernel git.
>
> Regards
>Andre
>
> >
> >
> > Regards
> >   Andre
> >
> >>
> >> Regards
> >>   Andre
> >>
> >
>


-- 
- DML

Re: [openjdk-x] Bind to a multicast address fails

[David Lloyd]

On Fri, Sep 7, 2018 at 11:08 AM David Lloyd  wrote:
> On Fri, Sep 7, 2018 at 6:56 AM Andre Naujoks  wrote:
> > On 9/7/18 1:15 PM, Alan Bateman wrote:
> > > On 07/09/2018 10:49, Decke, Hendrik (K-GERFA/A) wrote:
> > >>
> > >> Hello,
> > >>
> > >> it seems one of our external developers (Andre Naujoks, CC) found a
> > >> bug while binding a IPv6 multicast UDP-socket for one of our projects.
> > >>
> > >>
> > >>
> > >> Since this seems to be a fundamental bug (from our perspective), we
> > >> address this directly to this mailing list.
> > >>
> > >> (reproducible in OpenJDK 8-11, Windows and Linux)
> > >>
> > > This bug was submitted this week on this issue:
> > >   https://bugs.openjdk.java.net/browse/JDK-8210493
> > >
> > > Have you tried joining the mullticast group, specifying the network
> > > interface, rather than binding to the multicast address?
> >
> > Hi Alan.
> >
> > First of all, thank you for the quick reply. I was not aware, that there
> > was actually a bug opened for that issue.
> >
> > The join is not the problem at this point. We need to bind the socket to
> > the address to avoid receiving traffic from all multicast groups, that
> > are joined on the system. Since a join joins the system (not the socket)
> > to the group, all sockets bound to a port, which receive multicast
> > traffic will receive all of that traffic, no matter the destination
> > address. The bind prevents that. IP_MULTICAST_ALL sadly only works for
> > IPv4 and the patch I tried to get IPV6_MULTICAST_ALL upstream into the
> > kernel was even more sadly (almost) ignored. see
> > https://marc.info/?l=linux-netdev=152344460530252=2
>
> Hi Andre,
>
> I spoke with a colleague about this kernel patch.  They said first of
> all that multicast filtering is pretty complex in the kernel with a
> lot of subtle behaviors.  But, they also said that it may have been
> ignored because of the format of the patch, perhaps even
> accidentally/automatically.  The proposed patch has an "RFC" tag, and
> such patches apparently need to be in "git-format-patch" mode.
> Lastly, they said that since the time that the post was made,
> IP_MULTICAST_ALL (for IPv4 only of course) has changed a little bit in
> that "it only allows receipt of all multicast groups if a specific
> list of multicast groups has not already been set", so it may need to
> be updated accordingly.  FWIW they didn't mention seeing any actual
> problems with the content of the patch, though I'm not sure how
> thoroughly they reviewed it.
>
> So, while I myself am not a Linux kernel contributor, I do suspect
> that if you reposted an updated version of the patch, in the correct
> format, it will enter the patch queue and may be more actively
> discussed.  For more information, see [1].
>
> [1] 
> https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#the-canonical-patch-format

I should also mention that the recommended approach would be to send a
not-RFC version of the patch and see how that goes.
-- 
- DML

Re: [openjdk-x] Bind to a multicast address fails

[David Lloyd]

On Fri, Sep 7, 2018 at 6:56 AM Andre Naujoks  wrote:
> On 9/7/18 1:15 PM, Alan Bateman wrote:
> > On 07/09/2018 10:49, Decke, Hendrik (K-GERFA/A) wrote:
> >>
> >> Hello,
> >>
> >> it seems one of our external developers (Andre Naujoks, CC) found a
> >> bug while binding a IPv6 multicast UDP-socket for one of our projects.
> >>
> >>
> >>
> >> Since this seems to be a fundamental bug (from our perspective), we
> >> address this directly to this mailing list.
> >>
> >> (reproducible in OpenJDK 8-11, Windows and Linux)
> >>
> > This bug was submitted this week on this issue:
> >   https://bugs.openjdk.java.net/browse/JDK-8210493
> >
> > Have you tried joining the mullticast group, specifying the network
> > interface, rather than binding to the multicast address?
>
> Hi Alan.
>
> First of all, thank you for the quick reply. I was not aware, that there
> was actually a bug opened for that issue.
>
> The join is not the problem at this point. We need to bind the socket to
> the address to avoid receiving traffic from all multicast groups, that
> are joined on the system. Since a join joins the system (not the socket)
> to the group, all sockets bound to a port, which receive multicast
> traffic will receive all of that traffic, no matter the destination
> address. The bind prevents that. IP_MULTICAST_ALL sadly only works for
> IPv4 and the patch I tried to get IPV6_MULTICAST_ALL upstream into the
> kernel was even more sadly (almost) ignored. see
> https://marc.info/?l=linux-netdev=152344460530252=2

Hi Andre,

I spoke with a colleague about this kernel patch.  They said first of
all that multicast filtering is pretty complex in the kernel with a
lot of subtle behaviors.  But, they also said that it may have been
ignored because of the format of the patch, perhaps even
accidentally/automatically.  The proposed patch has an "RFC" tag, and
such patches apparently need to be in "git-format-patch" mode.
Lastly, they said that since the time that the post was made,
IP_MULTICAST_ALL (for IPv4 only of course) has changed a little bit in
that "it only allows receipt of all multicast groups if a specific
list of multicast groups has not already been set", so it may need to
be updated accordingly.  FWIW they didn't mention seeing any actual
problems with the content of the patch, though I'm not sure how
thoroughly they reviewed it.

So, while I myself am not a Linux kernel contributor, I do suspect
that if you reposted an updated version of the patch, in the correct
format, it will enter the patch queue and may be more actively
discussed.  For more information, see [1].

[1] 
https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#the-canonical-patch-format
-- 
- DML

Re: Connection timeouts in JEP 321 (HTTP Client)

[David Lloyd]

On Wed, Jul 25, 2018 at 9:43 AM Chris Hegarty  wrote:
>
> Clearly the request builder `timeout` method can be used to avoid
> extremely long connection timeouts ( as demonstrated below ), but I see
> Bernd's call for more fine grained control over various aspects of the
> request.
>
> I'm not opposed to an "HttpRequest.Builder.connectTimeout` method, but
> this is coming very late in the JDK 11 development cycle. How important
> is this for 11, given that the naked `timeout` can be used, somewhat, to
> mitigate against long connection timeouts?

FWIW I think this is a design error (one that I have made in multiple
areas in the past) and should be rectified ASAP.  It is becoming
increasingly evident (to me at least) that it is important to
distinguish between connection and request timeouts, especially for
protocols like HTTP where connections may be reused.  Connection
mutliplexing and reuse means that the first request has a different
overall disposition than subsequent requests.  The semantics of a
connection failure may be substantially different from a request
failure, and as was previously pointed out, different architectures
have different tolerances for the two that may not align easily.

-- 
- DML

Re: RFR : 8205959 : Do not restart close if errno is EINTR

[David Lloyd]

According to http://man7.org/linux/man-pages/man2/close.2.html it is currently 
platform-dependent whether close() must or must not (seems to be no middle 
ground) be retried.  Might have to do some #ifdef guarding?

--
- DML


> On Jun 27, 2018, at 6:15 PM, Ivan Gerasimov  wrote:
> 
> Hello!
> 
> When closing a socket via NET_SocketClose(int fd), a close(fd) is called.
> The later is wrapped in a retry-loop, which is wrong because close() is not 
> restartable.
> 
> The `man 2 close` states:
> """
> ... close() should not be retried after an EINTR since this may cause a 
> reused descriptor from another thread to be closed.
> """
> 
> Would you please help review a trivial fix?
> 
> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8205959
> WEBREV: http://cr.openjdk.java.net/~igerasim/8205959/00/webrev/
> 
> Thanks in advance!
> 
> -- 
> With kind regards,
> Ivan Gerasimov
> 

Re: RFR : 8205959 : Do not restart close if errno is EINTR

[David Lloyd]

I think because the only two possible outcomes are either that the FD
was not dup'd, in which case things carry on as before, or that it was
dup'd, in which case (at least in the JVM) re-dupping won't really do
anything harmful since the target FD already references the dead
socket FD.

The POSIX manpage doesn't seem to include any other possibilities.

On Mon, Jul 2, 2018 at 7:04 AM David Holmes  wrote:
>
> In reference to 8205959, where is it stated that dup2 is any more
> restartable than close ??
>
> AFAICS both leave things undefined/unspecified if they set EINTR.
>
> David
>
> On 2/07/2018 7:03 PM, Langer, Christoph wrote:
> > Hi Matthias,
> >
> > forwarding to serviceability-dev, because debugging is usually discussed 
> > there.
> >
> > Yes, I would think this coding should be fixed, too. Can you open a bug and 
> > prepare a change?
> >
> > Thanks
> > Christoph
> >
> >> -Original Message-
> >> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
> >> Norman Maurer
> >> Sent: Montag, 2. Juli 2018 10:23
> >> To: Baesken, Matthias 
> >> Cc: Stuefe, Thomas ; net-dev@openjdk.java.net
> >> Subject: Re: RFR : 8205959 : Do not restart close if errno is EINTR
> >>
> >> +1 retry a close on EINTR has most likely not the outcome you expect and
> >> may even close a wrong FD if the same FD is reused already (as even if 
> >> EINTR
> >> is returned it may have closed the FD)
> >>
> >>> Am 02.07.2018 um 10:17 schrieb Baesken, Matthias
> >> :
> >>>
> >>> Hello  ,  there is a similar pattern (attempt to restart close in case of 
> >>> EINTR)
> >> in the coding as well   in  socket_md.c   :
> >>>
> >>> src/jdk.jdwp.agent/unix/native/libdt_socket/socket_md.c-147-int rv;
> >>> src/jdk.jdwp.agent/unix/native/libdt_socket/socket_md.c-148-do {
> >>> src/jdk.jdwp.agent/unix/native/libdt_socket/socket_md.c-149-rv =
> >> close(fd);
> >>> src/jdk.jdwp.agent/unix/native/libdt_socket/socket_md.c:150:} while 
> >>> (rv
> >> == -1 && errno == EINTR);
> >>> src/jdk.jdwp.agent/unix/native/libdt_socket/socket_md.c-151-
> >>> src/jdk.jdwp.agent/unix/native/libdt_socket/socket_md.c-152-return rv;
> >>> src/jdk.jdwp.agent/unix/native/libdt_socket/socket_md.c-153-}
> >>>
> >>> Do you think this needs adjustment   (on LINUX)  as well ?
> >>>
> >>> Best regards, Matthias
> >>>
> >>>
> >>>> Message: 2
> >>>> Date: Thu, 28 Jun 2018 18:19:46 +0100
> >>>> From: Alan Bateman 
> >>>> To: David Lloyd , ivan.gerasi...@oracle.com
> >>>> Cc: OpenJDK Network Dev list 
> >>>> Subject: Re: RFR : 8205959 : Do not restart close if errno is EINTR
> >>>> Message-ID: <3fd1496f-ab83-a2d5-0699-13c8b735d...@oracle.com>
> >>>> Content-Type: text/plain; charset=utf-8; format=flowed
> >>>>
> >>>>> On 28/06/2018 17:35, David Lloyd wrote:
> >>>>> :
> >>>>> Do you (or Alan) think that this might have accounted for real-world
> >>>>> connection problems?
> >>>>>
> >>>> In the file I/O area, with NFS I think, we had an issue a long time ago
> >>>> where close was retried after EIO. That issue was fixed a long time ago
> >>>> but it's one that comes to mind in this general area.
> >>>>
> >>>> -Alan
> >>>>
> >>>



-- 
- DML

Re: RFR : 8205959 : Do not restart close if errno is EINTR

[David Lloyd]

On Wed, Jun 27, 2018 at 6:52 PM Ivan Gerasimov
 wrote:
> Here I'm patching only the Linux-specific file 
> src/java.base/linux/native/libnet/linux_close.c
> So no #ifdefs seem necessary.
>
> MacOS variant also uses the same pattern, but I'm not touching it exactly 
> because the behavior not quite clear on that platform.
>
> On Solaris we already call close(fd) with no retrying.

OK great.  I was on my phone so it was difficult to confirm that, but
I thought I'd just ping anyway to make sure.

Do you (or Alan) think that this might have accounted for real-world
connection problems?

-- 
- DML

Re: 8199329: Remove code that attempts to read bytes after connection reset reported

[David Lloyd]

+1 from me (non-reviewer).  Semantically, it is impossible to
distinguish the difference between the OS dropping bytes when it
receives an RST compared to Java doing the same thing, so there is no
observable behavior change here AFAICT.

On Wed, Mar 14, 2018 at 9:30 AM, Alan Bateman  wrote:
>
> Classic networking has some curious code to deal with connection resets. I
> needed to dig into ancient history to find the issues and original
> motivations.
>
> When a connection reset is reported (usually ECONNRESET) then further
> attempts to read from the socket will typically return -1 (EOF). Classic
> networking papers over this so that further attempts with the socket APIs to
> read bytes will continue to throw SocketException "connection reset".
> Furthermore, it allows for cases where the platform can read bytes from the
> socket buffer after ECONNRESET has been reported. None of the main stream
> platforms do this and it's hard to imagine anything depending on such
> unpredictable behavior. I'm running into this odd code as part of cleanup to
> the read/write code paths and reducing them down to a single blocking call.
>
> I would like to remove the legacy/undocumented behavior that attempts to
> read beyond the connection reset. The code to consistently throw IOException
> once ECONNRESET is returned is retained as it's possible that code relies on
> this.
>
> The proposed changes are here:
>http://cr.openjdk.java.net/~alanb/8199329/webrev/
>
> All existing tests pass with these changes.
>
> -Alan



-- 
- DML

Re: upgrade to jdk6 com.sun.httpserver

[David Lloyd]

On Tue, Feb 20, 2018 at 2:15 PM, Ashton Hogan  wrote:
> David, I understand that you don't use this feature of the JDK and that's
> absolutely fine. I'm not the type of person to impose my way of doing things
> on anyone. I hope that you aren't either. There are obviously many people in
> the community that DO love and use this httpserver for many reasons that I
> won't go into here as that is not the point of this discussion.
>
> IIRC JDK is backward compatible so highly unlikely that it's going to be
> removed.

The specification parts of the JDK are backwards compatible but the
HTTP server is not part of the Java SE specification; nevertheless,
even things that were part of the SE specification (CORBA, and the
other EE modules) have been removed for Java 11 so I wouldn't put all
my eggs in that basket.

> If you did find a way to remove it, I would also be open to having
> the source code donated to myself to refactor and rebuild under a different
> license. I'd be open to discussing this in more detail as well if need be.

I consider this unlikely as that's a question for the copyright
holders; however, you can fork this code even so, as it is available
under GPL+classpath as well as CDDL.  Talk to your lawyer about
options, and I think you'll find you have several.

> Going back to the original discussion, you mention that points 2, 3 and 4
> are subjective. As per my original request, please do put forward your
> points of view so that they can be discussed in more detail if you believe
> that they are wrong.

No thank you; I am not personally interested in this code.  I only
replied as a service to you, to help you understand the OpenJDK
community process a little bit better.

> Pragmatically speaking, the development can be done on your end to improve
> the JDK OR on my end at a cost. I'm open to either.

OK, sounds good.

> Please do try and stay on topic in future responses.

Ashton, I observe that you are not doing a great job at your first
engagement of an open source community.  I don't really have any dogs
in this race, other than to maybe guide you a little bit, but at this
point I'd suggest you "check yourself".  Coming into any community and
immediately making demands without any attempts to understand the
existing culture is not a great way to get started; you will only
alienate people (like me, now).  I do not owe you anything, nor would
I think that any other OpenJDK community member owes you anything, and
you would do well to remember that.

You will certainly not hear from me again on this thread.

-- 
- DML

Re: upgrade to jdk6 com.sun.httpserver

[David Lloyd]

Ashton, I don't think anyone disagrees with your four points at a high
level (though #4 might be a bit subjective, and #2 and #3 are
obviously design points that would theoretically be subject to
debate).

However, at the same time, you're not really going to see anyone
lining up and clamoring for a major rewrite of the JDK's HTTP server
right now, or maybe ever again.  And the reason is exactly that there
are many external options now - enough that the internal server is
almost a legacy artifact at this point (after all it was IIRC only
introduced to support the in-JDK web services classes which may soon
be dropped from the JDK altogether).  And I think that you are
definitely not going to inspire anyone to suddenly contribute effort
to overhauling it by listing off a couple of super-high-level
requirements.

That said, if you want to breathe life back into it, your best bet is
probably incremental improvement along a well-defined road map.  Based
on my perspective, I still wouldn't be super hopeful that the OpenJDK
maintainers would be really interested at this point (particularly in
major changes), but if you really like the code base for some reason,
you also have the option of forking it.  Particularly speaking, I
don't think you'll find many people who are interested in engaging in
any sort of design debate about it at this stage of its life.

Hope this helps.


On Tue, Feb 20, 2018 at 11:24 AM, Ashton Hogan  wrote:
> Hi Rob
>
> Can you please read what I initially asked again, I'm not asking about new
> frameworks or web servers. I'm stating 4 points that need to be addressed in
> the EXISTING jdk6 web server. Again, if you disagree with any of these 4
> points can you please state WHY and WHAT the alternative to the respective
> POINT is
>
> On Tuesday, 20 February 2018, 17:18:20 GMT, Rob McKenna
>  wrote:
>
>
> W.r.t. alternatives the HTTP serving landscape on the JVM is rich and
> diverse at this point. Projects worth a look include Grizzly, Netty, Jetty,
> Tomcat, Undertow, Rapidoid and the many cool frameworks build on top of
> these technologies. (e.g.  Jooby, SparkJava, Play to name a few)
>
> -Rob
>
> On 20/02/18 16:15, Ashton Hogan wrote:
>>  These items are essential to keeping the server up to date and keeping
>> the code at Oracle clean and up to standard:
>>
>> 1. Update to HTTP22. Remove excess threads, only one thread is needed3.
>> Replace handler with a FIFO queue4. Clean up code, ideally with
>> http://elegantobjects.org principles
>> If you disagree on any of them please reply with why and what the
>> alternative should be



-- 
- DML

Re: Adding SocketChannel toString to connection exception messages

[David Lloyd]

On Fri, Dec 29, 2017 at 11:15 AM, Chris Hegarty
 wrote:
> On 29 Dec 2017, at 00:33, Steven Schlansker  
> wrote:
>> Thanks for the discussion!
>>
>> So, it sounds like amending the message by default is going to be a 
>> non-starter -- but at least adding the information otherwise might be 
>> possible.
>>
>> One possibility would be to add new fields to SocketException, for example 
>> an InetSocketAddress representing both the local and remote (if available).
>
> You would need to careful to not disclose resolved addresses to untrusted 
> code. SocketException, since a subtype of IOException, can wind up in many 
> places.
>
> Would you be proposing to add these new fields to the serial-form of 
> SocketException? What behaviour do you envisage for deserializing instances 
> from previous releases? This will have an impact of any potential API.

This is an advantage to a setter-only message supplement method: if
the supplementary field is set, the writeReplace method can swap it
for a new instance with the combined message.

-- 
- DML

Re: JEP 321: HTTP Client (Standard)

[David Lloyd]

On Thu, Dec 7, 2017 at 8:32 AM, Chris Hegarty <chris.hega...@oracle.com> wrote:
> David,
>
> On 07/12/17 13:14, David Lloyd wrote:
>>
>> On Thu, Dec 7, 2017 at 5:53 AM, Alan Bateman <alan.bate...@oracle.com>
>> wrote:
>>>
>>> This thread is getting a little off-topic but...
>>
>>
>> Getting it back on topic again:
>>
>>> Proposal for the standard module name: java.net.httpclient. Proposal for
>>> the standard package name: java.net.http.
>>
>>
>> I think it would be better if both the module and the package were
>> "java.net.http.client", for two reasons.  Firstly, it is important to
>> align the package and module name whenever possible; I don't think
>> there's a compelling reason here (or most anywhere else) not to do so.
>> Secondly, it is not unreasonable to expect that the future may bring
>> other HTTP-related APIs that are not necessarily client-specific.
>>
>> Relatedly, it may be wise to rename "HttpRequest" and "HttpResponse"
>> to "HttpClientRequest" and "HttpClientResponse", respectively.
>
>
> I agree that symmetry between the module name and package name is
> desirable. If the module / package name contains `client`, then it is
> effectively redundant in the type name ( unless there are many types
> being imported from other unrelated HTTP libraries ).

HttpRequest and HttpResponse are very common names for both client and
server libraries.  I would not consider it odd for a server to have
HttpRequest for incoming requests; if the client also uses HttpRequest
for its outbound requests, then a bunch of annoying qualification will
be going on.

It seems redundant, but the namespacing of the package and the class
serve different purposes: the package is to reserve space for other
specs, the class is to avoid conflict with servers.

> The JEP does make a proposal on the module and package name, but I think
> a discussion on naming will be needed. I'm not sure that we need to
> decide that now, but your point has been noted.

OK.


-- 
- DML

Re: JEP 321: HTTP Client (Standard)

[David Lloyd]

On Thu, Dec 7, 2017 at 5:53 AM, Alan Bateman  wrote:
> This thread is getting a little off-topic but...

Getting it back on topic again:

> Proposal for the standard module name: java.net.httpclient. Proposal for the 
> standard package name: java.net.http.

I think it would be better if both the module and the package were
"java.net.http.client", for two reasons.  Firstly, it is important to
align the package and module name whenever possible; I don't think
there's a compelling reason here (or most anywhere else) not to do so.
Secondly, it is not unreasonable to expect that the future may bring
other HTTP-related APIs that are not necessarily client-specific.

Relatedly, it may be wise to rename "HttpRequest" and "HttpResponse"
to "HttpClientRequest" and "HttpClientResponse", respectively.


-- 
- DML

Re: JEP 321: HTTP Client (Standard)

[David Lloyd]

On Mon, Dec 4, 2017 at 5:01 PM, Chris Hegarty <chris.hega...@oracle.com> wrote:
> On 4 Dec 2017, at 22:03, David Lloyd <david.ll...@redhat.com> wrote:
>
> ...
>
> You mention general-purpose concepts such as ByteBufferReference and
> ByteBufferPool. Note that these are tiny implementation classes (150 lines
> in total) and not exposed in the API.
>
>
> Yes they are, currently - at least ByteBufferReference is at the heart of
> it:
>
> http://hg.openjdk.java.net/jdk/jdk/file/6dcbdc9f99fc/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/AsyncConnection.java#l61
>
>
> I see my error, this is a non-public interface.  Nonetheless, I'm not
> sure that it's really safe to say that this is ready.  Has there been
> _any_ external feedback on this API?
>
>
> [ You can probably ignore my previous email, I sent it before receiving your
> reply. The confusion seems to have been resolved now. ]
>
> Yes, there has been external feedback on the API. I’ll dig it up, I need to
> trawl the net-dev archives and JIRA issues.
>
> For your, and others, reference, here is a snapshot of the latest API, built
> from the ‘http-client-branch’ of the sandbox:
>
>
> http://cr.openjdk.java.net/~chegar/httpclient/javadoc/api/jdk/incubator/http/package-summary.html

Thanks.  If you don't mind answering one more question: is there any
possibility to intercept authentication completely?  There does not
seem to be a lot of documentation about authentication in this API, or
what happens (for example) if there is no Authenticator.

-- 
- DML

Re: JEP 321: HTTP Client (Standard)

[David Lloyd]

On Mon, Dec 4, 2017 at 3:56 PM, David Lloyd <david.ll...@redhat.com> wrote:
> On Mon, Dec 4, 2017 at 2:01 PM, Alan Bateman <alan.bate...@oracle.com> wrote:
>> On 04/12/2017 18:41, David Lloyd wrote:
>>>
>>> :
>>> Speaking *solely* in the interests of platform quality and integrity,
>>> I think that before _any_ high-level non-blocking/asynchronous
>>> protocol API is ever introduced into the platform, it would be an
>>> incredible waste to not have some kind of design consultation with
>>> other industry experts.  Now I'm not suggesting that a JDK API would
>>> have to be _agreeable_ to every expert, as we all know that is
>>> basically impossible; but at the very minimum, I am very confident
>>> that we can tell you what _doesn't_ work and the pitfalls we've found
>>> along the way, as well as what each of us would consider to be an
>>> ideal API, and that is information that has incredible value.
>>>
>> The HTTP client API has been an ongoing effort for several years, the
>> original JEP goes back to 2014. It was initially available in the OpenJDK
>> sandbox and in the JDK main-line before moving to an incubating module in
>> 2016. I can't tell if you've been following this project or not but there
>> has been lots of opportunity to try it out and provide feedback on both the
>> API and implementation.
>
> I've had opportunity to give feedback, perhaps, though the API always
> seemed incomplete.  At least nobody (that I saw) sent out a message
> saying "Here it is, it's all done, what do you think?".  I've
> certainly never had opportunity to try it out: given its status as an
> incubating module present only in OpenJDK, the only people who are
> really in a position to try it out are those using OpenJDK (as opposed
> to other JDKs) with the flexibility to rewrite their use case if and
> when the API changes status (being integrated or disappearing) or form
> (evolving, presumably as a response to feedback), or people writing
> throwaway applications for the sole purpose of testing this particular
> API.  But those who are best able to make this kind of determination
> are those who need to be able to immediately use the API, and rely
> upon it indefinitely (for good or bad), which is definitely not the
> case for anything incubated in the OpenJDK project.  Why take the risk
> when you can use the Apache HTTP client instead?
>
> The lack of feedback on a proposed standard should not be considered a
> tacit endorsement of it - quite the opposite in fact.  It should be
> considered overwhelming disinterest (i.e. there's nothing particularly
> compelling about it), or at absolute best, insufficient information to
> make a determination.  The burden should be on the proposer to
> evangelize their idea and seek out feedback, rather than waiting for
> interest to appear and feedback to come to them.
>
>> You mention general-purpose concepts such as ByteBufferReference and
>> ByteBufferPool. Note that these are tiny implementation classes (150 lines
>> in total) and not exposed in the API.
>
> Yes they are, currently - at least ByteBufferReference is at the heart of it:
>
> http://hg.openjdk.java.net/jdk/jdk/file/6dcbdc9f99fc/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/AsyncConnection.java#l61

I see my error, this is a non-public interface.  Nonetheless, I'm not
sure that it's really safe to say that this is ready.  Has there been
_any_ external feedback on this API?

-- 
- DML

Re: JEP 321: HTTP Client (Standard)

[David Lloyd]

On Mon, Dec 4, 2017 at 2:01 PM, Alan Bateman <alan.bate...@oracle.com> wrote:
> On 04/12/2017 18:41, David Lloyd wrote:
>>
>> :
>> Speaking *solely* in the interests of platform quality and integrity,
>> I think that before _any_ high-level non-blocking/asynchronous
>> protocol API is ever introduced into the platform, it would be an
>> incredible waste to not have some kind of design consultation with
>> other industry experts.  Now I'm not suggesting that a JDK API would
>> have to be _agreeable_ to every expert, as we all know that is
>> basically impossible; but at the very minimum, I am very confident
>> that we can tell you what _doesn't_ work and the pitfalls we've found
>> along the way, as well as what each of us would consider to be an
>> ideal API, and that is information that has incredible value.
>>
> The HTTP client API has been an ongoing effort for several years, the
> original JEP goes back to 2014. It was initially available in the OpenJDK
> sandbox and in the JDK main-line before moving to an incubating module in
> 2016. I can't tell if you've been following this project or not but there
> has been lots of opportunity to try it out and provide feedback on both the
> API and implementation.

I've had opportunity to give feedback, perhaps, though the API always
seemed incomplete.  At least nobody (that I saw) sent out a message
saying "Here it is, it's all done, what do you think?".  I've
certainly never had opportunity to try it out: given its status as an
incubating module present only in OpenJDK, the only people who are
really in a position to try it out are those using OpenJDK (as opposed
to other JDKs) with the flexibility to rewrite their use case if and
when the API changes status (being integrated or disappearing) or form
(evolving, presumably as a response to feedback), or people writing
throwaway applications for the sole purpose of testing this particular
API.  But those who are best able to make this kind of determination
are those who need to be able to immediately use the API, and rely
upon it indefinitely (for good or bad), which is definitely not the
case for anything incubated in the OpenJDK project.  Why take the risk
when you can use the Apache HTTP client instead?

The lack of feedback on a proposed standard should not be considered a
tacit endorsement of it - quite the opposite in fact.  It should be
considered overwhelming disinterest (i.e. there's nothing particularly
compelling about it), or at absolute best, insufficient information to
make a determination.  The burden should be on the proposer to
evangelize their idea and seek out feedback, rather than waiting for
interest to appear and feedback to come to them.

> You mention general-purpose concepts such as ByteBufferReference and
> ByteBufferPool. Note that these are tiny implementation classes (150 lines
> in total) and not exposed in the API.

Yes they are, currently - at least ByteBufferReference is at the heart of it:

http://hg.openjdk.java.net/jdk/jdk/file/6dcbdc9f99fc/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/AsyncConnection.java#l61

And that class relies directly on ByteBufferPool in its own API.

Unless the jdk/jdk branch does not reflect the latest incarnation of
this JEP, in which case I definitely haven't have been up to date with
it, despite following this list.

> Promoting these to java.nio would be
> outside the scope of this API. If the buffer API were to add such concepts
> in the future then there is nothing to stop the HTTP client implementation
> making use in its implementation.

See above.


-- 
- DML

Re: JEP 321: HTTP Client (Standard)

[David Lloyd]

On Mon, Dec 4, 2017 at 10:17 AM,   wrote:
> New JEP Candidate: http://openjdk.java.net/jeps/321

I have concerns.

This will be the first public, NIO-based, asynchronous/non-blocking
network protocol API introduced into the JDK proper, _ever_.

First, I want to note that the API seems to bear no resemblance
whatsoever with the asynchronous NIO.2 API.  Now I'm no fan of that
API as it stands for a couple of reasons, but nevertheless it calls
into question either the validity of the HTTP API as it stands, or the
scope of reusability of the NIO.2 API.

With that items put aside: there are a wide variety of mature,
non-blocking network protocol implementations out there, with
literally thousands of years of experience distributed amongst their
authors, maintainers, supporters, and communities, none of which were
used as a model.  There was, as far as I can see, no kind of study of
existing non-blocking approaches in Java, and their strengths and
weaknesses; there was no round table of engineers with experience in
this field, talking about what works and what doesn't work.

Some new, seemingly general-purpose, concepts are introduced by the
code base, for example: ByteBufferReference, and ByteBufferPool.  Are
these strategies that should be promoted to NIO proper?  If not, then
are they _really_ right for this particular use case, particularly if,
for some reason, a _second_ non-blocking network protocol API might be
introduced some day, probably duplicating these concepts?

Making this thing be the first real platform NIO-based asynchronous
network protocol API, yet being completely removed from the previous
NIO.2 asynchronous APIs and any other existing stable, mature API,
should be done very carefully and deliberately, and perhaps most
importantly, incrementally: first, establish a non-blocking byte
stream API that makes sense generally, and bring that into NIO
(NIO.3?); then, perhaps, enhancements to byte buffers to better
support efficient pooling.  By the time that point is reached, it is
hopefully rapidly becoming obvious that this is not something that
should be taken lightly.

I believe that most third-party implementations are taken less lightly
than this seems to have been.  I and my team have been developing an
asynchronous/non-blocking NIO library for ten years now, and while I'm
proud of our work and the discoveries we've made, I am realistic about
the fact that it's still pretty far from as good as it could be (not
in the least part due to existing platform limitations), certainly far
from something I'd say "hey let's standardize this as is".  I think
that to standardize something of this type that was just written over
the past 18-odd months reflects, to put it kindly, some pretty
incredible confidence that I wish I shared.

Speaking *solely* in the interests of platform quality and integrity,
I think that before _any_ high-level non-blocking/asynchronous
protocol API is ever introduced into the platform, it would be an
incredible waste to not have some kind of design consultation with
other industry experts.  Now I'm not suggesting that a JDK API would
have to be _agreeable_ to every expert, as we all know that is
basically impossible; but at the very minimum, I am very confident
that we can tell you what _doesn't_ work and the pitfalls we've found
along the way, as well as what each of us would consider to be an
ideal API, and that is information that has incredible value.

Talking about introducing the first-ever non-blocking protocol API
into the platform, at _this_ stage, seems premature and needlessly
risky.  I would suggest that maybe it's best for the API to stick to
blocking I/O, at least for now.  Or else, take it outside of the
platform, and let it mature in the wild where it can evolve without an
overbearing concern for compatibility for a decade or so (no, I'm not
kidding).  As long as this thing lives in the JDK, but isn't
standardized, it's probably not going to be used heavily enough to
really feel out its weak points.  And once it's in, its ability to
evolve is severely hampered by compatibility constraints.

I feel like it is impossible to over-emphasize the difficulty of the
problem of non-blocking I/O when it comes to interactions with user
programs.  Though the fruits of such an effort are probably small in
terms of API surface, the complexity is hard: hard enough that it is,
in my mind, a project of a larger scale, maybe JSR scale.  And the
benefit is potentially large: large enough that it could change the
landscape of other specifications and network applications.  Or at
least, I think so, which is why I've spent so many years of my life in
pursuit of such a thing.

-- 
- DML

Re: Parsing too strict in java.net.URI?

[David Lloyd]

On Mon, Nov 14, 2016 at 12:16 PM, Chris Hegarty
 wrote:
> David,
>
> On 14/11/16 16:47, David M. Lloyd wrote:
>>
>> The following statement:
>>
>> URI uri = URI.create("local:");
>>
>> fails with an exception like this:
[...]
>> However AFAICT scheme-only URIs are, while not strictly allowed by RFC
>> 2396 [1], in common usage (for example, "about:" in web browsers).
>
> This seems to be allowable in the "more recent" RFC [2], that obsoletes
> 2396, which of course java.net.URI does not, yet, support. There was an
> effort to support 3986 a number of years ago, but it was not successful.
> The desire to support, keep up with standards, etc, has caused us to
> give consideration to supporting 3986, but as of yet nothing concrete.

Have there been any recent developments, updates, or insights on this
issue?  I wonder if a gradual change to RFC 3986 support might be more
appropriate: for example, start with *just* accepting empty SSP.

If the compatibility requirements of java.net.URI are too stringent to
allow for any change, then surely all that can be done is to deprecate
& replace it, which would need no further delay that I can see.
-- 
- DML

Re: RFR: 8189366: SocketInputStream.available() should check for eof

[David Lloyd]

On Thu, Oct 26, 2017 at 4:44 AM, Bernd Eckenfels  wrote:
> What is currently returned at the end of a stream? This looks like a
> dangerous thing to do, if a existing implementation only read when something
> is available it might never detect that it reached EOF.

At present it ultimately delegates (on UNIXy) to ioctl(fd, FIONREAD,
xxx) which I guess will return an errno in this case.  If that's the
case then I agree that returning 0 is a smarter option.

-- 
- DML