[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
[tor-commits] [tor/master] sched: change most asserts to non-fatal BUGs
commit 6ff8c86ac663914901b4ea9ce64c20b59bec6011
Author: Matt Traudt <sirmatt@xxxxxxx>
Date: Wed Sep 13 16:35:15 2017 -0400
sched: change most asserts to non-fatal BUGs
---
src/or/scheduler.c | 69 ++++++++++++++++++++++++++++++++++------------
src/or/scheduler_kist.c | 39 +++++++++++++++++++++-----
src/or/scheduler_vanilla.c | 19 +++++++++++--
3 files changed, 99 insertions(+), 28 deletions(-)
diff --git a/src/or/scheduler.c b/src/or/scheduler.c
index cb93f93c6..8e4cec095 100644
--- a/src/or/scheduler.c
+++ b/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);
- tor_assert(c2);
+ IF_BUG_ONCE(!c1 || !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);
-
- tor_assert(channels_pending);
+ IF_BUG_ONCE(!chan) {
+ return;
+ }
+ 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);
- tor_assert(channels_pending);
+ IF_BUG_ONCE(!chan) {
+ 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);
- tor_assert(channels_pending);
+ IF_BUG_ONCE(!chan) {
+ 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);
- tor_assert(channels_pending);
+ IF_BUG_ONCE(!chan) {
+ 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 */
diff --git a/src/or/scheduler_kist.c b/src/or/scheduler_kist.c
index 0cedecd8f..aec8192ed 100644
--- a/src/or/scheduler_kist.c
+++ b/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
diff --git a/src/or/scheduler_vanilla.c b/src/or/scheduler_vanilla.c
index d35b48c74..efc0e98de 100644
--- a/src/or/scheduler_vanilla.c
+++ b/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);
_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits