[PATCH v2] [libpq] Try to avoid manually masking SIGPIPEs on every send()

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 <jk(at)ozlabs(dot)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 */