Re: [PATCH] Allow breaking out of hung connection attempts

On 09.01.2012 15:49, Ryan Kelly wrote:
> On Mon, Jan 09, 2012 at 10:35:50AM +0200, Heikki Linnakangas wrote:
>> That assumes that it's safe to longjmp out of PQconnectdbParams at
>> any instant. It's not.
> I'm guessing because it could result in a resource leak?

Yes, and other unfinished business, too.

>> I think you'd need to use the asynchronous connection functions
>> PQconnectStartParams() and PQconnectPoll(), and select().
> New patch attached.

Thanks, some comments:

* Why do you need the timeout?

* If a SIGINT arrives before you set sigint_interrupt_enabled, it just 
sets cancel_pressed but doesn't jump out of the connection attempt. You 
need to explicitly check cancel_pressed after setting 
sigint_interrupt_enabled to close that race condition.

* You have to reinitialize the fd mask with FD_ZERO/SET before each call 
to select(). select() modifies the mask.

* In case of PGRES_POLLING_WRITING, you have to wait until the socket 
becomes writable, not readable.

Attached is a new version that fixes those.

There's one caveat in the libpq docs about PQconnectStart/PQconnectPoll:

> The connect_timeout connection parameter is ignored when using PQconnectPoll; it is the application's responsibility to decide whether an excessive amount of time has elapsed. Otherwise, PQconnectStart followed by a PQconnectPoll loop is equivalent to PQconnectdb.

So after this patch, connect_timeout will be ignored in \connect. That 
probably needs to be fixed. You could incorporate a timeout fairly 
easily into the select() calls, but unfortunately there's no easy way to 
get the connect_timeout value. You could to parse the connection string 
the user gave with PQconninfoParse(), but the effective timeout setting 
could come from a configuration file, too.

Not sure what to do about that. If there was a PQconnectTimeout(conn) 
function, similar to PQuser(conn), PQhost(conn) et al, you could use 
that. Maybe we should add that, or even better, a generic function that 
could be used to return not just connect_timeout, but all the connection 
options in effect in a connection.

-- 
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com