[tor-commits] [obfsproxy/master] Improvements on the signal handling code.

nickm at torproject.org nickm at torproject.org
Thu Jul 14 15:39:26 UTC 2011


commit 9d96eb9422b23fecacc198bf110e3a944ab182be
Author: George Kadianakis <desnacked at gmail.com>
Date:   Fri Jul 8 18:15:55 2011 +0200

    Improvements on the signal handling code.
    
    * Implemented a generic double linked list, which is now used for
      connections and listeners.
    * We now close the listeners when we are shutting down instead of
      closing their sockets.
---
 src/main.c    |   69 ++++++++++++++++++++------
 src/network.c |  153 ++++++++++++++++++--------------------------------------
 src/network.h |    1 +
 src/util.c    |  141 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/util.h    |   19 +++++++
 5 files changed, 264 insertions(+), 119 deletions(-)

diff --git a/src/main.c b/src/main.c
index 81e8c31..56ea9c9 100644
--- a/src/main.c
+++ b/src/main.c
@@ -35,6 +35,9 @@ extern int n_supported_protocols;
 
 static struct event_base *the_event_base=NULL;
 
+/** Doubly linked list holding all our listeners. */
+static dll_t *ll=NULL;
+
 /**
    Prints the obfsproxy usage instructions then exits.
 */
@@ -58,6 +61,24 @@ usage(void)
 }
 
 /**
+   Disables all active listeners.
+*/
+static void
+disable_all_listeners(void)
+{
+  if (!ll)
+    return;
+  log_info("Disabling all listeners.");
+
+  /* Iterate listener doubly linked list and disable them all. */ 
+  dll_node_t *ll_node = ll->head;
+  while (ll_node) {
+    listener_disable(ll_node->data);
+    ll_node = ll_node->next;
+  }
+}
+
+/**
    This is called when we receive a signal.
    It figures out the signal type and acts accordingly.
 
@@ -73,17 +94,29 @@ handle_signal_cb(evutil_socket_t fd, short what, void *arg)
 {
   int signum = (int) fd;
   static int got_sigint=0;
+  static int disabled_listeners=0;
 
   switch (signum) {
   case SIGINT:
+    if (!disabled_listeners) {
+      disable_all_listeners();
+      disabled_listeners++;
+    }
     if (!got_sigint) {
+      log_info("Got SIGINT. Preparing shutdown.");
       start_shutdown(0);
       got_sigint++;
     } else {
+      log_info("Got SIGINT for the second time. Terminating.");
       start_shutdown(1);
     }
     break;
   case SIGTERM:
+    if (!disabled_listeners) {
+      disable_all_listeners();
+      disabled_listeners++;
+    }
+    log_info("Got SIGTERM. Terminating.");
     start_shutdown(1);
     break;
   }
@@ -348,18 +381,10 @@ main(int argc, const char **argv)
 
   /*Let's open a new listener for each protocol. */ 
   int h;
-  listener_t **listeners;
   listener_t *temp_listener;
   int n_listeners=0;
   protocol_params_t *proto_params=NULL;
-  listeners = calloc(sizeof(listener_t*), actual_protocols);
-  if (!listeners) {
-    log_warn("Allocation failure: %s", strerror(errno));
-    return 1;
-  }
-
   for (h=0;h<actual_protocols;h++) {
-
     log_debug("Spawning listener %d!", h+1);
 
     /** normally free'd in listener_free() */
@@ -379,8 +404,16 @@ main(int argc, const char **argv)
       continue;
     
     log_info("Succesfully created listener %d.", h+1);
-    listeners[n_listeners] = temp_listener;
-    
+
+    /* If we don't have a listener dll, create one now. */
+    if (!ll) {
+      ll = calloc(1, sizeof(dll_t));
+      if (!ll)
+        return 1;
+    }
+        
+    /* Append our new listener in the listener dll. */
+    dll_append(ll, temp_listener);
     n_listeners++;
   }
 
@@ -398,11 +431,17 @@ main(int argc, const char **argv)
   if (close_obfsproxy_logfile() < 0)
     printf("Failed closing logfile!\n");
 
