[tor-commits] [tor/master] Experimentally decouple the main body of circuit_mark_for_close
nickm at torproject.org
nickm at torproject.org
Thu Nov 12 19:34:40 UTC 2015
commit 8b4e5b7ee902fb7fa07767410a18433d752c7aef
Author: Nick Mathewson <nickm at torproject.org>
Date: Fri Oct 2 17:55:25 2015 +0200
Experimentally decouple the main body of circuit_mark_for_close
---
changes/decouple_circuit_mark | 6 ++++
src/or/circuitlist.c | 77 ++++++++++++++++++++++++++++++-----------
src/or/or.h | 8 +++++
3 files changed, 71 insertions(+), 20 deletions(-)
diff --git a/changes/decouple_circuit_mark b/changes/decouple_circuit_mark
new file mode 100644
index 0000000..4b7ed77
--- /dev/null
+++ b/changes/decouple_circuit_mark
@@ -0,0 +1,6 @@
+ o Code simplification and refactoring:
+ - Extract the more complicated parts of circuit_mark_for_close into
+ a new function run periodically before connections are freed.
+ This change removes more than half of the functions currently
+ in the "blob".
+ Closes ticket #17218.
diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c
index 716024d..324f9f3 100644
--- a/src/or/circuitlist.c
+++ b/src/or/circuitlist.c
@@ -44,11 +44,16 @@ static smartlist_t *global_circuitlist = NULL;
/** A list of all the circuits in CIRCUIT_STATE_CHAN_WAIT. */
static smartlist_t *circuits_pending_chans = NULL;
+/** A list of all the circuits that have been marked with circuit_mark_for_close
+ * and which are waiting for circuit_about_to_free(). */
+static smartlist_t *circuits_pending_close = NULL;
+
static void circuit_free_cpath_node(crypt_path_t *victim);
static void cpath_ref_decref(crypt_path_reference_t *cpath_ref);
//static void circuit_set_rend_token(or_circuit_t *circ, int is_rend_circ,
// const uint8_t *token);
static void circuit_clear_rend_token(or_circuit_t *circ);
+static void circuit_about_to_free(circuit_t *circ);
/********* END VARIABLES ************/
@@ -451,16 +456,27 @@ circuit_count_pending_on_channel(channel_t *chan)
void
circuit_close_all_marked(void)
{
+ if (circuits_pending_close == NULL)
+ return;
+
smartlist_t *lst = circuit_get_global_list();
- SMARTLIST_FOREACH_BEGIN(lst, circuit_t *, circ) {
- /* Fix up index if SMARTLIST_DEL_CURRENT just moved this one. */
- circ->global_circuitlist_idx = circ_sl_idx;
- if (circ->marked_for_close) {
- circ->global_circuitlist_idx = -1;
- circuit_free(circ);
- SMARTLIST_DEL_CURRENT(lst, circ);
+ SMARTLIST_FOREACH_BEGIN(circuits_pending_close, circuit_t *, circ) {
+ tor_assert(circ->marked_for_close);
+
+ /* Remove it from the circuit list. */
+ int idx = circ->global_circuitlist_idx;
+ smartlist_del(lst, idx);
+ if (idx < smartlist_len(lst)) {
+ circuit_t *replacement = smartlist_get(lst, idx);
+ replacement->global_circuitlist_idx = idx;
}
+ circ->global_circuitlist_idx = -1;
+
+ circuit_about_to_free(circ);
+ circuit_free(circ);
} SMARTLIST_FOREACH_END(circ);
+
+ smartlist_clear(circuits_pending_close);
}
/** Return the head of the global linked list of circuits. */
@@ -1703,6 +1719,39 @@ circuit_mark_for_close_, (circuit_t *circ, int reason, int line,
reason = END_CIRC_REASON_NONE;
}
+ circ->marked_for_close = line;
+ circ->marked_for_close_file = file;
+ circ->marked_for_close_reason = reason;
+ circ->marked_for_close_orig_reason = orig_reason;
+
+ if (!CIRCUIT_IS_ORIGIN(circ)) {
+ or_circuit_t *or_circ = TO_OR_CIRCUIT(circ);
+ if (or_circ->rend_splice) {
+ if (!or_circ->rend_splice->base_.marked_for_close) {
+ /* do this after marking this circuit, to avoid infinite recursion. */
+ circuit_mark_for_close(TO_CIRCUIT(or_circ->rend_splice), reason);
+ }
+ or_circ->rend_splice = NULL;
+ }
+ }
+
+ if (circuits_pending_close == NULL)
+ circuits_pending_close = smartlist_new();
+
+ smartlist_add(circuits_pending_close, circ);
+}
+
+/** Called immediately before freeing a marked circuit <b>circ</b>.
+ * Disconnects the circuit from other data structures, launches events
+ * as appropriate, and performs other housekeeping.
+ */
+static void
+circuit_about_to_free(circuit_t *circ)
+{
+
+ int reason = circ->marked_for_close_reason;
+ int orig_reason = circ->marked_for_close_orig_reason;
+
if (circ->state == CIRCUIT_STATE_ONIONSKIN_PENDING) {
onion_pending_remove(TO_OR_CIRCUIT(circ));
}
@@ -1726,6 +1775,7 @@ circuit_mark_for_close_, (circuit_t *circ, int reason, int line,
(circ->state == CIRCUIT_STATE_OPEN)?CIRC_EVENT_CLOSED:CIRC_EVENT_FAILED,
orig_reason);
}
+
if (circ->purpose == CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT) {
origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ);
int timed_out = (reason == END_CIRC_REASON_TIMEOUT);
@@ -1811,19 +1861,6 @@ circuit_mark_for_close_, (circuit_t *circ, int reason, int line,
ocirc->p_streams = NULL;
}
- circ->marked_for_close = line;
- circ->marked_for_close_file = file;
-
- if (!CIRCUIT_IS_ORIGIN(circ)) {
- or_circuit_t *or_circ = TO_OR_CIRCUIT(circ);
- if (or_circ->rend_splice) {
- if (!or_circ->rend_splice->base_.marked_for_close) {
- /* do this after marking this circuit, to avoid infinite recursion. */
- circuit_mark_for_close(TO_CIRCUIT(or_circ->rend_splice), reason);
- }
- or_circ->rend_splice = NULL;
- }
- }
}
/** Given a marked circuit <b>circ</b>, aggressively free its cell queues to
diff --git a/src/or/or.h b/src/or/or.h
index a80cd55..746e462 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -2892,6 +2892,14 @@ typedef struct circuit_t {
* where this circuit was marked.) */
const char *marked_for_close_file; /**< For debugging: in which file was this
* circuit marked for close? */
+ /** For what reason (See END_CIRC_REASON...) is this circuit being closed?
+ * This field is set in circuit_mark_for_close and used later in
+ * circuit_about_to_free. */
+ uint16_t marked_for_close_reason;
+ /** As marked_for_close_reason, but reflects the underlying reason for
+ * closing this circuit.
+ */
+ uint16_t marked_for_close_orig_reason;
/** Unique ID for measuring tunneled network status requests. */
uint64_t dirreq_id;
More information about the tor-commits
mailing list