sched: change most asserts to non-fatal BUGs

src/or/scheduler.c

@@ -184,13 +184,19 @@ scheduler_evt_callback(evutil_socket_t fd, short events, void *arg)
 
   log_debug(LD_SCHED, "Scheduler event callback called");
 
-  tor_assert(run_sched_ev);
-
   /* Run the scheduler. This is a mandatory function. */
+
+  /* We might as well assert on this. If this function doesn't exist, no cells
+   * are getting scheduled. Things are very broken. scheduler_t says the run()
+   * function is mandatory. */
   tor_assert(the_scheduler->run);
   the_scheduler->run();
 
   /* Schedule itself back in if it has more work. */
+
+  /* Again, might as well assert on this mandatory scheduler_t function. If it
+   * doesn't exist, there's no way to tell libevent to run the scheduler again
+   * in the future. */
   tor_assert(the_scheduler->schedule);
   the_scheduler->schedule();
 }
@@ -224,14 +230,18 @@ scheduler_compare_channels, (const void *c1_v, const void *c2_v))
   const circuitmux_policy_t *p1, *p2;
   uintptr_t p1_i, p2_i;
 
-  tor_assert(c1_v);
-  tor_assert(c2_v);
-
   c1 = (const channel_t *)(c1_v);
   c2 = (const channel_t *)(c2_v);
 
