Browse Source

io: handle errors on listening file descriptors.

While investigating the previous patch, a bug caused poll to return
POLLHUP on the listening socket, which caused us to spin.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rusty Russell 9 years ago
parent
commit
f08b8139fc
3 changed files with 69 additions and 2 deletions
  1. 2 1
      ccan/io/io.h
  2. 6 1
      ccan/io/poll.c
  3. 61 0
      ccan/io/test/run-22-POLLHUP-on-listening-socket.c

+ 2 - 1
ccan/io/io.h

@@ -109,7 +109,8 @@ void io_set_finish_(struct io_conn *conn,
  * @arg: the argument to @init.
  * @arg: the argument to @init.
  *
  *
  * When @fd becomes readable, we accept(), create a new connection,
  * When @fd becomes readable, we accept(), create a new connection,
- * (tal'ocated off @ctx) and pass that to init().
+ * (tal'ocated off @ctx) and pass that to init().  Note that if there is
+ * an error on this file descriptor, it will be freed.
  *
  *
  * Returns NULL on error (and sets errno).
  * Returns NULL on error (and sets errno).
  *
  *

+ 6 - 1
ccan/io/poll.c

@@ -282,9 +282,14 @@ void *io_loop(struct timers *timers, struct timer **expired)
 				break;
 				break;
 
 
 			if (fds[i]->listener) {
 			if (fds[i]->listener) {
+				struct io_listener *l = (void *)fds[i];
 				if (events & POLLIN) {
 				if (events & POLLIN) {
-					accept_conn((void *)c);
+					accept_conn(l);
 					r--;
 					r--;
+				} else if (events & (POLLHUP|POLLNVAL|POLLERR)) {
+					r--;
+					errno = EBADF;
+					io_close_listener(l);
 				}
 				}
 			} else if (events & (POLLIN|POLLOUT)) {
 			} else if (events & (POLLIN|POLLOUT)) {
 				r--;
 				r--;

+ 61 - 0
ccan/io/test/run-22-POLLHUP-on-listening-socket.c

@@ -0,0 +1,61 @@
+#include <ccan/io/io.h>
+/* Include the C files directly. */
+#include <ccan/io/poll.c>
+#include <ccan/io/io.c>
+#include <ccan/tap/tap.h>
+#include <sys/wait.h>
+#include <stdio.h>
+
+#define PORT "65022"
+
+static int make_listen_fd(const char *port, struct addrinfo **info)
+{
+	int fd, on = 1;
+	struct addrinfo *addrinfo, hints;
+
+	memset(&hints, 0, sizeof(hints));
+	hints.ai_family = AF_UNSPEC;
+	hints.ai_socktype = SOCK_STREAM;
+	hints.ai_flags = AI_PASSIVE;
+	hints.ai_protocol = 0;
+
+	if (getaddrinfo(NULL, port, &hints, &addrinfo) != 0)
+		return -1;
+
+	fd = socket(addrinfo->ai_family, addrinfo->ai_socktype,
+		    addrinfo->ai_protocol);
+	if (fd < 0)
+		return -1;
+
+	setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on));
+	if (bind(fd, addrinfo->ai_addr, addrinfo->ai_addrlen) != 0) {
+		close(fd);
+		return -1;
+	}
+	if (listen(fd, 1) != 0) {
+		close(fd);
+		return -1;
+	}
+	*info = addrinfo;
+	return fd;
+}
+
+int main(void)
+{
+	struct addrinfo *addrinfo = NULL;
+	int fd;
+
+	/* This is how many tests you plan to run */
+	plan_tests(1);
+	fd = make_listen_fd(PORT, &addrinfo);
+	freeaddrinfo(addrinfo);
+	io_new_listener(NULL, fd, io_never, NULL);
+
+	/* Anyone could do this; a child doing it will cause poll to return
+	 * POLLHUP only! */
+	shutdown(fd, SHUT_RDWR);
+	ok1(io_loop(NULL, NULL) == NULL);
+
+	/* This exits depending on whether all tests passed */
+	return exit_status();
+}