-  /* We are exiting. Clean everything. */
-  for (h=0;h<n_listeners;h++)
-    listener_free(listeners[h]);
-  if (listeners)
-    free(listeners);
+  /* Free all listeners in our listener dll. */
+  if (ll) {
+    dll_node_t *ll_node = ll->head;
+    while (ll_node) {
+      listener_free(ll_node->data);
+      dll_remove(ll, ll_node);
+      ll_node = ll->head;
+    }
+    /* free dll memory */
+    free(ll);
+  }
 
   free(protocol_options);
   free(n_options_array);
diff --git a/src/network.c b/src/network.c
index fc53564..1103eeb 100644
--- a/src/network.c
+++ b/src/network.c
@@ -26,19 +26,8 @@ struct listener_t {
   protocol_params_t *proto_params;
 };
 
-/**
-   Linked list carrying all currently active connections.
-   [linked list code was copied off tor.]
-*/
-typedef struct conn_list_t {
-  conn_t *conn;
-  struct conn_list_t *next;
-} conn_list_t;
-
-/** First and last elements in the linked list of active
-    connections. */
-static conn_list_t *cl_list=NULL;
-static conn_list_t *cl_tail=NULL;
+/** Doubly linked list holding all connections. */
+static dll_t *cl=NULL;
 /** Active connection counter */
 static int n_connections=0;
 
@@ -68,6 +57,8 @@ static void output_event_cb(struct bufferevent *bev, short what, void *arg);
 
    If 'barbaric' is set, we forcefully close all open connections and
    finish shutdown.
+   
+   (Only called by signal handlers)
 */
 void
 start_shutdown(int barbaric)
@@ -76,6 +67,8 @@ start_shutdown(int barbaric)
     shutting_down=1;
 
   if (!n_connections) {
+    if (cl)
+      free(cl);
     finish_shutdown();
     return;
   }
@@ -83,7 +76,6 @@ start_shutdown(int barbaric)
   if (barbaric) {
     if (n_connections)
       close_all_connections();
-    finish_shutdown();
     return;
   }
 }  
