[PATCHES] BUG #1467: fe_connect doesn't handle EINTR right

[AgentM]

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.

Advertising

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

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.

sockpatch.diff
Description: Binary data

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 <[EMAIL PROTECTED]>




Would you develop a patch in the next few days for this?



AgentM wrote:

Subject: Re: [HACKERS] [BUGS] BUG #1467: fe_connect doesn't handle EINTR right ---------------------------------------------------------------------- -----

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:

the code is unchanged and is in function fe-connect.c::PQconnectPoll
().


-----

Florian Hars wrote:

Would someone comment on this bug report from February? I can confirm -------------------------------------------------------------------- --

The following bug has been logged online:

Bug reference:      1467
Logged by:          Florian Hars
Email address:      [EMAIL PROTECTED]
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
[EMAIL PROTECTED]
|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-

--
  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
[EMAIL PROTECTED]
|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-|-


---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster