I have applied the following comment patch.  The current code resets the
timer when select() is interruped.  On OS's that modify timeout to show
the remaining time, we should be using that value instead of resetting
the timer to its original value on select retry.

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

pgman wrote:
> 
> Oops, overoptimized a little. ptmp_timeout is needed in case no time is
> passed;  ptmp_timeout restored.
> 
> ---------------------------------------------------------------------------
> 
> pgman wrote:
> > Tom Lane wrote:
> > > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > > That whole remains structure should be a time_t variable, and then we
> > > > _know_ we can't assume it is signed.  The use of timeval should
> > > > happen only in pqWaitTimed because it has to use select().
> > > 
> > > I think it's fine to use struct timeval as the parameter type for
> > > pqWaitTimed.  This particular caller of pqWaitTimed has no need for
> > > sub-second wait precision, but that doesn't mean we might not want it
> > > for other purposes later.
> > 
> > That was a question:  whether pqWaitTimed() was something exported by
> > libpq and therefore something that has an API that shouldn't change.  I
> > see it in libpq-int.h, which I think means it isn't exported, but yes,
> > there could be later cases where we need subsecond stuff.
> > 
> > I have applied the following patch to get us a little closer to sanity.
> 
> -- 
>   Bruce Momjian                        |  http://candle.pha.pa.us
>   pgman@candle.pha.pa.us               |  (610) 359-1001
>   +  If your life is a hard drive,     |  13 Roberts Road
>   +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Index: src/interfaces/libpq/fe-misc.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/interfaces/libpq/fe-misc.c,v
retrieving revision 1.82
diff -c -c -r1.82 fe-misc.c
*** src/interfaces/libpq/fe-misc.c	14 Oct 2002 17:33:08 -0000	1.82
--- src/interfaces/libpq/fe-misc.c	14 Oct 2002 18:08:14 -0000
***************
*** 824,830 ****
  		{
  			/*
  			 * 	select() may modify timeout argument on some platforms so
! 			 *	use copy
  			 */
  			tmp_timeout = *timeout;
  			ptmp_timeout = &tmp_timeout;
--- 824,835 ----
  		{
  			/*
  			 * 	select() may modify timeout argument on some platforms so
! 			 *	use copy.
! 			 *	XXX Do we really want to do that?  If select() returns
! 			 *	the number of seconds remaining, we are resetting
! 			 *	the timeout to its original value.  This will yeild
! 			 *	incorrect timings when select() is interrupted.
! 			 *	bjm 2002-10-14
  			 */
  			tmp_timeout = *timeout;
  			ptmp_timeout = &tmp_timeout;
Mon, 14 Oct 2002 14:10:41 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: droped out precise time calculations in
	src/interfaces/libpq/fe-connect.c
Bruce Momjian <pgman@candle.pha.pa.us> writes:
>               /*
>                *     select() may modify timeout argument on some platforms so
> !              *    use copy.
> !              *    XXX Do we really want to do that?  If select() returns
> !              *    the number of seconds remaining, we are resetting
> !              *    the timeout to its original value.  This will yeild
> !              *    incorrect timings when select() is interrupted.
> !              *    bjm 2002-10-14
>                */
>               tmp_timeout = *timeout;
>               ptmp_timeout = &tmp_timeout;


Actually, now that I look at this, the API for PQwaitTimed is wrong
after all.  The right way to implement this is for the caller to pass in
finish_time (or some indication that no timeout is wanted).  Evaluation
of the time left to wait should happen inside this retry loop.  That
way, you get the right behavior (plus or minus one second, anyway)
independently of whether the platform's select() reduces its timeout
argument or not.

			regards, tom lane

Mon, 14 Oct 2002 14:42:47 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: droped out precise time calculations in
	src/interfaces/libpq/fe-connect.c
Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> >               /*
> >                *     select() may modify timeout argument on some platforms so
> > !              *    use copy.
> > !              *    XXX Do we really want to do that?  If select() returns
> > !              *    the number of seconds remaining, we are resetting
> > !              *    the timeout to its original value.  This will yeild
> > !              *    incorrect timings when select() is interrupted.
> > !              *    bjm 2002-10-14
> >                */
> >               tmp_timeout = *timeout;
> >               ptmp_timeout = &tmp_timeout;
> 
> 
> Actually, now that I look at this, the API for PQwaitTimed is wrong
> after all.  The right way to implement this is for the caller to pass in
> finish_time (or some indication that no timeout is wanted).  Evaluation
> of the time left to wait should happen inside this retry loop.  That
> way, you get the right behavior (plus or minus one second, anyway)
> independently of whether the platform's select() reduces its timeout
> argument or not.

Yes, you are saying do the time() inside PQwaitTimed(), so we can
properly get new time() values on select() retry.  Yep.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Mon, 14 Oct 2002 18:41:27 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: droped out precise time calculations in
	src/interfaces/libpq/fe-connect.c