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
todecide whether an excessive amount of time has elapsed. Otherwise, PQconnectStart followed by a PQconnectPoll loop is
equivalentto 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