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

[David Buck]

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. 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). 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
>
>