Fix SourceForge bug #2789732 by discarding pending events for deleted sockets.
authorMichael Poole <mdpoole@troilus.org>
Mon, 4 Jan 2010 03:24:36 +0000 (03:24 +0000)
committerMichael Poole <mdpoole@troilus.org>
Mon, 4 Jan 2010 03:24:36 +0000 (03:24 +0000)
git-svn-id: file:///home/klmitch/undernet-ircu/undernet-ircu-svn/ircu2/branches/u2_10_12_branch@1933 c9e4aea6-c8fd-4c43-8297-357d70d61c8c

ChangeLog
ircd/engine_devpoll.c
ircd/engine_epoll.c
ircd/engine_kqueue.c

index 20e3c8a11b76e4deddaff4ec92f8b3d0cb31abe6..27312376038381a62961d8f0857c611eb35e3797 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,20 @@
+2010-01-03  Michael Poole <mdpoole@troilus.org>
+
+       * ircd/engine_devpoll.c: Make some variables that were local to
+       engine_loop() file-scoped so engine_delete() can see them.
+       (engine_delete): Discard unprocessed events for the deleted
+       socket.
+       (engine_loop): Update to use the new variable names.  Change a
+       frequent debug statement to DEBUG_ENGINE.  Fix the type of the
+       codesize variable.
+
+       * ircd/engine_epoll.c: Same as ircd/engine_devpoll.c, but the
+       debug statement and codesize variable were already fixed.
+
+       * ircd/engine_kqueue.c: Same as ircd/engine_devpoll.c, but also
+       fix a typo in an engine_signal() assertion, and get rid of dead
+       variables in engine_delete().
+
 2010-01-03  Michael Poole <mdpoole@troilus.org>
 
        * ircd/s_bsd.c (client_sock_callback): Invalidate cli_fd() when we
index 9ef4dee0b36a77817243469d58d25dfc20defd8b..e4fd20042120a216556f95cef53bb6b860170a4a 100644 (file)
@@ -74,6 +74,12 @@ static int devpoll_fd;
 static int errors = 0;
 /** Periodic timer to forget errors. */
 static struct Timer clear_error;
+/** Array of currently polled file descriptors. */
+static struct pollfd *polls;
+/** Number of ::polls elements that have been populated. */
+static int polls_used;
+/** Current processing position in ::polls. */
+static int polls_i;
 
 /** Decrement the error count (once per hour).
  * @param[in] ev Expired timer event (ignored).
@@ -254,6 +260,8 @@ engine_events(struct Socket* sock, unsigned int new_events)
 static void
 engine_delete(struct Socket* sock)
 {
+  int ii;
+
   assert(0 != sock);
   assert(sock == sockList[s_fd(sock)]);
 
@@ -263,6 +271,13 @@ engine_delete(struct Socket* sock)
   set_events(sock, 0); /* get rid of the socket */
 
   sockList[s_fd(sock)] = 0; /* zero the socket list entry */
+
+  /* Drop any unprocessed events citing this socket. */
+  for (ii = polls_i; ii < polls_used; ii++) {
+    if (polls[ii].fd == s_fd(sock)) {
+      polls[ii] = polls[--polls_used];
+    }
+  }
 }
 
 /** Run engine event loop.
@@ -272,13 +287,13 @@ static void
 engine_loop(struct Generators* gen)
 {
   struct dvpoll dopoll;
-  struct pollfd *polls;
   int polls_count;
   struct Socket* sock;
+  struct pollfd *pfd;
   int nfds;
   int i;
   int errcode;
-  size_t codesize;
+  socklen_t codesize;
 
   if ((polls_count = feature_int(FEAT_POLLS_PER_LOOP)) < 20)
     polls_count = 20;
@@ -297,15 +312,15 @@ engine_loop(struct Generators* gen)
     dopoll.dp_timeout = timer_next(gen) ?
       (timer_next(gen) - CurrentTime) * 1000 : -1;
 
-    Debug((DEBUG_INFO, "devpoll: delay: %Tu (%Tu) %d", timer_next(gen),
+    Debug((DEBUG_ENGINE, "devpoll: delay: %Tu (%Tu) %d", timer_next(gen),
           CurrentTime, dopoll.dp_timeout));
 
     /* check for active files */
