harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Hindess <mark.hind...@googlemail.com>
Subject Re: [classlib] Network changes causing linux hang in HttpURLConnectionTest
Date Thu, 11 Dec 2008 13:14:23 GMT

In message <4940FD24.5030506@gmail.com>, Tim Ellison writes:
>
> Mark Hindess wrote:
> > In message <200812110854.mBB8shW9030452@d06av02.portsmouth.uk.ibm.com>,
> > "Mark Hindess" writes:
> >> When running 
> >> org.apache.harmony.luni.tests...protocol.http.HttpURLConnectionTest
> >> I am seeing a hang in testConnectionPersistence method on linux
> >> (x86-64 and x86).
> >> [snip]
> > 
> > Ok.  It looks like there is a problem with the selectRead implementation on
> > unix.  The use of this function in:
> > 
> >   Java_org_apache_harmony_luni_platform_OSNetworkSystem_readDirect
> > 
> > compares the result of the selectRead call using portlib constants.  This
> > is valid for the windows implementation of selectRead - because it uses
> > hysock_select.  However, the unix implementation uses poll which is returni
> ng:
> > 
> >   On success, a positive number is returned; [snip].  A value of 0
> >   indicates that the call timed out and no file descriptors were ready.
> >   On error, -1 is returned, and errno is set appropriately.
> > 
> > I think the fix is:
> > 
> > 1) Check for other uses of selectRead and make sure they all use portlib 
> > constants.
> > 
> > 2) Fix selectRead on unix to map the poll return codes to portlib constants
> .
> > 
> > I'll take a look at doing this.  Shout if you don't think this is a good
> > approach.
> 
> 
> Having selectRead return different values on different platforms is not
> a good idea (even though it is not a portlib function), so yes it should
> be fixed.
> 
> There is plenty of work left in tidying up and optimizing the networking
> code.  Thanks for tracking this down.

It gets worse...

Fixing 1) is find because there are no other uses (except one commented out).

Fixing 2) is ugly because the findError function that converts system
socket errors to portlib ones is static and thus not accessible.  I've worked
around this by just returning either HYPORT_ERROR_SOCKET_TIMEOUT or the
generic HYPORT_ERROR_SOCKET_OPFAILED which is sufficient.  See r725677.

I also notice that hysock_select in unix/hysock.c says:

  @return 0 if timeout, number of ready FDs, or otherwise return the (negative)
          error code.

but in fact actually does:

  result = select(...);
  ...
  if (result) {
    rc = result
  } else {
    rc = HYPORT_ERROR_SOCKET_TIMEOUT;
  }

so I now wonder if any hysock_select callers are expecting the documented
behaviour.  (I'm afraid to even look at the windows version.)

Regards,
 Mark.



Mime
View raw message