-  tor_assert(c1);
+  IF_BUG_ONCE(!c1 || !c2) {
-  tor_assert(c2);
+    if (c1 && !c2) {
+      return -1;
+    } else if (c2 && !c1) {
+      return 1;
+    } else {
+      return -1;
+    }
+  }
 
   if (c1 != c2) {
     if (circuitmux_get_policy(c1->cmux) ==
@@ -368,9 +378,12 @@ scheduler_free_all(void)
 MOCK_IMPL(void,
 scheduler_channel_doesnt_want_writes,(channel_t *chan))
 {
-  tor_assert(chan);
+  IF_BUG_ONCE(!chan) {
-
+    return;
-  tor_assert(channels_pending);
+  }
+  IF_BUG_ONCE(!channels_pending) {
+    return;
+  }
 
   /* If it's already in pending, we can put it in waiting_to_write */
   if (chan->scheduler_state == SCHED_CHAN_PENDING) {
@@ -408,8 +421,12 @@ scheduler_channel_doesnt_want_writes,(channel_t *chan))
 MOCK_IMPL(void,
 scheduler_channel_has_waiting_cells,(channel_t *chan))
 {
-  tor_assert(chan);
+  IF_BUG_ONCE(!chan) {
-  tor_assert(channels_pending);
+    return;
+  }
+  IF_BUG_ONCE(!channels_pending) {
+    return;
+  }
 
   /* First, check if this one also writeable */
   if (chan->scheduler_state == SCHED_CHAN_WAITING_FOR_CELLS) {
@@ -456,7 +473,13 @@ scheduler_init(void)
 {
   log_debug(LD_SCHED, "Initting scheduler");
 
-  tor_assert(!run_sched_ev);
+  // Two '!' because we really do want to check if the pointer is non-NULL
+  IF_BUG_ONCE(!!run_sched_ev) {
+    log_warn(LD_SCHED, "We should not already have a libevent scheduler event."
+             "I'll clean the old one up, but this is odd.");
+    tor_event_free(run_sched_ev);
+    run_sched_ev = NULL;
+  }
   run_sched_ev = tor_event_new(tor_libevent_get_base(), -1,
                                0, scheduler_evt_callback, NULL);
   channels_pending = smartlist_new();
@@ -473,8 +496,12 @@ scheduler_init(void)
 MOCK_IMPL(void,
 scheduler_release_channel,(channel_t *chan))
 {
-  tor_assert(chan);
+  IF_BUG_ONCE(!chan) {
-  tor_assert(channels_pending);
+    return;
+  }
+  IF_BUG_ONCE(!channels_pending) {
+    return;
+  }
 
   if (chan->scheduler_state == SCHED_CHAN_PENDING) {
     if (smartlist_pos(channels_pending, chan) == -1) {
@@ -500,8 +527,12 @@ scheduler_release_channel,(channel_t *chan))
 void
 scheduler_channel_wants_writes(channel_t *chan)
 {
-  tor_assert(chan);
+  IF_BUG_ONCE(!chan) {
-  tor_assert(channels_pending);
+    return;
+  }
+  IF_BUG_ONCE(!channels_pending) {
+    return;
+  }
 
   /* If it's already in waiting_to_write, we can put it in pending */
   if (chan->scheduler_state == SCHED_CHAN_WAITING_TO_WRITE) {
@@ -544,7 +575,9 @@ scheduler_channel_wants_writes(channel_t *chan)
 void
 scheduler_touch_channel(channel_t *chan)
 {
-  tor_assert(chan);
+  IF_BUG_ONCE(!chan) {
+    return;
+  }
 
   if (chan->scheduler_state == SCHED_CHAN_PENDING) {
     /* Remove and re-add it */

src/or/scheduler_kist.c

@@ -332,7 +332,9 @@ socket_can_write(socket_table_t *table, const channel_t *chan)
 {
   socket_table_ent_t *ent = NULL;
   ent = socket_table_search(table, chan);
-  tor_assert(ent);
+  IF_BUG_ONCE(!ent) {
+    return 1; // Just return true, saying that kist wouldn't limit the socket
+  }
 
   int64_t kist_limit_space =
     (int64_t) (ent->limit - ent->written) /
@@ -346,7 +348,9 @@ update_socket_info(socket_table_t *table, const channel_t *chan)
 {
   socket_table_ent_t *ent = NULL;
   ent = socket_table_search(table, chan);
-  tor_assert(ent);
+  IF_BUG_ONCE(!ent) {
+    return; // Whelp. Entry didn't exist for some reason so nothing to do.
+  }
   update_socket_info_impl(ent);
 }
 
@@ -356,7 +360,9 @@ update_socket_written(socket_table_t *table, channel_t *chan, size_t bytes)
 {
   socket_table_ent_t *ent = NULL;
   ent = socket_table_search(table, chan);
-  tor_assert(ent);
+  IF_BUG_ONCE(!ent) {
+    return; // Whelp. Entry didn't exist so nothing to do.
+  }
 
   log_debug(LD_SCHED, "chan=%" PRIu64 " wrote %lu bytes, old was %" PRIi64,
             chan->global_identifier, bytes, ent->written);
@@ -396,7 +402,9 @@ static int
 have_work(void)
 {
   smartlist_t *cp = get_channels_pending();
-  tor_assert(cp);
+  IF_BUG_ONCE(!cp) {
+    return 0; // channels_pending doesn't exist so... no work?
+  }
   return smartlist_len(cp) > 0;
 }
 
@@ -441,7 +449,13 @@ static void
 kist_scheduler_init(void)
 {
   kist_scheduler_on_new_options();
-  tor_assert(sched_run_interval > 0);
+  IF_BUG_ONCE(sched_run_interval <= 0) {
+    log_warn(LD_SCHED, "We are initing the KIST scheduler and noticed the "
+             "KISTSchedRunInterval is telling us to not use KIST. That's "
+             "weird! We'll continue using KIST, but at %dms.",
+             KIST_SCHED_RUN_INTERVAL_DEFAULT);
+    sched_run_interval = KIST_SCHED_RUN_INTERVAL_DEFAULT;
+  }
 }
 
 /* Function of the scheduler interface: schedule() */
@@ -452,7 +466,13 @@ kist_scheduler_schedule(void)
   struct timeval next_run;
   int32_t diff;
   struct event *ev = get_run_sched_ev();
-  tor_assert(ev);
+  IF_BUG_ONCE(!ev) {
+    log_warn(LD_SCHED, "Wow we don't have a scheduler event. That's really "
+             "weird! We can't really schedule a scheduling run with libevent "
+             "without it. So we're going to stop trying now and hope we have "
+             "one next time. If we never get one, we're broken.");
+    return;
+  }
   if (!have_work()) {
     return;
   }
@@ -496,7 +516,12 @@ kist_scheduler_run(void)
     /* get best channel */
     chan = smartlist_pqueue_pop(cp, scheduler_compare_channels,
                                 offsetof(channel_t, sched_heap_idx));
-    tor_assert(chan);
+    IF_BUG_ONCE(!chan) {
+      /* Some-freaking-how a NULL got into the channels_pending. That should
+       * never happen, but it should be harmless to ignore it and keep looping.
+       */
+      continue;
+    }
     outbuf_table_add(&outbuf_table, chan);
 
     /* if we have switched to a new channel, consider writing the previous

src/or/scheduler_vanilla.c

@@ -29,7 +29,9 @@ static int
 have_work(void)
 {
   smartlist_t *cp = get_channels_pending();
-  tor_assert(cp);
+  IF_BUG_ONCE(!cp) {
+    return 0; // channels_pending doesn't exist so... no work?
+  }
   return smartlist_len(cp) > 0;
 }
 
@@ -42,7 +44,13 @@ vanilla_scheduler_schedule(void)
     return;
   }
   struct event *ev = get_run_sched_ev();
-  tor_assert(ev);
+  IF_BUG_ONCE(!ev) {
+    log_warn(LD_SCHED, "Wow we don't have a scheduler event. That's really "
+             "weird! We can't really schedule a scheduling run with libevent "
+             "without it. So we're going to stop trying now and hope we have "
+             "one next time. If we never get one, we're broken.");
+    return;
+  }
   event_active(ev, EV_TIMEOUT, 1);
 }
 
@@ -64,7 +72,12 @@ vanilla_scheduler_run(void)
     chan = smartlist_pqueue_pop(cp,
                                 scheduler_compare_channels,
                                 offsetof(channel_t, sched_heap_idx));
-    tor_assert(chan);
+    IF_BUG_ONCE(!chan) {
+      /* Some-freaking-how a NULL got into the channels_pending. That should
+       * never happen, but it should be harmless to ignore it and keep looping.
+       */
+      continue;
+    }
 
     /* Figure out how many cells we can write */
     n_cells = channel_num_cells_writeable(chan);