@@ -94,87 +86,18 @@ start_shutdown(int barbaric)
 static void
 close_all_connections(void)
 {
-  conn_t *conn;
-
-  while (cl_list) {
-    assert(cl_list->conn);
-    assert(n_connections > 0);
-    conn = cl_list->conn;
-    conn_free(conn);
-  }    
-}
-
-/** 
-    Places 'conn' to the end of the linked list of active connections.
-    Returns 1 on success, -1 on fail.
-*/
-static int
-cl_add(conn_t *conn)
-{
-  conn_list_t *cl;
-  
-  cl = calloc(1, sizeof(conn_list_t));
-  if (!cl)
-    return -1;
-
-  cl->conn = conn;
-  
-  if (!cl_tail) {
-    assert(!cl_list);
-    assert(!n_connections);
-    cl_list=cl;
-    cl_tail=cl;
-  } else {
-    assert(cl_list);
-    assert(!cl_tail->next);
-    cl_tail->next = cl;
-    cl_tail = cl;
-  }
-
-  n_connections++;
-
-  return 1;
-}
-
-/**
-   Removes 'conn' from the linked list of connections:
-   It frees the list node carrying 'conn', but leaves 'conn' itself intact.
-*/ 
-static void
-cl_remove(conn_t *conn)
-{
-  conn_list_t *tmpo, *victim;
-
-  if (!cl_list)
-    return; /* nothing here. */
-
-  /* first check to see if it's the first entry */
-  tmpo = cl_list;
-  if (tmpo->conn == conn) {
-    /* it's the first one. remove it from the list. */
-    cl_list = tmpo->next;
-    if (!cl_list)
-      cl_tail = NULL;
-    n_connections--;
-    victim = tmpo;
-  } else { /* we need to hunt through the rest of the list */
-    for ( ;tmpo->next && tmpo->next->conn != conn; tmpo=tmpo->next) ;
-    if (!tmpo->next) {
-      return;
+  /** Traverse the dll and close all connections */
+  dll_node_t *cl_node = cl->head;
+  while (cl_node) {
+    conn_free(cl_node->data);
+
+    if (cl) { /* last conn_free() wipes the cl. */
+      cl_node = cl->head; /* move to next connection */
+    } else {
+      assert(!n_connections);
+      return; /* connections are now all closed. */  
     }
-    /* now we know tmpo->next->conn == conn */
-    victim = tmpo->next;
-    tmpo->next = victim->next;
-    if (cl_tail == victim)
-      cl_tail = tmpo;
-    n_connections--;
-  }
-
-  assert(n_connections>=0);
-
-  /* now victim points to the element that needs to be removed */
-  free(victim);
-  victim = NULL;
+  }    
 }
   
 /**
@@ -197,6 +120,13 @@ listener_new(struct event_base *base,
     return NULL;
   }
 
+  /** If we don't have a connection dll, create one now. */
+  if (!cl) {
+    cl = calloc(1, sizeof(dll_t));
+    if (!cl)
+      return NULL;
+  }
+
   lsn->proto_params = proto_params;
   
   lsn->listener = evconnlistener_new_bind(base, simple_listener_cb, lsn,
@@ -214,6 +144,17 @@ listener_new(struct event_base *base,
   return lsn;
 }
 
+/**
+   Disable listener 'lsn'.
+*/
+void
+listener_disable(listener_t *lsn)
+{
+  assert(lsn);
+  
+  evconnlistener_disable(lsn->listener);  
+}
+
 void
 listener_free(listener_t *lsn)
 {
@@ -229,12 +170,6 @@ static void
 simple_listener_cb(struct evconnlistener *evcl,
     evutil_socket_t fd, struct sockaddr *sourceaddr, int socklen, void *arg)
 {
-  if (shutting_down) {
-    if (fd >= 0)
-      evutil_closesocket(fd);
-    return ;
-  }
-
   listener_t *lsn = arg;
   struct event_base *base;
   conn_t *conn = calloc(1, sizeof(conn_t));
@@ -315,8 +250,9 @@ simple_listener_cb(struct evconnlistener *evcl,
   }
 
   /* add conn to the linked list of connections */
-  if (cl_add(conn)<0)
+  if (dll_append(cl, conn)<0)
     goto err;
+  n_connections++;
 
   log_debug("Connection setup completed. "
             "We currently have %d connections!", n_connections);
@@ -340,18 +276,27 @@ conn_free(conn_t *conn)
     bufferevent_free(conn->input);
   if (conn->output)
     bufferevent_free(conn->output);
-  memset(conn, 0x99, sizeof(conn_t));
+
   /* remove conn from the linked list of connections */
-  cl_remove(conn);
+  dll_remove_with_data(cl, conn);
+  n_connections--;
+
+  memset(conn, 0x99, sizeof(conn_t));
   free(conn);
 
+  assert(n_connections>=0);
   log_debug("Connection destroyed. "
             "We currently have %d connections!", n_connections);
 
   /** If this was the last connection AND we are shutting down,
       finish shutdown. */
-  if (!n_connections && shutting_down)
+  if (!n_connections && shutting_down) {
+    if (cl) { /* free connection dll */ 
+      free(cl);
+      cl = NULL;
+    }
     finish_shutdown();
+  }
 }
 
 static void
diff --git a/src/network.h b/src/network.h
index 28a0d7d..a8c02ac 100644
--- a/src/network.h
+++ b/src/network.h
@@ -46,6 +46,7 @@ struct addrinfo;
 listener_t *listener_new(struct event_base *base,
                          struct protocol_params_t *params);
 void listener_free(listener_t *listener);
+void listener_disable(listener_t *lsn);
 
 void start_shutdown(int barbaric);
 
diff --git a/src/util.c b/src/util.c
index 7e3e747..1ee7bcb 100644
--- a/src/util.c
+++ b/src/util.c
@@ -148,6 +148,146 @@ obfs_vsnprintf(char *str, size_t size, const char *format, va_list args)
   return r;
 }
 
+/************************ Doubly Linked List (DLL) ******************/
+
+/**
+   Insert 'new_node' after 'node' in the doubly linked list 'list'.
+*/ 
+static void
+dll_insert_after(dll_t *list, dll_node_t *node, dll_node_t *new_node)
+{
+  assert(node);
+  assert(new_node);
+
+  if (!list)
+    return;
+
+  new_node->prev = node;
+  new_node->next = node->next;
+  if (!node->next)
+    list->tail = new_node;
+  else
+    node->next->prev = new_node;
+  node->next = new_node;
+}
+
+/**
+   Insert 'new_node' before 'node' in the doubly linked list 'list'.
+*/ 
+static void
+dll_insert_before(dll_t *list, dll_node_t *node, dll_node_t *new_node)
+{
+  assert(node);
+  assert(new_node);
+
+  if (!list)
+    return;
+
+  new_node->prev = node->prev;
+  new_node->next = node;
+  if (!node->prev)
+    list->head = new_node;
+  else
+    node->prev->next = new_node;
+  node->prev = new_node;
+}
+  
+/**
+   Insert 'node' in the beginning of the doubly linked 'list'.
+*/ 
+static void
+dll_insert_beginning(dll_t *list, dll_node_t *node)
+{
+  assert(node);
+
+  if (!list)
+    return;
+
+  if (!list->head) {
+    list->head = node;
+    list->tail = node;
+    node->prev = NULL;
+    node->next = NULL;
+  } else {
+    dll_insert_before(list, list->head, node);
+  }
+}
+  
+/** 
+    Appends 'data' to the end of the doubly linked 'list'.
+    Returns 1 on success, -1 on fail.
+*/
+int
+dll_append(dll_t *list, void *data)
+{
+  assert(data);
+
+  if (!list)
+    return -1;
+
+  dll_node_t *node;
+  
+  node = calloc(1, sizeof(dll_node_t));
+  if (!node)
+    return -1;
+  node->data = data;
+  
+  if (!list->tail)
+    dll_insert_beginning(list, node);
+  else
+    dll_insert_after(list, list->tail, node);
+      
+  return 1;
+}
+
+/**
+   Removes 'node' from the doubly linked list 'list'.
+   It frees the list node, but leaves its data intact.
+*/ 
+void
+dll_remove(dll_t *list, dll_node_t *node)
+{
+  assert(node);
+
+  if (!list)
+    return;
+
+  if (!node->prev)
+    list->head = node->next;
+  else
+    node->prev->next = node->next;
+  if (!node->next)
+    list->tail = node->prev;
+  else
+    node->next->prev = node->prev;
+
+  free(node);
+}
+
+/**
+   Removes node carrying 'data' from the doubly linked list 'list'.
+   It frees the list node, but leaves 'data' intact.
+*/
+void
+dll_remove_with_data(dll_t *list, void *data)
+{
+  assert(data);
+  
+  if (!list)
+    return;
+
+  dll_node_t *node = list->head;
+  while (node) {
+    if (node->data == data) {
+      dll_remove(list, node);
+      return;
+    } else {
+      node = node->next;
+    }
+  }
+  assert(0); /*too brutal?*/
+}
+
 /************************ Logging Subsystem *************************/
 /** The code of this section was to a great extend shamelessly copied
     off tor. It's basicaly a stripped down version of tor's logging
@@ -404,3 +544,4 @@ log_debug(const char *format, ...)
   va_end(ap);
 }
 #endif
+
diff --git a/src/util.h b/src/util.h
index 2705d51..999c5df 100644
--- a/src/util.h
+++ b/src/util.h
@@ -50,6 +50,25 @@ int obfs_snprintf(char *str, size_t size,
                   const char *format, ...)
   __attribute__((format(printf, 3, 4)));
 
+/***** Doubly Linked List stuff. *****/
+
+/** A doubly linked list node.
+    [algorithms ripped off Wikipedia (Doubly_linked_list) ] */
+typedef struct dll_node_t {
+  struct dll_node_t *next, *prev;
+  void *data;
+} dll_node_t;
+
+/** A doubly linked list. */
+typedef struct dll_t {
+  struct dll_node_t *head;
+  struct dll_node_t *tail;
+} dll_t;
+
+int dll_append(dll_t *list, void *data);
+void dll_remove(dll_t *list, dll_node_t *node);
+void dll_remove_with_data(dll_t *list, void *data);
+
 /***** Logging subsystem stuff. *****/
 
 void log_fn(int severity, const char *format, ...)





More information about the tor-commits mailing list