Skip site navigation (1) Skip section navigation (2)

Re: [PATCHES] [patch] fe-connect.c doesn't handle EINTR correctly

From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: David Ford <david+cert(at)blue-labs(dot)org>
Cc: pgsql-patches(at)postgresql(dot)org, pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCHES] [patch] fe-connect.c doesn't handle EINTR correctly
Date: 2002-04-14 04:54:46
Message-ID: 200204140454.g3E4skw02153@candle.pha.pa.us (view raw or flat)
Thread:
Lists: pgsql-hackerspgsql-patches
David, sorry you patch didn't make it into 7.2.X.  That whole EINTR
discussion was quite complicated so I am not surprised we missed it.

The attached patch implements your ENITR test in cases that seems to
need it.  I have followed the method we used for ENITRY in fe-misc.c.


---------------------------------------------------------------------------

David Ford wrote:
> Last year we had a drawn out discussion about this and I created a patch 
> for it.  I never noticed that the patch didn't go in until I installed 
> 7.2 the other day and realised that fe-connect.c never was fixed.
> 
> Here is the patch again.  It is against CVS 3/16/2002.  This time I only 
> rewrote the connect procedure at line 912, I leave it up to the regular 
> hackers to copy it's functionality to the SSL procedure just below it.
> 
> In summary, if a software writer implements timer events or other events 
> which generate a signal with a timing fast enough to occur while libpq 
> is inside connect(), then connect returns -EINTR.  The code following 
> the connect call does not handle this and generates an error message. 
>  The sum result is that the pg_connect() fails.  If the timer or other 
> event is right on the window of the connect() completion time, the 
> pg_connect() may appear to work sporadically.  If the event is too slow, 
> pg_connect() will appear to always work and if the event is too fast, 
> pg_connect() will always fail.
> 
> David
> 

> Index: src/interfaces/libpq/fe-connect.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
> retrieving revision 1.181
> diff -u -r1.181 fe-connect.c
> --- src/interfaces/libpq/fe-connect.c	2001/11/11 02:09:05	1.181
> +++ src/interfaces/libpq/fe-connect.c	2002/03/16 05:17:47
> @@ -909,29 +909,48 @@
>  	 * Thus, we have to make arrangements for all eventualities.
>  	 * ----------
>  	 */
> -	if (connect(conn->sock, &conn->raddr.sa, conn->raddr_len) < 0)
> -	{
> -		if (SOCK_ERRNO == EINPROGRESS || SOCK_ERRNO == EWOULDBLOCK || SOCK_ERRNO == 0)
> -		{
> -			/*
> -			 * This is fine - we're in non-blocking mode, and the
> -			 * connection is in progress.
> -			 */
> -			conn->status = CONNECTION_STARTED;
> -		}
> -		else
> -		{
> -			/* Something's gone wrong */
> -			connectFailureMessage(conn, SOCK_ERRNO);
> -			goto connect_errReturn;
> +	do {
> +		int e;
> +		e=connect(conn->sock, &conn->raddr.sa, conn->raddr_len)
> +
> +		if(e < 0) {
> +			switch (e) {
> +				case EINTR:
> +					/*
> +					 * Interrupted by a signal, keep trying.  This handling is
> +					 * required because the user may have turned on signals in
> +					 * his program.  Previously, libpq would erronously fail to
> +					 * connect if the user's timer event fired and interrupted
> +					 * this syscall.  It is important that we don't try to sleep
> +					 * here because this may cause havoc with the user program.
> +					 */
> +					continue;
> +					break;
> +				case 0:
> +				case EINPROGRESS:
> +				case EWOULDBLOCK:
> +					/*
> +					 * This is fine - we're in non-blocking mode, and the
> +					 * connection is in progress.
> +					 */
> +					conn->status = CONNECTION_STARTED;
> +					break;
> +				default:
> +					/* Something's gone wrong */
> +					connectFailureMessage(conn, SOCK_ERRNO);
> +					goto connect_errReturn;
> +					break;
> +			}
> +		} else {
> +			/* We're connected now */
> +			conn->status = CONNECTION_MADE;
>  		}
> -	}
> -	else
> -	{
> -		/* We're connected already */
> -		conn->status = CONNECTION_MADE;
> -	}
> +		
> +		if(conn->status == CONNECTION_STARTED || conn->status == CONNECTION_MADE)
> +			break;
>  
> +	} while(1);
> +	
>  #ifdef USE_SSL
>  	/* Attempt to negotiate SSL usage */
>  	if (conn->allow_ssl_try)

> 
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
> 
> http://www.postgresql.org/users-lounge/docs/faq.html

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman(at)candle(dot)pha(dot)pa(dot)us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

In response to

Responses

pgsql-hackers by date

Next:From: Christopher Kings-LynneDate: 2002-04-14 04:58:43
Subject: Re: DROP COLUMN (was RFC: Restructuring pg_aggregate)
Previous:From: Neil ConwayDate: 2002-04-14 04:36:38
Subject: Re: experimental pg_qcache patch

pgsql-patches by date

Next:From: Christopher Kings-LynneDate: 2002-04-14 04:56:25
Subject: BETWEEN Help
Previous:From: Bruce MomjianDate: 2002-04-13 20:37:07
Subject: Re: docbook.m4

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group