Changeset 1895c29

Timestamp:

Dec 17, 2008, 5:13:20 PM (15 years ago)

Author:

Nelson Elhage <nelhage@mit.edu>

Branches:

master, debian, release-1.10, release-1.4, release-1.5, release-1.6, release-1.7, release-1.8, release-1.9

Children:

cf0cc64

Parents:

f36cd97

git-author:

Nelson Elhage <nelhage@mit.edu> (12/17/08 16:58:41)

git-committer:

Nelson Elhage <nelhage@mit.edu> (12/17/08 17:13:20)

Message:

Fix owl_select_remove_dispatch bugs.

Fix two bugs in owl_select_remove_dispatch:

(1) Removing a dispatch from that dispatch's callback was bad, because
    it mutated the dispatch list while it was being traversed. Fix
    that by noting if we're in the middle of a dispatch, and deferring
    cleanup until later.
(2) Calling owl_free on dispatches in select.c is a strange API, since
    it doesn't allocate them in the first place. Since we now have
    destructors, make freeing the dispatch their job.

Files:

• owl.h (modified) (1 diff)
• perlconfig.c (modified) (1 diff)
• select.c (modified) (9 diffs)

owl.h

@@ -527 +527 @@
 typedef struct _owl_dispatch {
   int fd;                                 /* FD to watch for dispatch. */
+  int needs_gc;
   void (*cfunc)(struct _owl_dispatch*);   /* C function to dispatch to. */
   void (*destroy)(struct _owl_dispatch*); /* Destructor */

perlconfig.c

@@ -447 +447 @@
 {
   SvREFCNT_dec(d->data);
+  owl_free(d);
 }
 

select.c

@@ -2 +2 @@
 
 static const char fileIdent[] = "$Id: select.c 894 2008-01-17 07:13:44Z asedeno $";
+
+static int dispatch_active = 0;
 
 int _owl_select_timer_cmp(owl_timer *t1, owl_timer *t2) {
@@ -96 +98 @@
 }
 
+void owl_select_remove_dispatch_at(int elt) /* noproto */
+{
+  owl_list *dl;
+  owl_dispatch *d;
+
+  dl = owl_global_get_dispatchlist(&g);
+  d = (owl_dispatch*)owl_list_get_element(dl, elt);
+  owl_list_remove_element(dl, elt);
+  if (d->destroy) {
+    d->destroy(d);
+  }
+}
+
 /* Adds a new owl_dispatch to the list, replacing existing ones if needed. */
 void owl_select_add_dispatch(owl_dispatch *d)
@@ -101 +116 @@
   int elt;
   owl_list *dl;
+
+  d->needs_gc = 0;
 
   elt = owl_select_find_dispatch(d->fd);
@@ -111 +128 @@
        replace the old dispatch. */
     if (d_old != d) {
-      owl_list_replace_element(dl, elt, d);
-      owl_free(d_old);
-    }
-  }
-  else {
-    owl_list_append_element(dl, d);
-  }
+      owl_select_remove_dispatch_at(elt);
+    }
+  }
+  owl_list_append_element(dl, d);
 }
 
@@ -125 +139 @@
   int elt;
   owl_list *dl;
+  owl_dispatch *d;
 
   elt = owl_select_find_dispatch(fd);
-  dl = owl_global_get_dispatchlist(&g);
-  
-  if (elt != -1) {
-    owl_dispatch *d;
+  if(elt == -1) {
+    return;
+  } else if(dispatch_active) {
+    /* Defer the removal until dispatch is done walking the list */
+    dl = owl_global_get_dispatchlist(&g);
     d = (owl_dispatch*)owl_list_get_element(dl, elt);
-    owl_list_remove_element(dl, elt);
-    if (d->destroy) {
-      d->destroy(d);
-    }
-    owl_free(d);
+    d->needs_gc = 1;
+  } else {
+    owl_select_remove_dispatch_at(elt);
   }
 }
@@ -176 +190 @@
     d = (owl_dispatch*)owl_list_get_element(owl_global_get_dispatchlist(&g), elt);
     if (d->cfunc == owl_perlconfig_dispatch) {
-      owl_select_remove_dispatch(fd);
+      owl_select_remove_dispatch_at(elt);
       return 0;
     }
@@ -203 +217 @@
 }
 
+void owl_select_gc()
+{
+  int i;
+  owl_list *dl;
+
+  dl = owl_global_get_dispatchlist(&g);
+  /*
+   * Count down so we aren't set off by removing items from the list
+   * during the iteration.
+   */
+  for(i = owl_list_get_size(dl) - 1; i >= 0; i--) {
+    owl_dispatch *d = owl_list_get_element(dl, i);
+    if(d->needs_gc) {
+      owl_select_remove_dispatch_at(i);
+    }
+  }
+}
+
 void owl_select_dispatch(fd_set *fds, int max_fd)
 {
@@ -211 +243 @@
   dl = owl_global_get_dispatchlist(&g);
   len = owl_select_dispatch_count();
+
+  dispatch_active = 1;
+
   for(i = 0; i < len; i++) {
     d = (owl_dispatch*)owl_list_get_element(dl, i);
     /* While d shouldn't normally be null, the list may be altered by
      * functions we dispatch to. */
-    if (d != NULL && FD_ISSET(d->fd, fds)) {
+    if (d != NULL && !d->needs_gc && FD_ISSET(d->fd, fds)) {
       if (d->cfunc != NULL) {
         d->cfunc(d);
@@ -221 +256 @@
     }
   }
+
+  dispatch_active = 0;
+  owl_select_gc();
 }