harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tim Ellison <t.p.elli...@gmail.com>
Subject Re: [classlib] Network changes causing linux hang in HttpURLConnectionTest
Date Thu, 11 Dec 2008 15:49:31 GMT
Mark Hindess wrote:
> 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.)

Windows also returns HYPORT_ERROR_SOCKET_TIMEOUT(-209) for the timeout,
so it appears to be a documentation bug.

Regards,
Tim

Mime
View raw message