[or-cvs] r9482: Fix an XXXX012, and make circuits_pending_or_conns a static (in tor/trunk: . src/or)
nickm at seul.org
nickm at seul.org
Mon Feb 5 17:42:42 UTC 2007
Author: nickm
Date: 2007-02-05 12:42:40 -0500 (Mon, 05 Feb 2007)
New Revision: 9482
Modified:
tor/trunk/
tor/trunk/ChangeLog
tor/trunk/src/or/circuitbuild.c
tor/trunk/src/or/circuitlist.c
tor/trunk/src/or/connection_or.c
tor/trunk/src/or/control.c
tor/trunk/src/or/or.h
Log:
r11637 at catbus: nickm | 2007-02-05 12:41:51 -0500
Fix an XXXX012, and make circuits_pending_or_conns a static variable. In addition to cleaning up the code, this may also resolve Bug 386 if Roger has the right intuition there.
Property changes on: tor/trunk
___________________________________________________________________
svk:merge ticket from /tor/trunk [r11637] on 8246c3cf-6607-4228-993b-4d95d33730f1
Modified: tor/trunk/ChangeLog
===================================================================
--- tor/trunk/ChangeLog 2007-02-04 01:02:08 UTC (rev 9481)
+++ tor/trunk/ChangeLog 2007-02-05 17:42:40 UTC (rev 9482)
@@ -64,6 +64,8 @@
- Stop using the reserved ac_cv namespace in our configure script.
- Call stat() slightly less often; use fstat() when possible.
- Treat failure to parse resolv.conf as an error.
+ - Refactor the way we handle pending circuits when an OR connection
+ completes or fails, in an attempt to fix a rare crash bug.
o Major features:
- Weight directory requests by advertised bandwidth. Now we can
Modified: tor/trunk/src/or/circuitbuild.c
===================================================================
--- tor/trunk/src/or/circuitbuild.c 2007-02-04 01:02:08 UTC (rev 9481)
+++ tor/trunk/src/or/circuitbuild.c 2007-02-05 17:42:40 UTC (rev 9482)
@@ -388,8 +388,6 @@
return 0;
}
-extern smartlist_t *circuits_pending_or_conns;
-
/** Find any circuits that are waiting on <b>or_conn</b> to become
* open and get them to send their create cells forward.
*
@@ -398,25 +396,24 @@
void
circuit_n_conn_done(or_connection_t *or_conn, int status)
{
- smartlist_t *changed_circs;
+ smartlist_t *pending_circs;
int err_reason = 0;
log_debug(LD_CIRC,"or_conn to %s, status=%d",
or_conn->nickname ? or_conn->nickname : "NULL", status);
- if (!circuits_pending_or_conns)
- return;
+ pending_circs = smartlist_create();
+ circuit_get_all_pending_on_or_conn(pending_circs, or_conn);
- changed_circs = smartlist_create();
-
- SMARTLIST_FOREACH(circuits_pending_or_conns, circuit_t *, circ,
- {
- if (circ->marked_for_close)
- continue;
- tor_assert(circ->state == CIRCUIT_STATE_OR_WAIT);
- if (!circ->n_conn &&
- !memcmp(or_conn->identity_digest, circ->n_conn_id_digest,
- DIGEST_LEN)) {
+ SMARTLIST_FOREACH(pending_circs, circuit_t *, circ,
+ {
+ /* This check is redundant wrt get_all_pending_on_or_conn, but I'm
+ * leaving them in in case it's possible for the status of a circuit to
+ * change as we're going down the list. */
+ if (circ->marked_for_close || circ->n_conn ||
+ circ->state != CIRCUIT_STATE_OR_WAIT ||
+ memcmp(or_conn->identity_digest, circ->n_conn_id_digest, DIGEST_LEN))
+ continue;
if (!status) { /* or_conn failed; close circ */
log_info(LD_CIRC,"or_conn failed. Closing circ.");
circuit_mark_for_close(circ, END_CIRC_REASON_OR_CONN_CLOSED);
@@ -445,18 +442,11 @@
continue;
}
tor_free(circ->onionskin);
- /* We don't want to change circ's state here, since the act
- * of doing that modifies the circuits_pending_or_conns list
- * that we're looping through right now. So collect a list of
- * circs to change their state when we're done. */
- smartlist_add(changed_circs, circ);
+ circuit_set_state(circ, CIRCUIT_STATE_OPEN);
}
- }
- });
+ });
- SMARTLIST_FOREACH(changed_circs, circuit_t *, circ,
- circuit_set_state(circ, CIRCUIT_STATE_OPEN));
- smartlist_free(changed_circs);
+ smartlist_free(pending_circs);
}
/** Find a new circid that isn't currently in use on the circ->n_conn
Modified: tor/trunk/src/or/circuitlist.c
===================================================================
--- tor/trunk/src/or/circuitlist.c 2007-02-04 01:02:08 UTC (rev 9481)
+++ tor/trunk/src/or/circuitlist.c 2007-02-05 17:42:40 UTC (rev 9482)
@@ -21,7 +21,7 @@
circuit_t *global_circuitlist=NULL;
/** A list of all the circuits in CIRCUIT_STATE_OR_WAIT. */
-smartlist_t *circuits_pending_or_conns=NULL;
+static smartlist_t *circuits_pending_or_conns=NULL;
static void circuit_free(circuit_t *circ);
static void circuit_free_cpath(crypt_path_t *cpath);
@@ -165,15 +165,14 @@
tor_assert(circ);
if (state == circ->state)
return;
+ if (!circuits_pending_or_conns)
+ circuits_pending_or_conns = smartlist_create();
if (circ->state == CIRCUIT_STATE_OR_WAIT) {
/* remove from waiting-circuit list. */
- if (circuits_pending_or_conns)
- smartlist_remove(circuits_pending_or_conns, circ);
+ smartlist_remove(circuits_pending_or_conns, circ);
}
if (state == CIRCUIT_STATE_OR_WAIT) {
/* add to waiting-circuit list. */
- if (!circuits_pending_or_conns)
- circuits_pending_or_conns = smartlist_create();
smartlist_add(circuits_pending_or_conns, circ);
}
circ->state = state;
@@ -194,6 +193,45 @@
}
}
+/** Append to <b>out</b> the number of circuits in state OR_WAIT, waiting for
+ * the given connection. */
+void
+circuit_get_all_pending_on_or_conn(smartlist_t *out, or_connection_t *or_conn)
+{
+ tor_assert(out);
+ tor_assert(or_conn);
+
+ if (!circuits_pending_or_conns)
+ return;
+
+ SMARTLIST_FOREACH(circuits_pending_or_conns, circuit_t *, circ,
+ {
+ if (circ->marked_for_close)
+ continue;
+ tor_assert(circ->state == CIRCUIT_STATE_OR_WAIT);
+ if (!circ->n_conn &&
+ !memcmp(or_conn->identity_digest, circ->n_conn_id_digest,
+ DIGEST_LEN)) {
+ smartlist_add(out, circ);
+ }
+ });
+}
+
+/** Return the number of circuits in state OR_WAIT, waiting for the given
+ * connection. */
+int
+circuit_count_pending_on_or_conn(or_connection_t *or_conn)
+{
+ int cnt;
+ smartlist_t *sl = smartlist_create();
+ circuit_get_all_pending_on_or_conn(sl, or_conn);
+ cnt = smartlist_len(sl);
+ smartlist_free(sl);
+ log_debug(LD_CIRC,"or_conn to %s, %d pending circs",
+ or_conn->nickname ? or_conn->nickname : "NULL", cnt);
+ return cnt;
+}
+
/** Detach from the global circuit list, and deallocate, all
* circuits that have been marked for close.
*/
Modified: tor/trunk/src/or/connection_or.c
===================================================================
--- tor/trunk/src/or/connection_or.c 2007-02-04 01:02:08 UTC (rev 9481)
+++ tor/trunk/src/or/connection_or.c 2007-02-05 17:42:40 UTC (rev 9482)
@@ -779,37 +779,6 @@
return 0;
}
-/* XXXX012 This global is getting _too_ global. -NM */
-extern smartlist_t *circuits_pending_or_conns;
-
-/** Count number of pending circs on an or_conn */
-int
-connection_or_count_pending_circs(or_connection_t *or_conn)
-{
- int cnt = 0;
-
- if (!circuits_pending_or_conns)
- return 0;
-
- tor_assert(or_conn);
-
- SMARTLIST_FOREACH(circuits_pending_or_conns, circuit_t *, circ,
- {
- if (circ->marked_for_close)
- continue;
- tor_assert(circ->state == CIRCUIT_STATE_OR_WAIT);
- if (!circ->n_conn &&
- !memcmp(or_conn->identity_digest, circ->n_conn_id_digest,
- DIGEST_LEN)) {
- cnt++;
- }
- });
-
- log_debug(LD_CIRC,"or_conn to %s, %d pending circs",
- or_conn->nickname ? or_conn->nickname : "NULL", cnt);
- return cnt;
-}
-
/** DOCDOC */
#define BUF_FULLNESS_THRESHOLD (128*1024)
/** DOCDOC */
Modified: tor/trunk/src/or/control.c
===================================================================
--- tor/trunk/src/or/control.c 2007-02-04 01:02:08 UTC (rev 9481)
+++ tor/trunk/src/or/control.c 2007-02-05 17:42:40 UTC (rev 9482)
@@ -3356,7 +3356,7 @@
log_warn(LD_BUG, "Unrecognized status code %d", (int)tp);
return 0;
}
- ncircs = connection_or_count_pending_circs(conn);
+ ncircs = circuit_count_pending_on_or_conn(conn);
ncircs += conn->n_circuits;
if (ncircs && (tp == OR_CONN_EVENT_FAILED || tp == OR_CONN_EVENT_CLOSED)) {
tor_snprintf(ncircs_buf, sizeof(ncircs_buf), "%sNCIRCS=%d",
Modified: tor/trunk/src/or/or.h
===================================================================
--- tor/trunk/src/or/or.h 2007-02-04 01:02:08 UTC (rev 9481)
+++ tor/trunk/src/or/or.h 2007-02-05 17:42:40 UTC (rev 9482)
@@ -1989,6 +1989,9 @@
void _circuit_mark_for_close(circuit_t *circ, int reason,
int line, const char *file);
int circuit_get_cpath_len(origin_circuit_t *circ);
+void circuit_get_all_pending_on_or_conn(smartlist_t *out,
+ or_connection_t *or_conn);
+int circuit_count_pending_on_or_conn(or_connection_t *or_conn);
#define circuit_mark_for_close(c, reason) \
_circuit_mark_for_close((c), (reason), __LINE__, _SHORT_FILE_)
@@ -2250,7 +2253,6 @@
or_connection_t *conn);
int connection_or_send_destroy(uint16_t circ_id, or_connection_t *conn,
int reason);
-int connection_or_count_pending_circs(or_connection_t *or_conn);
/********************************* control.c ***************************/
More information about the tor-commits
mailing list