Mark Hindess wrote:
> In message <[EMAIL PROTECTED]>, Tim Ellison writes:
>> Mark Hindess wrote:
>>> In message <[EMAIL PROTECTED]>,
>>> "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
