Code Review Request : 7011591 JDWP socket transport should restart interrupted system calls (EINTR)

[Dmitry Samersoff]

On 2011-08-09 10:18, David Buck wrote:
> Hi Dmitry!
>
> Thank you for the helpful feedback.
>
> Even on Solaris, I should have called getsockopt() after poll() returns.
> Not sure how I missed that. Thanks for the catch!
>
> I am not so sure i should include a second call to connect() after the
> first one returns with EINTR. All of the "standard" unix docs (SUS,
> Richard Steven's "Unix Network Programming") for connect() clearly
> require that an interrupted call to connect() (on a blocking socket)
> result in a asynchronous connection setup.

connect() continues async setup for both blocking/non-blocking socket.

> This seems to work fine on
> Linux as well. My understanding is that the fact that Linux will
> sometimes let us just re-call connect() and get the blocking behavior
> (just like our original call) or return EINPROGRESS is more of a
> convenience (that encourages programmers to write non-portable code).

Behavior of second connect() call is portable:
for blocking/non-blocking sockets it can: - block/retry; return EISCONN;
return EALREADY/EINPROGRESS.

Generally speaking it allows better error handling than just poll 
because it can catch an error happened between connect and poll calls 
(e.g. a peer reset connection) but in you probably don't need it.

See http://pubs.opengroup.org/onlinepubs/009695399/functions/connect.html

-Dmitry


> In
> other words, I don't think you HAVE to call connect() again before doing
> a poll() or select(). The specification guarantees an asynchronous
> attempt at connection setup and if we found an instance where it didn't
> do that, we would be justified in calling it a bug.
>
> If possible, I would like one implementation that works on both (and
> follows the specification strictly) and avoid adding any ifdefs to the
> code.
>
> Cheers,
> -Buck
>
> On 08/04/11 22:25, Dmitry Samersoff wrote:
>> David,
>>
>> To be on safe side with connect() (on Linux, not sure for Solaris)
>> you need to call connect() again and if it returns EINPROGRESS, run
>> polling loop. After poll() it's recommended to call getsockopt() and
>> read the SO_ERROR to determine whether connect was successful or not.
>>
>> -Dmitry
>>
>>
>> On 2011-08-04 16:02, David Buck wrote:
>>> Hi!
>>>
>>> I would like to request that my fix for 7011591 be reviewed for push
>>> into JDK8 (and JDK7u if possible).
>>>
>>> CR: 7011591 JDWP socket transport should restart interrupted system
>>> calls (EINTR)
>>>
>>> Weblink: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=2205030
>>> (for some reason, the public URL for the parent bug is not working. (I
>>> believe that SubCR 2205030 will have all needed information.)
>>>
>>> Webrev: http://cr.openjdk.java.net/~dbuck/7011591/webrev.00/
>>>
>>> Description:
>>> Syscalls that can be interrupted by signal handling should automatically
>>> retry if they fail with EINTR. Unfortunately, the JDWP socket transport
>>> implementation does not do this. Since HotSpot does not use signals for
>>> thread suspension, HotSpot customers have not run into any problem (as
>>> far as I know). But because JRockit R27 and earlier releases use signals
>>> to suspend threads, all threads are constantly being interrupted thus
>>> JRockit customers are much more likely (almost guaranteed under some
>>> circumstances) to have system calls interrupted by signal handling.
>>>
>>> I have added a loop to automatically retry send(), recv(), sendto(),
>>> recvfrom(), accept(), and poll() calls if the call fails with EINTR.
>>> Note that I do not believe that the UDP versions of the calls
>>> (recvfrom() and sendto()) are actualy ever used in the JDK, but I fixed
>>> them in case they are used in the future.
>>>
>>> The implementation for connect() was more complicated because Solaris
>>> does not let us just call connect() again after EINTR. Once a call to
>>> connect() fails with EINTR on Solaris, the connection setup continues
>>> asynchronously and we need to use select() or poll() to determine when
>>> the socket can be used.
>>>
>>> The patch for jdk7u2 is identical to this (jdk8) fix, if possible, I
>>> would like to receive pre-approval to push to jdk7u-dev repository as
>>> well.
>>>
>>> This fix has already been done for 1.4.2/5/6 as part of the JRockit
>>> Simplification Project. This needs to be forward ported into 7 and 8 to
>>> avoid a regression.
>>>
>>> Best Regards,
>>> -Buck
>>
>>


-- 
Dmitry Samersoff
Java Hotspot development team, SPB04
* There will come soft rains ...