BUG #1467: fe_connect doesn't handle EINTR right - Mailing list pgsql-patches

Attached is a patch which corrects the behavior. I verified that the
patch does not interfere with normal operation (using psql) but
unfortunately the code path is virtually impossible to test without a
really slow connection to a postgresql server [which I thankfully
don't have]. To test the patch, you would need to send an interrupt
at the exact time that the kernel is connect()ing in blocking mode-
good luck.

Also, I recommend removing a (sarcastic?) comment left by a previous
developer- I wrote a note about in my patch.

The patch is against 8.0.3 [because HEAD requires access to a
specific version of bison] but I imagine that the code hasn't changed
in fe-connect.c since then.

Patch against src/interfaces/libpq/fe-connect.c (v 8.0.3)

Bonus link: http://maps.google.com/maps?q=13+Roberts+Road,+Newtown
+Square,+Pennsylvania&spn=0.007639,0.010654&t=k&hl=en

Begin forwarded message:

> From: Bruce Momjian <pgman@candle.pha.pa.us>
> Date: June 24, 2005 9:27:09 PM CDT
> To: AgentM <agentm@themactionfaction.com>
> Subject: Re: [HACKERS] [BUGS] BUG #1467: fe_connect doesn't handle
> EINTR right
>
>
>
> Would you develop a patch in the next few days for this?
>
> ----------------------------------------------------------------------
> -----
>
> AgentM wrote:
>
>> It seems pretty cut-and-dry. The reporter is correct:
>> "If connect() is interrupted by a signal that is caught while blocked
>> waiting to establish a connection, connect() shall fail and set errno
>> to [EINTR], but the connection request shall not be aborted, and the
>> connection shall be established asynchronously."
>> http://www.opengroup.org/onlinepubs/009695399/functions/connect.html
>>
>> This must obviously be the case because the kernel cannot "rollback"
>> a TCP/IP connection. This is equivalent to the general case of
>> establishing an asynchronous connection (whose description follows in
>> the next paragraph of the linked text).
>>
>> So the correct code should be:
>>    if (connect(conn->sock, addr_cur->ai_addr,
>>                        addr_cur->ai_addrlen) < 0)
>>    {
>>      fd_set writefd;
>>      if (SOCK_ERRNO == EINTR) //pause until connection established
>>      {
>>          do
>>      {
>>          int ret;
>>              FD_ZERO(&writefd);
>>              FD_SET(conn->sock,&writefd);
>>              errno=0;
>>          ret=select(sock->conn+1,NULL,&writefd,NULL,NULL); //wait
>> forever
>>      }while((ret<0 && SOCK_ERRNO==EINTR) || ret<1)
>>      }
>>    } //now ready to write to socket
>>
>> Disclaimer: typed in Mail.app.
>>
>>
>> On Jun 6, 2005, at 8:19 PM, Bruce Momjian wrote:
>>
>>
>>>
>>> Would someone comment on this bug report from February?  I can
>>> confirm
>>> the code is unchanged and is in function fe-connect.c::PQconnectPoll
>>> ().
>>>
>>> --------------------------------------------------------------------
>>> --
>>> -----
>>>
>>> Florian Hars wrote:
>>>
>>>
>>>>
>>>> The following bug has been logged online:
>>>>
>>>> Bug reference:      1467
>>>> Logged by:          Florian Hars
>>>> Email address:      hars@bik-gmbh.de
>>>> PostgreSQL version: 8.0.1
>>>> Operating system:   All
>>>> Description:        fe_connect doesn't handle EINTR right
>>>> Details:
>>>>
>>>> The file pgsql/src/interfaces/libpq/fe-connect.c contains the code
>>>> fragment
>>>>
>>>> retry_connect:
>>>>   if (connect(conn->sock, addr_cur->ai_addr,
>>>>                       addr_cur->ai_addrlen) < 0)
>>>>   {
>>>>     if (SOCK_ERRNO == EINTR)
>>>>     /* Interrupted system call - just try again */
>>>>         goto retry_connect;
>>>>   }
>>>>
>>>> This is not in accordance with a strict legalistic reading of the
>>>> POSIX
>>>> spec, according to which connect is not restartable so that you
>>>> have to use
>>>> select or poll after connect returned with EINTR.
>>>>
>>>> See
>>>> http://www.eleves.ens.fr:8080/home/madore/computers/connect-
>>>> intr.html
>>>> for the ugly details, your code should work on Linux, but not on
>>>> Solaris or
>>>> (Free|Open)BSD.
>>>>
>>>> ---------------------------(end of
>>>> broadcast)---------------------------
>>>> TIP 4: Don't 'kill -9' the postmaster
>>>>
>>>>
>>>>
>>>
>>> --
>>>   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
>>>
>>> ---------------------------(end of
>>> broadcast)---------------------------
>>> TIP 8: explain analyze is your friend
>>>
>>>
>>
>> |-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-
>> AgentM
>> agentm@themactionfaction.com
>> |-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-
>>
>>
>
> --
>   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
>

|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-
AgentM
agentm@themactionfaction.com
|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-