Clear cell queues when marking or truncating a circuit.

At best, this patch helps us avoid sending queued relayed cells that
would get ignored during the time between when a destroy cell is
sent and when the circuit is finally freed.  At worst, it lets us
release some memory a little earlier than it would otherwise.

Fix for bug #1184.  Bugfix on 0.2.0.1-alpha.
This commit is contained in:
Nick Mathewson
2010-07-30 18:55:24 -04:00
parent 15424bf800
commit 6f45101327
6 changed files with 37 additions and 7 deletions
+7
View File
@@ -0,0 +1,7 @@
o Minor bugfixes:
- Never relay a cell for a circuit we have already destroyed.
Between marking a circuit as closeable and finally closing it,
it may have been possible for a few queued cells to get relayed,
even though they would have been immediately dropped by the next
OR in the circuit. Fix 1184; bugfix on 0.2.0.1-alpha.
+3 -1
View File
@@ -595,7 +595,9 @@ see tor-design.pdf.
To tear down part of a circuit, the OP may send a RELAY_TRUNCATE cell
signaling a given OR (Stream ID zero). That OR sends a DESTROY
cell to the next node in the circuit, and replies to the OP with a
RELAY_TRUNCATED cell.
RELAY_TRUNCATED cell. If the OR has any RELAY cells queued on the
circuit for the next node in that it had not yet sent, it MAY
drop them without sending them.
When an unrecoverable error occurs along one connection in a
circuit, the nodes on either side of the connection should, if they
+6 -2
View File
@@ -1124,8 +1124,10 @@ _circuit_mark_for_close(circuit_t *circ, int reason, int line,
rend_client_remove_intro_point(ocirc->build_state->chosen_exit,
ocirc->rend_data);
}
if (circ->n_conn)
if (circ->n_conn) {
circuit_clear_cell_queue(circ, circ->n_conn);
connection_or_send_destroy(circ->n_circ_id, circ->n_conn, reason);
}
if (! CIRCUIT_IS_ORIGIN(circ)) {
or_circuit_t *or_circ = TO_OR_CIRCUIT(circ);
@@ -1149,8 +1151,10 @@ _circuit_mark_for_close(circuit_t *circ, int reason, int line,
conn->on_circuit = NULL;
}
if (or_circ->p_conn)
if (or_circ->p_conn) {
circuit_clear_cell_queue(circ, or_circ->p_conn);
connection_or_send_destroy(or_circ->p_circ_id, or_circ->p_conn, reason);
}
} else {
origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ);
edge_connection_t *conn;
-4
View File
@@ -1291,10 +1291,6 @@ connection_or_send_destroy(circid_t circ_id, or_connection_t *conn, int reason)
cell.payload[0] = (uint8_t) reason;
log_debug(LD_OR,"Sending destroy (circID %d).", circ_id);
/* XXXX It's possible that under some circumstances, we want the destroy
* to take precedence over other data waiting on the circuit's cell queue.
*/
connection_or_write_cell_to_buf(&cell, conn);
return 0;
}
+20
View File
@@ -1186,6 +1186,7 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
}
if (circ->n_conn) {
uint8_t trunc_reason = *(uint8_t*)(cell->payload + RELAY_HEADER_SIZE);
circuit_clear_cell_queue(circ, circ->n_conn);
connection_or_send_destroy(circ->n_circ_id, circ->n_conn,
trunc_reason);
circuit_set_n_circid_orconn(circ, 0, NULL);
@@ -2368,6 +2369,25 @@ decode_address_from_payload(tor_addr_t *addr_out, const char *payload,
return payload + 2 + (uint8_t)payload[1];
}
/** Remove all the cells queued on <b>circ</b> for <b>orconn</b>. */
void
circuit_clear_cell_queue(circuit_t *circ, or_connection_t *orconn)
{
cell_queue_t *queue;
if (circ->n_conn == orconn) {
queue = &circ->n_conn_cells;
} else {
or_circuit_t *orcirc = TO_OR_CIRCUIT(circ);
tor_assert(orcirc->p_conn == orconn);
queue = &orcirc->p_conn_cells;
}
if (queue->n)
make_circuit_inactive_on_conn(circ,orconn);
cell_queue_clear(queue);
}
/** Fail with an assert if the active circuits ring on <b>orconn</b> is
* corrupt. */
void
+1
View File
@@ -60,6 +60,7 @@ const char *decode_address_from_payload(tor_addr_t *addr_out,
unsigned cell_ewma_get_tick(void);
void cell_ewma_set_scale_factor(or_options_t *options,
networkstatus_t *consensus);
void circuit_clear_cell_queue(circuit_t *circ, or_connection_t *orconn);
#endif