Currently, libpq will wrap each send() call on the connection with
two system calls to mask SIGPIPEs. This results in 3 syscalls instead
of one, and (on Linux) can lead to high contention on the signal
mask locks in threaded apps.

We have a couple of other methods to avoid SIGPIPEs:
sockopt(SO_NOSIGPIPE) and the MSG_NOSIGNAL flag to send().

This change attempts to use these if they're available at compile-
and run-time. If not, we drop back to manipulating the signal mask as
before.

Signed-off-by: Jeremy Kerr <j...@ozlabs.org>

---
v2: initialise optval

---
 src/interfaces/libpq/fe-connect.c |   40 ++++++++++++++++++
 src/interfaces/libpq/fe-secure.c  |   83 +++++++++++++++++++++++++++++---------
 src/interfaces/libpq/libpq-int.h  |    2 
 3 files changed, 107 insertions(+), 18 deletions(-)

diff --git a/src/interfaces/libpq/fe-connect.c 
b/src/interfaces/libpq/fe-connect.c
index 7f4ae4c..92ab4e0 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -1085,6 +1085,7 @@ keep_going:                                               
/* We will come back to here until there is
                                while (conn->addr_cur != NULL)
                                {
                                        struct addrinfo *addr_cur = 
conn->addr_cur;
+                                       int optval;
 
                                        /* Remember current address for 
possible error msg */
                                        memcpy(&conn->raddr.addr, 
addr_cur->ai_addr,
@@ -1149,6 +1150,45 @@ keep_going:                                              
/* We will come back to here until there is
                                        }
 #endif   /* F_SETFD */
 
+                                       /* We have three methods of blocking 
sigpipe during
+                                        * send() calls to this socket:
+                                        *
+                                        *  - setsockopt(sock, SO_NOSIGPIPE)
+                                        *  - send(sock, ..., MSG_NOSIGNAL)
+                                        *  - setting the signal mask to 
SIG_IGN during send()
+                                        *
+                                        * The first two reduce the number of 
syscalls (for the
+                                        * third, we require three syscalls to 
implement a send()),
+                                        * so use them if they're available. 
Their availability is
+                                        * flagged in the following members of 
PGconn:
+                                        *
+                                        * conn->sigpipe_so             - we 
have set up SO_NOSIGPIPE
+                                        * conn->sigpipe_flag   - we're 
specifying MSG_NOSIGNAL
+                                        *
+                                        * If we can use SO_NOSIGPIPE, then set 
sigpipe_so here and
+                                        * we don't need to care about anything 
else. Otherwise,
+                                        * try MSG_NOSIGNAL by setting 
sigpipe_flag. If we get an
+                                        * error with MSG_NOSIGNAL, we clear 
the flag and revert
+                                        * to manual masking.
+                                        */
+                                       conn->sigpipe_so = false;
+#ifdef MSG_NOSIGNAL
+                                       conn->sigpipe_flag = true;
+#else /* !MSG_NOSIGNAL */
+                                       conn->sigpipe_flag = false;
+#endif /* MSG_NOSIGNAL */
+
+#ifdef SO_NOSIGPIPE
+                                       optval = 1;
+                                       if (!setsockopt(conn->sock, SOL_SOCKET, 
SO_NOSIGPIPE,
+                                                       (char *)&optval, 
sizeof(optval)))
+                                       {
+                                               conn->sigpipe_so = true;
+                                               conn->sigpipe_flag = false;
+                                       }
+#endif /* SO_NOSIGPIPE */
+
+
                                        /*
                                         * Start/make connection.  This should 
not block, since we
                                         * are in nonblock mode.  If it does, 
well, too bad.
diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index 13c97ac..949cd0f 100644
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -122,6 +122,18 @@ static long win32_ssl_create_mutex = 0;
  */
 
 #ifndef WIN32
+
+static inline int sigpipe_masked(PGconn *conn)
+{
+       /* If we're on an SSL connection, we can only use SO_NOSIGPIPE masking.
+        * Otherwise, we can handle SO_NOSIGPIPE or the MSG_NOSIGNAL flag */
+#ifdef USE_SSL
+       if (conn->ssl)
+               return conn->sigpipe_so;
+#endif
+       return conn->sigpipe_so || conn->sigpipe_flag;
+}
+
 #ifdef ENABLE_THREAD_SAFETY
 
 struct sigpipe_info {
@@ -130,8 +142,11 @@ struct sigpipe_info {
        bool            got_epipe;
 };
 
-static inline int disable_sigpipe(struct sigpipe_info *info)
+static inline int disable_sigpipe(PGconn *conn, struct sigpipe_info *info)
 {
+       if (sigpipe_masked(conn))
+               return 0;
+
        info->got_epipe = false;
        return pq_block_sigpipe(&info->oldsigmask, &info->sigpipe_pending) < 0;
 }
@@ -142,8 +157,11 @@ static inline void remember_epipe(struct sigpipe_info 
*info, bool cond)
                info->got_epipe = true;
 }
 
-static inline void restore_sigpipe(struct sigpipe_info *info)
+static inline void restore_sigpipe(PGconn *conn, struct sigpipe_info *info)
 {
+       if (sigpipe_masked(conn))
+               return;
+
        pq_reset_sigpipe(&info->oldsigmask, info->sigpipe_pending, 
info->got_epipe);
 }
 
@@ -153,9 +171,10 @@ struct sigpipe_info {
        pqsigfunc       oldhandler;
 };
 
-static inline int disable_sigpipe(struct sigpipe_info *info)
+static inline int disable_sigpipe(PGconn *conn, struct sigpipe_info *info)
 {
-       info->oldhandler = pqsignal(SIGPIPE, SIG_IGN);
+       if (!sigpipe_masked(conn))
+               info->oldhandler = pqsignal(SIGPIPE, SIG_IGN);
        return 0;
 }
 
@@ -163,18 +182,22 @@ static inline void remember_epipe(struct sigpipe_info 
*info, bool cond)
 {
 }
 
-static inline void restore_sigpipe(struct sigpipe_info *info)
+static inline void restore_sigpipe(PGconn *conn, struct sigpipe_info *info)
 {
-       pqsignal(SIGPIPE, info->oldhandler);
+       if (!sigpipe_masked(conn))
+               pqsignal(SIGPIPE, info->oldhandler);
 }
 
 #endif /* ENABLE_THREAD_SAFETY */
 #else  /* WIN32 */
 
 struct sigpipe_info { };
-static inline int disable_sigpipe(struct sigpipe_info *info) { return 0; }
+static inline int disable_sigpipe(PGConn *conn, struct sigpipe_info *info)
+{
+       return 0;
+}
 static inline void remember_epipe(struct sigpipe_info *info, bool cond) { }
-static inline void restore_sigpipe(struct sigpipe_info *info) { }
+static inline void restore_sigpipe(PGConn *conn, struct sigpipe_info *info) { }
 
 #endif /* WIN32 */
 
@@ -306,7 +329,7 @@ pqsecure_read(PGconn *conn, void *ptr, size_t len)
                struct sigpipe_info info;
 
                /* SSL_read can write to the socket, so we need to disable 
SIGPIPE */
-               if (disable_sigpipe(&info))
+               if (disable_sigpipe(conn, &info))
                        return -1;
 
 rloop:
@@ -370,7 +393,7 @@ rloop:
                                break;
                }
 
