Our select() function is emulated using poll(), which is a sensible thing

nyh · nyh · commit b53d39ac7bf9 · 2013-09-23T08:32:10.000+03:00
to do. However, it did several things wrong that this patch fixes. Thanks to Paolo Bonzini for finding these problems (see issue #35). 1. When poll() returned a bare POLLHUP, without POLLIN, our select() didn't return a read event. But nothing in the manpages guarantees that POLLHUP is accompanied by POLLIN, and some special file implementations might forget it. As an example, in Linux POLLHUP without POLLIN is common. But POLLHUP on its own already means that there's nothing more to read, so a read() will return immediately without blocking - and therefore select() needs to turn on the readable bit for this fd. 2. Similarly, a bare POLLRDHUP should turn on the writable bit: The reader on this file hug up, so a write will fail immediately. 3. Our poll() and select() confused what POLLERR means. POLLERR does not mean poll() found a bad file descriptor - there is POLLNVAL for that. So this patch fixes poll() to set POLLNVAL, not POLLERR, and select() to return with errno=EBADF when it sees POLLNVAL, not POLLERR. 4. Rather, POLLERR means the file descriptor is in an error state, so every read() or write() will return immediately (with an error). So when we see it, we need to turn on both read and write bits in this case. 5. The meaning of "exceptfds" isn't clear in any manual page, and it seems there're a lot of opinions on what it might mean. In this patch I did what Paolo suggested, which is to set the except bit when POLLPRI. (I don't set exceptfds on POLLERR, or any other case). Signed-off-by: Nadav Har'El <nyh@cloudius-systems.com>
diff --git a/core/poll.cc b/core/poll.cc
@@ -121,7 +121,7 @@ int poll_scan(struct pollfd _pfd[], nfds_t _nfds)
 
         error = fget(entry->fd, &fp);
         if (error) {
-            entry->revents |= POLLERR;
+            entry->revents |= POLLNVAL;
             nr_events++;
             continue;
         }
diff --git a/core/select.c b/core/select.c
@@ -115,30 +115,35 @@ int select (int nfds,
     for (i=0; i < poll_fd_idx; i++) {
         if (req[i].revents == 0)
             continue;
-
+        if (req[i].revents & POLLNVAL) {
+            errno = EBADF;
+            error = -1;
+            break;
+        }
+        bool event = false;
         if (readfds) {
-            if (req[i].revents & POLLIN) {
+            if (req[i].revents & (POLLIN | POLLHUP | POLLERR)) {
                 FD_SET(req[i].fd, readfds);
-                num_fds++;
+                event = true;
             }
         }
 
         if (writefds) {
-            if (req[i].revents & POLLOUT) {
+            if (req[i].revents & (POLLOUT | POLLRDHUP | POLLERR)) {
                 FD_SET(req[i].fd, writefds);
-                num_fds++;
+                event = true;
             }
         }
 
-        if (req[i].revents & POLLERR) {
-            if (exceptfds) {
+        if (exceptfds) {
+            if (req[i].revents & POLLPRI) {
                 FD_SET(req[i].fd, exceptfds);
-                num_fds++;
+                event = true;
             }
+        }
 
-            errno = EBADF;
-            error = -1;
-            break;
+        if (event) {
+            num_fds++;
         }
     }
 

Original file line number	Diff line number	Diff line change
`@@ -121,7 +121,7 @@ int poll_scan(struct pollfd _pfd[], nfds_t _nfds)`
`121`	`121`
`122`	`122`	`error = fget(entry->fd, &fp);`
`123`	`123`	`if (error) {`
`124`		`- entry->revents \|= POLLERR;`
	`124`	`+ entry->revents \|= POLLNVAL;`
`125`	`125`	`nr_events++;`
`126`	`126`	`continue;`
`127`	`127`	`}`