r11873@catbus: nickm | 2007-02-22 01:21:14 -0500

 twiddle signewnym rate-limiting patch so every signal gets handled eventually.  document it in control-spec. add a changelog.


svn:r9615

4 files changed, 30 insertions, 9 deletions

diff --git a/ChangeLog b/ChangeLog
index 0be54717b0..1b0e70cd6c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -52,6 +52,9 @@ Changes in version 0.1.2.8-alpha - 2007-??-??
       advance warning.
     - Add STREAM_BW events to report per-entry-stream bandwidth use. (Patch
       from Robert Hogan.)
+    - Rate-limit SIGNEWNYM events in response to controllers that impolitely
+      generate them for every single stream.  (Patch from mwenge; closes bug
+      394.)
 
   o Minor bugfixes (performance):
     - Call router_have_min_dir_info half as often.  (This is showing up in
diff --git a/doc/TODO b/doc/TODO
index ba8ca31333..8cd55881b8 100644
--- a/doc/TODO
+++ b/doc/TODO
@@ -334,7 +334,7 @@ R - add d64 and fp64 along-side d and fp so people can paste status
     https thing in the default configuration:
     http://wiki.noreply.org/noreply/TheOnionRouter/TorFAQ#PrivoxyWeirdSSLPort
   . Flesh out options_description array in src/or/config.c
-  . Don't let 'newnym' be triggered more often than every n seconds.
+  o Don't let 'newnym' be triggered more often than every n seconds.
   X If we try to publish as a nickname that's already claimed, should
     we append a number (or increment the number) and try again? This
     way people who read their logs can fix it as before, but people
diff --git a/doc/spec/control-spec.txt b/doc/spec/control-spec.txt
index 05eec0ca96..32deedf34d 100644
--- a/doc/spec/control-spec.txt
+++ b/doc/spec/control-spec.txt
@@ -258,7 +258,8 @@ $Id$
       CLEARDNSCACHE -- Forget the client-side cached IPs for all hostnames.
       NEWNYM    -- Switch to clean circuits, so new application requests
                    don't share any circuits with old ones.  Also clears
-                   the client-side DNS cache.
+                   the client-side DNS cache.  (Tor MAY rate-limit its
+                   response to this signal.)
 
   The server responds with "250 OK" if the signal is recognized (or simply
   closes the socket if it was asked to close immediately), or "552
diff --git a/src/or/main.c b/src/or/main.c
index dc70a401bf..9b7cea697b 100644
--- a/src/or/main.c
+++ b/src/or/main.c
@@ -54,8 +54,13 @@ static time_t time_to_fetch_directory = 0;
 static time_t time_to_fetch_running_routers = 0;
 /** When do we next launch DNS wildcarding checks? */
 static time_t time_to_check_for_correct_dns = 0;
-/** When do we next allow a SIGNEWNYM? */
-static time_t time_to_allow_next_signewnym = 0;
+
+/** How often will we honor SIGNEWNYM requests? */
+#define MAX_SIGNEWNYM_RATE 10
+/** When did we last process a SIGNEWNYM request? */
+static time_t time_of_last_signewnym = 0;
+/** Is there a signewnym request we're currently waiting to handle? */
+static int signewnym_is_pending = 0;
 
 /** Array of all open connections.  The first n_conns elements are valid. */
 static connection_t *connection_array[MAXCONNECTIONS+1] =
@@ -748,6 +753,16 @@ run_scheduled_events(time_t now)
    */
   consider_hibernation(now);
 
+  /* 0b. If we've deferred a signewnym, make sure it gets handled
+   * eventually */
+  if (signewnym_is_pending &&
+      time_of_last_signewnym + MAX_SIGNEWNYM_RATE < now) {
+    circuit_expire_all_dirty_circs();
+    addressmap_clear_transient();
+    time_of_last_signewnym = now;
+    signewnym_is_pending = 0;
+  }
+
   /** 1a. Every MIN_ONION_KEY_LIFETIME seconds, rotate the onion keys,
    *  shut down and restart all cpuworkers, and update the directory if
    *  necessary.
@@ -1330,7 +1345,6 @@ signal_callback(int fd, short events, void *arg)
   uintptr_t sig = (uintptr_t)arg;
   (void)fd;
   (void)events;
-  time_t now = time(NULL);
   switch (sig)
     {
     case SIGTERM:
@@ -1373,14 +1387,17 @@ signal_callback(int fd, short events, void *arg)
                                                 zombies */
       break;
 #endif
-    case SIGNEWNYM:
-      if (time_to_allow_next_signewnym < now) {
+    case SIGNEWNYM: {
+      time_t now = time(NULL);
+      if (time_of_last_signewnym + MAX_SIGNEWNYM_RATE >= now) {
+        signewnym_is_pending = 1;
+      } else {
         circuit_expire_all_dirty_circs();
         addressmap_clear_transient();
-#define NEXT_SIGNEWNYM (5)
-        time_to_allow_next_signewnym = now + NEXT_SIGNEWNYM;
+        time_of_last_signewnym = now;
       }
       break;
+    }
     case SIGCLEARDNSCACHE:
       addressmap_clear_transient();
       break;