diff --git a/changes/detect-full-queues b/changes/detect-full-queues new file mode 100644 index 0000000000..c00e3ea8cc --- /dev/null +++ b/changes/detect-full-queues @@ -0,0 +1,8 @@ + o Major bugfixes: + - Newly created streams were allowed to read cells onto circuits, + even if the circuit's cell queue was blocked and waiting to drain. + This created potential unfairness, as older streams would be + blocked, but newer streams would gladly fill the queue completely. + We add code to detect this situation and prevent any stream from + getting more than one free cell. Bugfix on 0.2.0.1-alpha. + Possible partial fix for bug 1298. diff --git a/src/or/relay.c b/src/or/relay.c index c123eb3973..88106e5d96 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -2100,12 +2100,16 @@ connection_or_unlink_all_active_circs(or_connection_t *orconn) /** Block (if block is true) or unblock (if block is false) * every edge connection that is using circ to write to orconn, - * and start or stop reading as appropriate. */ -static void + * and start or stop reading as appropriate. + * + * Returns the number of streams whose status we changed. + */ +static int set_streams_blocked_on_circ(circuit_t *circ, or_connection_t *orconn, int block) { edge_connection_t *edge = NULL; + int n = 0; if (circ->n_conn == orconn) { circ->streams_blocked_on_n_conn = block; if (CIRCUIT_IS_ORIGIN(circ)) @@ -2118,7 +2122,10 @@ set_streams_blocked_on_circ(circuit_t *circ, or_connection_t *orconn, for (; edge; edge = edge->next_stream) { connection_t *conn = TO_CONN(edge); - edge->edge_blocked_on_circ = block; + if (edge->edge_blocked_on_circ != block) { + ++n; + edge->edge_blocked_on_circ = block; + } if (!conn->read_event) { /* This connection is a placeholder for something; probably a DNS @@ -2135,6 +2142,8 @@ set_streams_blocked_on_circ(circuit_t *circ, or_connection_t *orconn, connection_start_reading(conn); } } + + return n; } /** Pull as many cells as possible (but no more than max) from the @@ -2301,6 +2310,18 @@ append_cell_to_circuit_queue(circuit_t *circ, or_connection_t *orconn, if (!streams_blocked && queue->n >= CELL_QUEUE_HIGHWATER_SIZE) set_streams_blocked_on_circ(circ, orconn, 1); /* block streams */ + if (streams_blocked) { + /* We must have missed a connection! */ + int n = set_streams_blocked_on_circ(circ, orconn, 1); + if (n) { + log_info(LD_BUG, "Got a cell added to a cell queue when streams were " + "supposed to be blocked; found that %d streams weren't.", n); + } else { + log_info(LD_BUG, "Got a cell added to a cell queue when streams were " + "all blocked. We should figure out why."); + } + } + if (queue->n == 1) { /* This was the first cell added to the queue. We need to make this * circuit active. */