-    nfds = ioctl(devpoll_fd, DP_POLL, &dopoll);
+    polls_used = ioctl(devpoll_fd, DP_POLL, &dopoll);
 
     CurrentTime = time(0); /* set current time... */
 
-    if (nfds < 0) {
+    if (polls_used < 0) {
       if (errno != EINTR) { /* ignore interrupts */
        /* Log the poll error */
        log_write(LS_SOCKET, L_ERROR, 0, "ioctl(DP_POLL) error: %m");
@@ -321,14 +336,15 @@ engine_loop(struct Generators* gen)
       continue;
     }
 
-    for (i = 0; i < nfds; i++) {
-      assert(-1 < polls[i].fd);
+    for (polls_i = 0; polls_i < polls_used; polls_i++) {
+      pfd = &polls[polls_i];
+      assert(-1 < pfd->fd);
 
-      sock = sockList[polls[i].fd];
+      sock = sockList[pfd->fd];
       if (!sock) /* slots may become empty while processing events */
        continue;
 
-      assert(s_fd(sock) == polls[i].fd);
+      assert(s_fd(sock) == pfd->fd);
 
       gen_ref_inc(sock); /* can't have it going away on us */
 
@@ -352,10 +368,10 @@ engine_loop(struct Generators* gen)
        }
       }
 
-      assert(!(polls[i].revents & POLLERR));
+      assert(!(pfd->revents & POLLERR));
 
 #ifdef POLLHUP
-      if (polls[i].revents & POLLHUP) { /* hang-up on socket */
+      if (pfd->revents & POLLHUP) { /* hang-up on socket */
        Debug((DEBUG_ENGINE, "devpoll: EOF from client (POLLHUP)"));
        event_generate(ET_EOF, sock, 0);
        nfds--;
@@ -365,21 +381,21 @@ engine_loop(struct Generators* gen)
 
       switch (s_state(sock)) {
       case SS_CONNECTING:
-       if (polls[i].revents & POLLWRITEFLAGS) { /* connection completed */
+       if (pfd->revents & POLLWRITEFLAGS) { /* connection completed */
          Debug((DEBUG_ENGINE, "devpoll: Connection completed"));
          event_generate(ET_CONNECT, sock, 0);
        }
        break;
 
       case SS_LISTENING:
-       if (polls[i].revents & POLLREADFLAGS) { /* connect. to be accept. */
+       if (pfd->revents & POLLREADFLAGS) { /* connect. to be accept. */
          Debug((DEBUG_ENGINE, "devpoll: Ready for accept"));
          event_generate(ET_ACCEPT, sock, 0);
        }
        break;
 
       case SS_NOTSOCK:
-       if (polls[i].revents & POLLREADFLAGS) { /* data on socket */
+       if (pfd->revents & POLLREADFLAGS) { /* data on socket */
          /* can't peek; it's not a socket */
          Debug((DEBUG_ENGINE, "devpoll: non-socket readable"));
          event_generate(ET_READ, sock, 0);
@@ -387,7 +403,7 @@ engine_loop(struct Generators* gen)
        break;
 
       case SS_CONNECTED:
-       if (polls[i].revents & POLLREADFLAGS) { /* data on socket */
+       if (pfd->revents & POLLREADFLAGS) { /* data on socket */
          char c;
 
          switch (recv(s_fd(sock), &c, 1, MSG_PEEK)) { /* check EOF */
@@ -412,18 +428,18 @@ engine_loop(struct Generators* gen)
            break;
          }
        }
-       if (polls[i].revents & POLLWRITEFLAGS) { /* socket writable */
+       if (pfd->revents & POLLWRITEFLAGS) { /* socket writable */
          Debug((DEBUG_ENGINE, "devpoll: Data can be written"));
          event_generate(ET_WRITE, sock, 0);
        }
        break;
 
       case SS_DATAGRAM: case SS_CONNECTDG:
-       if (polls[i].revents & POLLREADFLAGS) { /* socket readable */
+       if (pfd->revents & POLLREADFLAGS) { /* socket readable */
          Debug((DEBUG_ENGINE, "devpoll: Datagram to be read"));
          event_generate(ET_READ, sock, 0);
        }
-       if (polls[i].revents & POLLWRITEFLAGS) { /* socket writable */
+       if (pfd->revents & POLLWRITEFLAGS) { /* socket writable */
          Debug((DEBUG_ENGINE, "devpoll: Datagram can be written"));
          event_generate(ET_WRITE, sock, 0);
        }
index 11281a2f46bd589776a21bb73e4933461822c529..080f42a9b1c4ef00bdc770e6ad0833e7934e7545 100644 (file)
@@ -104,6 +104,12 @@ static int epoll_fd;
 static int errors;
 /** Periodic timer to forget errors. */
 static struct Timer clear_error;
+/** Current array of event descriptors. */
+static struct epoll_event *events;
+/** Number of ::events elements that have been populated. */
+static int events_used;
+/** Current processing position in ::events. */
+static int events_i;
 
 /** Decrement the error count (once per hour).
  * @param[in] ev Expired timer event (ignored).
@@ -236,10 +242,17 @@ engine_set_events(struct Socket *sock, unsigned new_events)
 static void
 engine_delete(struct Socket *sock)
 {
+  int ii;
+
   assert(0 != sock);
   Debug((DEBUG_ENGINE, "epoll: Deleting socket %d [%p], state %s",
         s_fd(sock), sock, state_to_name(s_state(sock))));
-  /* No action necessary; epoll removes the socket on close(). */
+  /* Drop any unprocessed events citing this socket. */
+  for (ii = events_i; ii < events_used; ii++) {
+    if (events[ii].data.ptr == sock) {
+      events[ii] = events[--events_used];
+    }
+  }
 }
 
 /** Run engine event loop.
@@ -248,27 +261,27 @@ engine_delete(struct Socket *sock)
 static void
 engine_loop(struct Generators *gen)
 {
-  struct epoll_event *events;
+  struct epoll_event *evt;
   struct Socket *sock;
   socklen_t codesize;
-  int events_count, i, wait, nevs, errcode;
+  int events_count, tmp, wait, errcode;
 
   if ((events_count = feature_int(FEAT_POLLS_PER_LOOP)) < 20)
     events_count = 20;
   events = MyMalloc(sizeof(events[0]) * events_count);
   while (running) {
-    if ((i = feature_int(FEAT_POLLS_PER_LOOP)) >= 20 && i != events_count) {
-      events = MyRealloc(events, sizeof(events[0]) * i);
-      events_count = i;
+    if ((tmp = feature_int(FEAT_POLLS_PER_LOOP)) >= 20 && tmp != events_count) {
+      events = MyRealloc(events, sizeof(events[0]) * tmp);
+      events_count = tmp;
     }
 
     wait = timer_next(gen) ? (timer_next(gen) - CurrentTime) * 1000 : -1;
     Debug((DEBUG_ENGINE, "epoll: delay: %d (%d) %d", timer_next(gen),
            CurrentTime, wait));
-    nevs = epoll_wait(epoll_fd, events, events_count, wait);
+    events_used = epoll_wait(epoll_fd, events, events_count, wait);
     CurrentTime = time(0);
 
-    if (nevs < 0) {
+    if (events_used < 0) {
       if (errno != EINTR) {
         log_write(LS_SOCKET, L_ERROR, 0, "epoll() error: %m");
         if (!errors++)
@@ -280,8 +293,9 @@ engine_loop(struct Generators *gen)
       continue;
     }
 
-    for (i = 0; i < nevs; i++) {
-      if (!(sock = events[i].data.ptr))
+    for (events_i = 0; events_i < events_used; ) {
+      evt = &events[events_i++];
+      if (!(sock = evt->data.ptr))
         continue;
       gen_ref_inc(sock);
       Debug((DEBUG_ENGINE,
@@ -289,7 +303,7 @@ engine_loop(struct Generators *gen)
              sock, s_fd(sock), state_to_name(s_state(sock)),
              sock_flags(s_events(sock))));
 
-      if (events[i].events & EPOLLERR) {
+      if (evt->events & EPOLLERR) {
         errcode = 0;
         codesize = sizeof(errcode);
         if (getsockopt(s_fd(sock), SOL_SOCKET, SO_ERROR, &errcode,
@@ -300,16 +314,16 @@ engine_loop(struct Generators *gen)
           gen_ref_dec(sock);
           continue;
         }
-      } else if (events[i].events & EPOLLHUP) {
+      } else if (evt->events & EPOLLHUP) {
         event_generate(ET_EOF, sock, 0);
       } else switch (s_state(sock)) {
       case SS_CONNECTING:
-        if (events[i].events & EPOLLOUT) /* connection completed */
+        if (evt->events & EPOLLOUT) /* connection completed */
           event_generate(ET_CONNECT, sock, 0);
         break;
 
       case SS_LISTENING:
-        if (events[i].events & EPOLLIN) /* incoming connection */
+        if (evt->events & EPOLLIN) /* incoming connection */
           event_generate(ET_ACCEPT, sock, 0);
         break;
 
@@ -317,9 +331,9 @@ engine_loop(struct Generators *gen)
       case SS_CONNECTED:
       case SS_DATAGRAM:
       case SS_CONNECTDG:
-        if (events[i].events & EPOLLIN)
+        if (evt->events & EPOLLIN)
           event_generate(ET_READ, sock, 0);
-        if (events[i].events & EPOLLOUT)
+        if (evt->events & EPOLLOUT)
           event_generate(ET_WRITE, sock, 0);
         break;
       }
index 577cac01d57cd4785dbbe70a26f00b61fda9daa8..b3f535f7d8db52825068a42f959ac06d62cadd9b 100644 (file)
@@ -49,6 +49,12 @@ static struct Socket** sockList;
 static int kqueue_max;
 /** File descriptor for kqueue pseudo-file. */
 static int kqueue_id;
+/** Current array of event descriptors. */
+static struct kevent *events;
+/** Number of ::events elements that have been populated. */
+static int events_used;
+/** Current processing position in ::events. */
+static int events_i;
 
 /** Number of recent errors from kqueue. */
 static int errors = 0;
@@ -101,7 +107,7 @@ engine_signal(struct Signal* sig)
   struct kevent sigevent;
   struct sigaction act;
 
-  assert(0 != signal);
+  assert(0 != sig);
 
   Debug((DEBUG_ENGINE, "kqueue: Adding filter for signal %d [%p]",
         sig_signal(sig), sig));
@@ -276,7 +282,7 @@ engine_events(struct Socket* sock, unsigned int new_events)
 static void
 engine_delete(struct Socket* sock)
 {
-  struct kevent dellist[2];
+  int ii;
 
   assert(0 != sock);
   assert(sock == sockList[s_fd(sock)]);
@@ -284,21 +290,14 @@ engine_delete(struct Socket* sock)
   Debug((DEBUG_ENGINE, "kqueue: Deleting socket %d [%p], state %s",
         s_fd(sock), sock, state_to_name(s_state(sock))));
 
-  dellist[0].ident = s_fd(sock); /* set up the delete list */
-  dellist[0].filter = EVFILT_READ; /* readable filter */
-  dellist[0].flags = EV_DELETE; /* delete it */
-  dellist[0].fflags = 0;
-  dellist[0].data = 0;
-  dellist[0].udata = 0;
-
-  dellist[1].ident = s_fd(sock);
-  dellist[1].filter = EVFILT_WRITE; /* writable filter */
-  dellist[1].flags = EV_DELETE; /* delete it */
-  dellist[1].fflags = 0;
-  dellist[1].data = 0;
-  dellist[1].udata = 0;
-
   sockList[s_fd(sock)] = 0;
+
+  /* Drop any unprocessed events citing this socket. */
+  for (ii = events_i; ii < events_used; ii++) {
+    if (events[ii].ident == s_fd(sock)) {
+      events[ii] = events[--events_used];
+    }
+  }
 }
 
 /** Run engine event loop.
@@ -307,14 +306,13 @@ engine_delete(struct Socket* sock)
 static void
 engine_loop(struct Generators* gen)
 {
-  struct kevent *events;
   int events_count;
+  struct kevent *evt;
   struct Socket* sock;
   struct timespec wait;
-  int nevs;
   int i;
   int errcode;
-  size_t codesize;
+  socklen_t codesize;
 
   if ((events_count = feature_int(FEAT_POLLS_PER_LOOP)) < 20)
     events_count = 20;
@@ -330,16 +328,16 @@ engine_loop(struct Generators* gen)
     wait.tv_sec = timer_next(gen) ? (timer_next(gen) - CurrentTime) : -1;
     wait.tv_nsec = 0;
 
-    Debug((DEBUG_INFO, "kqueue: delay: %Tu (%Tu) %Tu", timer_next(gen),
+    Debug((DEBUG_ENGINE, "kqueue: delay: %Tu (%Tu) %Tu", timer_next(gen),
           CurrentTime, wait.tv_sec));
 
     /* check for active events */
-    nevs = kevent(kqueue_id, 0, 0, events, events_count,
-                 wait.tv_sec < 0 ? 0 : &wait);
+    events_used = kevent(kqueue_id, 0, 0, events, events_count,
+                         wait.tv_sec < 0 ? 0 : &wait);
 
     CurrentTime = time(0); /* set current time... */
 
-    if (nevs < 0) {
+    if (events_used < 0) {
       if (errno != EINTR) { /* ignore kevent interrupts */
        /* Log the kqueue error */
        log_write(LS_SOCKET, L_ERROR, 0, "kevent() error: %m");
@@ -355,21 +353,22 @@ engine_loop(struct Generators* gen)
       continue;
     }
 
-    for (i = 0; i < nevs; i++) {
-      if (events[i].filter == EVFILT_SIGNAL) {
+    for (events_i = 0; events_i < events_used; events_i++) {
+      evt = &events[events_i];
+
+      if (evt->filter == EVFILT_SIGNAL) {
        /* it's a signal; deal appropriately */
-       event_generate(ET_SIGNAL, events[i].udata, events[i].ident);
+       event_generate(ET_SIGNAL, evt->udata, evt->ident);
        continue; /* skip socket processing loop */
       }
 
-      assert(events[i].filter == EVFILT_READ ||
-            events[i].filter == EVFILT_WRITE);
+      assert(evt->filter == EVFILT_READ || evt->filter == EVFILT_WRITE);
 
-      sock = sockList[events[i].ident];
+      sock = sockList[evt->ident];
       if (!sock) /* slots may become empty while processing events */
        continue;
 
-      assert(s_fd(sock) == events[i].ident);
+      assert(s_fd(sock) == evt->ident);
 
       gen_ref_inc(sock); /* can't have it going away on us */
 
@@ -395,14 +394,14 @@ engine_loop(struct Generators* gen)
 
       switch (s_state(sock)) {
       case SS_CONNECTING:
-       if (events[i].filter == EVFILT_WRITE) { /* connection completed */
+       if (evt->filter == EVFILT_WRITE) { /* connection completed */
          Debug((DEBUG_ENGINE, "kqueue: Connection completed"));
          event_generate(ET_CONNECT, sock, 0);
        }
        break;
 
       case SS_LISTENING:
-       if (events[i].filter == EVFILT_READ) { /* connect. to be accept. */
+       if (evt->filter == EVFILT_READ) { /* connect. to be accept. */
          Debug((DEBUG_ENGINE, "kqueue: Ready for accept"));
          event_generate(ET_ACCEPT, sock, 0);
        }
@@ -410,22 +409,22 @@ engine_loop(struct Generators* gen)
 
       case SS_NOTSOCK: /* doing nothing socket-specific */
       case SS_CONNECTED:
-       if (events[i].filter == EVFILT_READ) { /* data on socket */
+       if (evt->filter == EVFILT_READ) { /* data on socket */
          Debug((DEBUG_ENGINE, "kqueue: EOF or data to be read"));
-         event_generate(events[i].flags & EV_EOF ? ET_EOF : ET_READ, sock, 0);
+         event_generate(evt->flags & EV_EOF ? ET_EOF : ET_READ, sock, 0);
        }
-       if (events[i].filter == EVFILT_WRITE) { /* socket writable */
+       if (evt->filter == EVFILT_WRITE) { /* socket writable */
          Debug((DEBUG_ENGINE, "kqueue: Data can be written"));
          event_generate(ET_WRITE, sock, 0);
        }
        break;
 
       case SS_DATAGRAM: case SS_CONNECTDG:
-       if (events[i].filter == EVFILT_READ) { /* socket readable */
+       if (evt->filter == EVFILT_READ) { /* socket readable */
          Debug((DEBUG_ENGINE, "kqueue: Datagram to be read"));
          event_generate(ET_READ, sock, 0);
        }
-       if (events[i].filter == EVFILT_WRITE) { /* socket writable */
+       if (evt->filter == EVFILT_WRITE) { /* socket writable */
          Debug((DEBUG_ENGINE, "kqueue: Datagram can be written"));
          event_generate(ET_WRITE, sock, 0);
        }