-               restore_sigpipe(&info);
+               restore_sigpipe(conn, &info);
        }
        else
 #endif
@@ -388,14 +411,14 @@ pqsecure_write(PGconn *conn, const void *ptr, size_t len)
        ssize_t         n;
        struct sigpipe_info info;
 
-       if (disable_sigpipe(&info))
-               return -1;
-
 #ifdef USE_SSL
        if (conn->ssl)
        {
                int                     err;
 
+               if (disable_sigpipe(conn, &info))
+                       return -1;
+
                n = SSL_write(conn->ssl, ptr, len);
                err = SSL_get_error(conn->ssl, n);
                switch (err)
@@ -458,11 +481,35 @@ pqsecure_write(PGconn *conn, const void *ptr, size_t len)
        else
 #endif
        {
-               n = send(conn->sock, ptr, len, 0);
-               remember_epipe(&info, n < 0 && SOCK_ERRNO == EPIPE);
+               int flags = 0;
+
+#ifdef MSG_NOSIGNAL
+               if (!conn->sigpipe_so && conn->sigpipe_flag)
+                       flags |= MSG_NOSIGNAL;
+#endif /* MSG_NOSIGNAL */
+
+retry_masked:
+               if (disable_sigpipe(conn, &info))
+                       return -1;
+
+               n = send(conn->sock, ptr, len, flags);
+
+               if (n < 0) {
+                       /* if we see an EINVAL, it may be because MSG_NOSIGNAL 
isn't
+                        * available on this machine. So, clear sigpipe_flag so 
we don't
+                        * try this flag again, and retry the send().
+                        */
+                       if (flags != 0 && SOCK_ERRNO == EINVAL) {
+                               conn->sigpipe_flag = false;
+                               flags = 0;
+                               goto retry_masked;
+                       }
+
+                       remember_epipe(&info, SOCK_ERRNO == EPIPE);
+               }
        }
 
-       restore_sigpipe(&info);
+       restore_sigpipe(conn, &info);
 
        return n;
 }
@@ -1219,14 +1266,14 @@ close_SSL(PGconn *conn)
        if (conn->ssl)
        {
                struct sigpipe_info info;
-               disable_sigpipe(&info);
+               disable_sigpipe(conn, &info);
                SSL_shutdown(conn->ssl);
                SSL_free(conn->ssl);
                conn->ssl = NULL;
                pqsecure_destroy();
                /* We have to assume we got EPIPE */
                remember_epipe(&info, true);
-               restore_sigpipe(&info);
+               restore_sigpipe(conn, &info);
        }
 
        if (conn->peer)
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 2340347..71dfefc 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -336,6 +336,8 @@ struct pg_conn
        ProtocolVersion pversion;       /* FE/BE protocol version in use */
        int                     sversion;               /* server version, e.g. 
70401 for 7.4.1 */
        bool            password_needed;        /* true if server demanded a 
password */
+       bool            sigpipe_so;             /* have we masked sigpipes via 
SO_NOSIGPIPE? */
+       bool            sigpipe_flag;   /* can we mask sigpipes via 
MSG_NOSIGNAL? */
 
        /* Transient state needed while establishing connection */
        struct addrinfo *addrlist;      /* list of possible backend addresses */

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to