[PATCH lib/libxtrans] If Socket is getting interrupted with signal EINTR, after re attempts we are closing socket connection, which is wrong.

[Arvind Umrao]

Could I get approvals for change in behavior of socket, when interrupted 
with signal ENTR? I have tested my fixes in Solaris and Ubuntu. My code 
changes are not breaking anything. Mark Kettenis, suggest me to fix 
caller function also, for eg libICE/src/connect.c:ConnectToPeer()  and I 
will do it.

Thanks and Regards
-Arvind


On 08/27/12 12:45 PM, Arvind Umrao wrote:
> Code changes are integrated in Solaris and now I am trying to give back to
> community. If Socket is getting interrupted with signal EINTR, we should keep
> socket in progress state(TRANS_IN_PROGRESS) instead of trying again to
> connect(TRANS_TRY_CONNECT_AGAIN). When we close the socket connection on signal
> EINTR and retry, we will end up in same old state and stuck in loop. As per
> documentation if Connect() is interrupted by a signal that is caught, while
> blocked waiting to establish a connection, connect() shall fail and set
> connect() to [EINTR], but the connection request shall not be aborted, and the
> connection shall be established asynchronously. When the connection has been
> established asynchronously, select() and poll() shall indicate that the file
> descriptor for the socket is ready for writing.
>
> For both blocking and non blocking, if connect()interrupted with Signal EINTR,
> it should return TRANS_IN_PROGRESS. In Unix Network Programming, volume 1,
> section 5.9, W. Richard Stevens states:
> What we are doing […] is restarting the interrupted system call ourself.
> This is fine for accept, along with the functions such as read, write, select
> and open. But there is one function that we cannot restart ourself: connect. If
> this function returns EINTR, we cannot call it again, as doing so will return
> an immediate error. When connect is interrupted by a caught signal and is not
> automatically restarted, we must call "Select" to wait for the connection to
> complete, as we describe in section 15.3.
>
> Refer http://www.madore.org/~david/computers/connect-intr.html
>
> Signed-off-by: Arvind Umrao<arvind.umrao at oracle.com>
> ---
>   Xtranssock.c |   12 +++++-------
>   1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/Xtranssock.c b/Xtranssock.c
> index dfa41cf..eaef6f0 100644
> --- a/Xtranssock.c
> +++ b/Xtranssock.c
> @@ -1717,7 +1717,7 @@ TRANS(SocketINETConnect) (XtransConnInfo ciptr, char *host, char *port)
>   	 * was non-blocking and we should poll using select
>   	 *
>   	 * If the error was EINTR, the connect was interrupted and we
> -	 * should try again.
> +	 * should poll using select.
>   	 *
>   	 * If multiple addresses are found for a host then we should
>   	 * try to connect again with a different address for a larger
> @@ -1727,7 +1727,7 @@ TRANS(SocketINETConnect) (XtransConnInfo ciptr, char *host, char *port)
>   	 * only affect one of a set of addresses.
>   	 */
>
> -	if (olderrno == ECONNREFUSED || olderrno == EINTR
> +	if (olderrno == ECONNREFUSED
>   #if defined(IPv6)&&  defined(AF_INET6)
>   	  || (((addrlist->addr->ai_next != NULL) ||
>   	        (addrlist->addr != addrlist->firstaddr))&&
> @@ -1740,7 +1740,7 @@ TRANS(SocketINETConnect) (XtransConnInfo ciptr, char *host, char *port)
>   #endif
>   	    )
>   	    res = TRANS_TRY_CONNECT_AGAIN;
> -	else if (olderrno == EWOULDBLOCK || olderrno == EINPROGRESS)
> +	else if (olderrno == EWOULDBLOCK || olderrno == EINPROGRESS || olderrno == EINTR)
>   	    res = TRANS_IN_PROGRESS;
>   	else
>   	{
> @@ -2022,13 +2022,11 @@ TRANS(SocketUNIXConnect) (XtransConnInfo ciptr, char *host, char *port)
>   	     * was non-blocking and we should poll using select
>   	     *
>   	     * If the error was EINTR, the connect was interrupted and we
> -	     * should try again.
> +	     * should poll using select.
>   	     */
>
> -	    if (olderrno == EWOULDBLOCK || olderrno == EINPROGRESS)
> +	    if (olderrno == EWOULDBLOCK || olderrno == EINPROGRESS || olderrno == EINTR)
>   		return TRANS_IN_PROGRESS;
> -	    else if (olderrno == EINTR)
> -		return TRANS_TRY_CONNECT_AGAIN;
>   	    else if (olderrno == ENOENT || olderrno == ECONNREFUSED) {
>   		/* If opening as abstract socket failed, try again normally */
>   		if (abstract) {