Accept SOCK_NONBLOCK/SOCK_CLOEXEC in type argument of socketpair (#1567)

Setting `SOCK_NONBLOCK` and `SOCK_CLOEXEC` in the `type` argument of `socketpair()` is widely supported across UNIX-like OS: Linux, *BSD, Solaris, etc., as is the `socket()`. This will conserve several extra system calls, we should use it where available.

### References

- [socketpair(2) on Linux](https://man7.org/linux/man-pages/man2/socketpair.2.html#HISTORY)
- [socketpair(2) on FreeBSD](https://man.freebsd.org/cgi/man.cgi?query=socketpair&sektion=2#DESCRIPTION)
- [socketpair(2) on DragonFly](https://man.dragonflybsd.org/?command=socketpair&section=2)
- [socketpair(2) on NetBSD](https://man.netbsd.org/socketpair.2#DESCRIPTION)
- [socketpair(2) on OpenBSD](https://man.openbsd.org/socketpair.2)
- [socketpair(3C) on Solaris](https://docs.oracle.com/cd/E88353_01/html/E37843/socketpair-3c.html)

Changelog:
- Set SOCK_NONBLOCK and SOCK_CLOEXEC in the type argument of socketpair
- Avoid EPROTOTYPE on macOS and OpenBSD
- Eliminate the warnings about unused variables
- Add some comments

evutil.c

@@ -403,125 +403,6 @@ evutil_win_socketpair(int family, int type, int protocol,
 }
 #endif
 
-int
-evutil_socketpair(int family, int type, int protocol, evutil_socket_t fd[2])
-{
-#if defined(_WIN32)
-	return evutil_win_socketpair(family, type, protocol, fd);
-#elif defined(EVENT__HAVE_SOCKETPAIR)
-	return socketpair(family, type, protocol, fd);
-#else
-	return evutil_ersatz_socketpair_(family, type, protocol, fd);
-#endif
-}
-
-int
-evutil_ersatz_socketpair_(int family, int type, int protocol,
-    evutil_socket_t fd[2])
-{
-	/* This code is originally from Tor.  Used with permission. */
-
-	/* This socketpair does not work when localhost is down. So
-	 * it's really not the same thing at all. But it's close enough
-	 * for now, and really, when localhost is down sometimes, we
-	 * have other problems too.
-	 */
-#undef ERR
-#ifdef _WIN32
-#define ERR(e) WSA##e
-#else
-#define ERR(e) e
-#endif
-	evutil_socket_t listener = -1;
-	evutil_socket_t connector = -1;
-	evutil_socket_t acceptor = -1;
-	struct sockaddr_in listen_addr;
-	struct sockaddr_in connect_addr;
-	ev_socklen_t size;
-	int saved_errno = -1;
-	int family_test;
-
-	family_test = family != AF_INET;
-#ifdef AF_UNIX
-	family_test = family_test && (family != AF_UNIX);
-#endif
-	if (protocol || family_test) {
-		EVUTIL_SET_SOCKET_ERROR(ERR(EAFNOSUPPORT));
-		return -1;
-	}
-
-	if (!fd) {
-		EVUTIL_SET_SOCKET_ERROR(ERR(EINVAL));
-		return -1;
-	}
-
-	listener = socket(AF_INET, type, 0);
-	if (listener < 0)
-		return -1;
-	memset(&listen_addr, 0, sizeof(listen_addr));
-	listen_addr.sin_family = AF_INET;
-	listen_addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
-	listen_addr.sin_port = 0;	/* kernel chooses port.	 */
-	if (bind(listener, (struct sockaddr *) &listen_addr, sizeof (listen_addr))
-		== -1)
-		goto tidy_up_and_fail;
-	if (listen(listener, 1) == -1)
-		goto tidy_up_and_fail;
-
-	connector = socket(AF_INET, type, 0);
-	if (connector < 0)
-		goto tidy_up_and_fail;
-
-	memset(&connect_addr, 0, sizeof(connect_addr));
-
-	/* We want to find out the port number to connect to.  */
-	size = sizeof(connect_addr);
-	if (getsockname(listener, (struct sockaddr *) &connect_addr, &size) == -1)
-		goto tidy_up_and_fail;
-	if (size != sizeof (connect_addr))
-		goto abort_tidy_up_and_fail;
-	if (connect(connector, (struct sockaddr *) &connect_addr,
-				sizeof(connect_addr)) == -1)
-		goto tidy_up_and_fail;
-
-	size = sizeof(listen_addr);
-	acceptor = accept(listener, (struct sockaddr *) &listen_addr, &size);
-	if (acceptor < 0)
-		goto tidy_up_and_fail;
-	if (size != sizeof(listen_addr))
-		goto abort_tidy_up_and_fail;
-	/* Now check we are talking to ourself by matching port and host on the
-	   two sockets.	 */
-	if (getsockname(connector, (struct sockaddr *) &connect_addr, &size) == -1)
-		goto tidy_up_and_fail;
-	if (size != sizeof (connect_addr)
-		|| listen_addr.sin_family != connect_addr.sin_family
-		|| listen_addr.sin_addr.s_addr != connect_addr.sin_addr.s_addr
-		|| listen_addr.sin_port != connect_addr.sin_port)
-		goto abort_tidy_up_and_fail;
-	evutil_closesocket(listener);
-	fd[0] = connector;
-	fd[1] = acceptor;
-
-	return 0;
-
- abort_tidy_up_and_fail:
-	saved_errno = ERR(ECONNABORTED);
- tidy_up_and_fail:
-	if (saved_errno < 0)
-		saved_errno = EVUTIL_SOCKET_ERROR();
-	if (listener != -1)
-		evutil_closesocket(listener);
-	if (connector != -1)
-		evutil_closesocket(connector);
-	if (acceptor != -1)
-		evutil_closesocket(acceptor);
-
-	EVUTIL_SET_SOCKET_ERROR(saved_errno);
-	return -1;
-#undef ERR
-}
-
 int
 evutil_make_socket_nonblocking(evutil_socket_t fd)
 {
@@ -2891,6 +2772,176 @@ evutil_socket_(int domain, int type, int protocol)
 	return r;
 }
 
+int
+evutil_socketpair(int family, int type, int protocol, evutil_socket_t fd[2])
+{
+	int ret = 0;
+	int sock_type = type;
+	(void) sock_type;
+	/* SOCK_NONBLOCK and SOCK_CLOEXEC are UNIX-specific. Therefore, the predefined and
+	 * platform-independent macros EVUTIL_SOCK_NONBLOCK and EVUTIL_SOCK_CLOEXEC are used
+	 * in type argument as combination while SOCK_NONBLOCK and SOCK_CLOEXEC are used for
+	 * distinguishing platforms.
+	 */
+#ifndef SOCK_NONBLOCK
+	type &= ~EVUTIL_SOCK_NONBLOCK;
+#endif
+#ifndef SOCK_CLOEXEC
+	type &= ~EVUTIL_SOCK_CLOEXEC;
+#endif
+#if defined(_WIN32)
+	ret = evutil_win_socketpair(family, type, protocol, fd);
+#elif defined(EVENT__HAVE_SOCKETPAIR)
+	ret = socketpair(family, type, protocol, fd);
+#else
+	ret = evutil_ersatz_socketpair_(family, type, protocol, fd);
+#endif
+	if (ret)
+		return ret;
+#ifndef SOCK_NONBLOCK
+	if (sock_type & EVUTIL_SOCK_NONBLOCK) {
+		if ((ret = evutil_fast_socket_nonblocking(fd[0]))) {
+			evutil_closesocket(fd[0]);
+			evutil_closesocket(fd[1]);
+			return ret;
+		}
+		if ((ret = evutil_fast_socket_nonblocking(fd[1]))) {
+			evutil_closesocket(fd[0]);
+			evutil_closesocket(fd[1]);
+			return ret;
+		}
+	}
+#endif
+#ifndef SOCK_CLOEXEC
+	if (sock_type & EVUTIL_SOCK_CLOEXEC) {
+		if ((ret = evutil_fast_socket_closeonexec(fd[0]))) {
+			evutil_closesocket(fd[0]);
+			evutil_closesocket(fd[1]);
+			return ret;
+		}
+		if ((ret = evutil_fast_socket_closeonexec(fd[1]))) {
+			evutil_closesocket(fd[0]);
+			evutil_closesocket(fd[1]);
+			return ret;
+		}
+	}
+#endif
+	return ret;
+}
+
+int
+evutil_ersatz_socketpair_(int family, int type, int protocol,
+    evutil_socket_t fd[2])
+{
+	/* This code is originally from Tor.  Used with permission. */
+
+	/* This socketpair does not work when localhost is down. So
+	 * it's really not the same thing at all. But it's close enough
+	 * for now, and really, when localhost is down sometimes, we
+	 * have other problems too.
+	 */
+#undef ERR
+#ifdef _WIN32
+#define ERR(e) WSA##e
+#else
+#define ERR(e) e
+#endif
+	evutil_socket_t listener = -1;
+	evutil_socket_t connector = -1;
+	evutil_socket_t acceptor = -1;
+	struct sockaddr_in listen_addr;
+	struct sockaddr_in connect_addr;
+	ev_socklen_t size;
+	int saved_errno = -1;
+	int family_test;
+
+	family_test = family != AF_INET;
+#ifdef AF_UNIX
+	family_test = family_test && (family != AF_UNIX);
+#endif
+	if (protocol || family_test) {
+		EVUTIL_SET_SOCKET_ERROR(ERR(EAFNOSUPPORT));
+		return -1;
+	}
+
+	if (!fd) {
+		EVUTIL_SET_SOCKET_ERROR(ERR(EINVAL));
+		return -1;
+	}
+
+	listener = socket(AF_INET, type, 0);
+	if (listener < 0)
+		return -1;
+	memset(&listen_addr, 0, sizeof(listen_addr));
+	listen_addr.sin_family = AF_INET;
+	listen_addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
+	listen_addr.sin_port = 0;	/* kernel chooses port.	 */
+	if (bind(listener, (struct sockaddr *) &listen_addr, sizeof (listen_addr))
+		== -1)
+		goto tidy_up_and_fail;
+	if (listen(listener, 1) == -1)
+		goto tidy_up_and_fail;
+
+	connector = socket(AF_INET, type, 0);
+	if (connector < 0)
+		goto tidy_up_and_fail;
+
+	memset(&connect_addr, 0, sizeof(connect_addr));
+
+	/* We want to find out the port number to connect to.  */
+	size = sizeof(connect_addr);
+	if (getsockname(listener, (struct sockaddr *) &connect_addr, &size) == -1)
+		goto tidy_up_and_fail;
+	if (size != sizeof(connect_addr))
+		goto abort_tidy_up_and_fail;
+	if (connect(connector, (struct sockaddr *) &connect_addr,
+				sizeof(connect_addr)) == -1) {
+		/* It's OK for a non-blocking socket to get an EINPROGRESS from connect(). */
+		int err = evutil_socket_geterror(connector);
+		if (!(EVUTIL_ERR_CONNECT_RETRIABLE(err) && type & EVUTIL_SOCK_NONBLOCK))
+			goto tidy_up_and_fail;
+	}
+
+	size = sizeof(listen_addr);
+	do {
+		acceptor = accept(listener, (struct sockaddr *) &listen_addr, &size);
+	} while(acceptor < 0 && EVUTIL_ERR_ACCEPT_RETRIABLE(errno) && type & EVUTIL_SOCK_NONBLOCK);
+	if (acceptor < 0)
+		goto tidy_up_and_fail;
+	if (size != sizeof(listen_addr))
+		goto abort_tidy_up_and_fail;
+	/* Now check we are talking to ourself by matching port and host on the
+	   two sockets.	 */
+	if (getsockname(connector, (struct sockaddr *) &connect_addr, &size) == -1)
+		goto tidy_up_and_fail;
+	if (size != sizeof (connect_addr)
+		|| listen_addr.sin_family != connect_addr.sin_family
+		|| listen_addr.sin_addr.s_addr != connect_addr.sin_addr.s_addr
+		|| listen_addr.sin_port != connect_addr.sin_port)
+		goto abort_tidy_up_and_fail;
+	evutil_closesocket(listener);
+	fd[0] = connector;
+	fd[1] = acceptor;
+
+	return 0;
+
+ abort_tidy_up_and_fail:
+	saved_errno = ERR(ECONNABORTED);
+ tidy_up_and_fail:
+	if (saved_errno < 0)
+		saved_errno = EVUTIL_SOCKET_ERROR();
+	if (listener != -1)
+		evutil_closesocket(listener);
+	if (connector != -1)
+		evutil_closesocket(connector);
+	if (acceptor != -1)
+		evutil_closesocket(acceptor);
+
+	EVUTIL_SET_SOCKET_ERROR(saved_errno);
+	return -1;
+#undef ERR
+}
+
 /* Internal wrapper around 'accept' or 'accept4' to provide Linux-style
  * support for syscall-saving methods where available.
  *
@@ -2976,20 +3027,11 @@ evutil_make_internal_pipe_(evutil_socket_t fd[2])
 #else
 #define LOCAL_SOCKETPAIR_AF AF_UNIX
 #endif
-	if (evutil_socketpair(LOCAL_SOCKETPAIR_AF, SOCK_STREAM, 0, fd) == 0) {
+	if (evutil_socketpair(LOCAL_SOCKETPAIR_AF, SOCK_STREAM|EVUTIL_SOCK_CLOEXEC|EVUTIL_SOCK_NONBLOCK, 0, fd)) {
-		if (evutil_fast_socket_nonblocking(fd[0]) < 0 ||
+		fd[0] = fd[1] = -1;
-		    evutil_fast_socket_nonblocking(fd[1]) < 0 ||
+		return -1;
-		    evutil_fast_socket_closeonexec(fd[0]) < 0 ||
-		    evutil_fast_socket_closeonexec(fd[1]) < 0) {
-			evutil_closesocket(fd[0]);
-			evutil_closesocket(fd[1]);
-			fd[0] = fd[1] = -1;
-			return -1;
-		}
-		return 0;
 	}
-	fd[0] = fd[1] = -1;
+	return 0;
-	return -1;
 }
 
 /* Wrapper around eventfd on systems that provide it.  Unlike the system

include/event2/util.h

@@ -377,9 +377,14 @@ int evutil_gettime_monotonic(struct evutil_monotonic_timer *timer,
 
 /** Create two new sockets that are connected to each other.
 
-    On Unix, this simply calls socketpair().  On Windows, it uses the
+    On Unix, this simply calls socketpair() and creates an unnamed pair of connected sockets
-    loopback network interface on 127.0.0.1, and only
+    in the specified domain, of the specified type, and using the optionally specified protocol.
-    AF_INET,SOCK_STREAM are supported.
+
+    On Windows, it will try to use the AF_UNIX to create unix socket pair if available, otherwise
+    it instead uses AF_INET to create socket pair, binding the loopback network interface 127.0.0.1.
+
+    Including the bitwise OR of the EVUTIL_SOCK_NONBLOCK and/or EVUTIL_SOCK_CLOEXEC
+    in the type argument will apply to both file descriptors.
 
     (This may fail on some Windows hosts where firewall software has cleverly
     decided to keep 127.0.0.1 from talking to itself.)
@@ -387,7 +392,7 @@ int evutil_gettime_monotonic(struct evutil_monotonic_timer *timer,
     Parameters and return values are as for socketpair()
 */
 EVENT2_EXPORT_SYMBOL
-int evutil_socketpair(int d, int type, int protocol, evutil_socket_t sv[2]);
+int evutil_socketpair(int domain, int type, int protocol, evutil_socket_t sv[2]);
 /** Do platform-specific operations as needed to make a socket nonblocking.
 
     @param sock The socket to make nonblocking

test/regress_buffer.c

@@ -1237,14 +1237,12 @@ test_evbuffer_add_file(void *ptr)
 #if defined(EVENT__HAVE_SENDFILE) && defined(__sun__) && defined(__svr4__)
 	/* We need to use a pair of AF_INET sockets, since Solaris
 	   doesn't support sendfile() over AF_UNIX. */
-	if (evutil_ersatz_socketpair_(AF_INET, SOCK_STREAM, 0, pair) == -1)
+	if (evutil_ersatz_socketpair_(AF_INET, SOCK_STREAM|EVUTIL_SOCK_NONBLOCK, 0, pair) == -1)
 		tt_abort_msg("ersatz_socketpair failed");
 #else
-	if (evutil_socketpair(AF_UNIX, SOCK_STREAM, 0, pair) == -1)
+	if (evutil_socketpair(AF_UNIX, SOCK_STREAM|EVUTIL_SOCK_NONBLOCK, 0, pair) == -1)
 		tt_abort_msg("socketpair failed");
 #endif
-	evutil_make_socket_nonblocking(pair[0]);
-	evutil_make_socket_nonblocking(pair[1]);
 
 	tt_assert(fd != -1);
 

test/regress_main.c

@@ -286,20 +286,10 @@ basic_test_setup(const struct testcase_t *testcase)
 	}
 
 	if (testcase->flags & TT_NEED_SOCKETPAIR) {
-		if (evutil_socketpair(AF_UNIX, SOCK_STREAM, 0, spair) == -1) {
+		if (evutil_socketpair(AF_UNIX, SOCK_STREAM|EVUTIL_SOCK_NONBLOCK, 0, spair) == -1) {
 			fprintf(stderr, "%s: socketpair\n", __func__);
 			exit(1);
 		}
-
-		if (evutil_make_socket_nonblocking(spair[0]) == -1) {
-			fprintf(stderr, "fcntl(O_NONBLOCK)");
-			exit(1);
-		}
-
-		if (evutil_make_socket_nonblocking(spair[1]) == -1) {
-			fprintf(stderr, "fcntl(O_NONBLOCK)");
-			exit(1);
-		}
 	}
 	if (testcase->flags & TT_NEED_BASE) {
 		if (testcase->flags & TT_LEGACY)

test/regress_zlib.c

@@ -49,6 +49,8 @@
 #include <assert.h>
 #include <errno.h>
 
+#include "util-internal.h"
+
 #include "event2/util.h"
 #include "event2/event.h"
 #include "event2/event_compat.h"
@@ -286,13 +288,10 @@ test_bufferevent_zlib(void *arg)
 	infilter_calls = outfilter_calls = readcb_finished = writecb_finished
 	    = errorcb_invoked = 0;
 
-	if (evutil_socketpair(AF_UNIX, SOCK_STREAM, 0, pair) == -1) {
+	if (evutil_socketpair(AF_UNIX, SOCK_STREAM|EVUTIL_SOCK_NONBLOCK, 0, pair) == -1) {
 		tt_abort_perror("socketpair");
 	}
 
-	evutil_make_socket_nonblocking(pair[0]);
-	evutil_make_socket_nonblocking(pair[1]);
-
 	bev1 = bufferevent_socket_new(NULL, pair[0], 0);
 	bev2 = bufferevent_socket_new(NULL, pair[1], 0);