Skip to contentSkip to conversation

Fix select loop to handle handler deletions

PATCH New Anthony Liguori 1 participant 1 message Jul 24, 2006

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Fix select loop to handle handler deletions

The current select loop can SEGV if a handler removes itself. The following patch changes the select loop to be safe no matter how the io handler list is changed.

It definitely changes the complexity of the dispatch but since there are usually so few fds, I don't think it really matters.

Regards,

Anthony Liguori

# HG changeset patch
# User Anthony Liguori <[email protected]>
# Node ID 2b8742c4699e6726de3fde00831a3b1248fe1a79
# Parent 14d871ce5717d63e4bea98bca85653aa1cecdcba
Fix dispatch loop. If a callback removes itself, it can lead to a SEGV.

vl.c +26 -12
diff -r 14d871ce5717 -r 2b8742c4699e vl.c
@@ -4942,14 +4942,26 @@ void qemu_system_powerdown_request(void)
         cpu_interrupt(cpu_single_env, CPU_INTERRUPT_EXIT);
 }
 
+static IOHandlerRecord *find_io_handler(int fd)
+{
+    IOHandlerRecord *ioh;
+
+    for(ioh = first_io_handler; ioh != NULL; ioh = ioh->next) {
+        if (ioh->fd == fd)
+	    break;
+    }
+
+    return ioh;
+}
+
 void main_loop_wait(int timeout)
 {
-    IOHandlerRecord *ioh, *ioh_next;
+    IOHandlerRecord *ioh;
     fd_set rfds, wfds, xfds;
     int ret, nfds;
     struct timeval tv;
     PollingEntry *pe;
-
+    int i;
 
     /* XXX: need to suppress polling by better using win32 events */
     ret = 0;
@@ -5006,16 +5018,18 @@ void main_loop_wait(int timeout)
 #endif
     ret = select(nfds + 1, &rfds, &wfds, &xfds, &tv);
     if (ret > 0) {
-        /* XXX: better handling of removal */
-        for(ioh = first_io_handler; ioh != NULL; ioh = ioh_next) {
-            ioh_next = ioh->next;
-            if (FD_ISSET(ioh->fd, &rfds)) {
-                ioh->fd_read(ioh->opaque);
-            }
-            if (FD_ISSET(ioh->fd, &wfds)) {
-                ioh->fd_write(ioh->opaque);
-            }
-        }
+	for (i = 0; i < nfds + 1; i++) {
+	    if (FD_ISSET(i, &rfds)) {
+		ioh = find_io_handler(i);
+		if (ioh && ioh->fd_read)
+		    ioh->fd_read(ioh->opaque);
+	    }
+	    if (FD_ISSET(i, &wfds)) {
+		ioh = find_io_handler(i);
+		if (ioh && ioh->fd_write)
+		    ioh->fd_write(ioh->opaque);
+	    }
+	}
     }
 #if defined(CONFIG_SLIRP)
     if (slirp_inited) {