[or-cvs] r13394: Initial attempts to track down bug 600, and refactor possibl (in tor/trunk: . src/or)
nickm at seul.org
nickm at seul.org
Tue Feb 5 23:20:50 UTC 2008
Author: nickm
Date: 2008-02-05 18:20:49 -0500 (Tue, 05 Feb 2008)
New Revision: 13394
Modified:
tor/trunk/
tor/trunk/ChangeLog
tor/trunk/src/or/circuitbuild.c
tor/trunk/src/or/circuitlist.c
tor/trunk/src/or/command.c
tor/trunk/src/or/cpuworker.c
tor/trunk/src/or/onion.c
tor/trunk/src/or/or.h
Log:
r17930 at catbus: nickm | 2008-02-05 18:20:40 -0500
Initial attempts to track down bug 600, and refactor possibly offending code. 1) complain early if circuit state is set to OPEN when an onionskin is pending. 2) refactor onionskin field into one only used when n_conn is pending, and a separate onionskin field waiting for attention by a cpuworker. This might even fix the bug. More likely, it will make it fail with a more useful core.
Property changes on: tor/trunk
___________________________________________________________________
svk:merge ticket from /tor/trunk [r17930] on 8246c3cf-6607-4228-993b-4d95d33730f1
Modified: tor/trunk/ChangeLog
===================================================================
--- tor/trunk/ChangeLog 2008-02-05 23:20:44 UTC (rev 13393)
+++ tor/trunk/ChangeLog 2008-02-05 23:20:49 UTC (rev 13394)
@@ -42,7 +42,15 @@
port fails because we have overrun the limit on the number of
connections, tell the controller that the request has failed.
+ o Code simplifications and refactoring:
+ - Remove some needless generality from cpuworker code, for improved
+ type-safety.
+ - Stop overloading the circuit_t.onionskin field for both "onionskin
+ from a CREATE cell that we are waiting for a cpuworker to be
+ assigned" and "onionskin from an EXTEND cell that we are going to
+ send to an OR as soon as we are connected".
+
Changes in version 0.2.0.18-alpha - 2008-01-25
o New directory authorities:
- Set up dannenberg (run by CCC) as the sixth v3 directory
Modified: tor/trunk/src/or/circuitbuild.c
===================================================================
--- tor/trunk/src/or/circuitbuild.c 2008-02-05 23:20:44 UTC (rev 13393)
+++ tor/trunk/src/or/circuitbuild.c 2008-02-05 23:20:49 UTC (rev 13394)
@@ -461,12 +461,13 @@
}
} else {
/* pull the create cell out of circ->onionskin, and send it */
- tor_assert(circ->onionskin);
- if (circuit_deliver_create_cell(circ,CELL_CREATE,circ->onionskin)<0) {
+ tor_assert(circ->n_conn_onionskin);
+ if (circuit_deliver_create_cell(circ,CELL_CREATE,
+ circ->n_conn_onionskin)<0) {
circuit_mark_for_close(circ, END_CIRC_REASON_RESOURCELIMIT);
continue;
}
- tor_free(circ->onionskin);
+ tor_free(circ->n_conn_onionskin);
circuit_set_state(circ, CIRCUIT_STATE_OPEN);
}
});
@@ -757,8 +758,8 @@
log_info(LD_CIRC|LD_OR,"Next router (%s:%d) not connected. Connecting.",
tmpbuf, circ->n_port);
- circ->onionskin = tor_malloc(ONIONSKIN_CHALLENGE_LEN);
- memcpy(circ->onionskin, onionskin, ONIONSKIN_CHALLENGE_LEN);
+ circ->n_conn_onionskin = tor_malloc(ONIONSKIN_CHALLENGE_LEN);
+ memcpy(circ->n_conn_onionskin, onionskin, ONIONSKIN_CHALLENGE_LEN);
circuit_set_state(circ, CIRCUIT_STATE_OR_WAIT);
/* imprint the circuit with its future n_conn->id */
Modified: tor/trunk/src/or/circuitlist.c
===================================================================
--- tor/trunk/src/or/circuitlist.c 2008-02-05 23:20:44 UTC (rev 13393)
+++ tor/trunk/src/or/circuitlist.c 2008-02-05 23:20:49 UTC (rev 13394)
@@ -200,6 +200,8 @@
/* add to waiting-circuit list. */
smartlist_add(circuits_pending_or_conns, circ);
}
+ if (state == CIRCUIT_STATE_OPEN)
+ tor_assert(!circ->n_conn_onionskin);
circ->state = state;
}
@@ -413,8 +415,6 @@
other->rend_splice = NULL;
}
- tor_free(circ->onionskin);
-
/* remove from map. */
circuit_set_p_circid_orconn(ocirc, 0, NULL);
@@ -423,6 +423,8 @@
cell_queue_clear(ô->p_conn_cells);
}
+ tor_free(circ->n_conn_onionskin);
+
/* Remove from map. */
circuit_set_n_circid_orconn(circ, 0, NULL);
@@ -1162,7 +1164,7 @@
tor_assert(c->deliver_window >= 0);
tor_assert(c->package_window >= 0);
if (c->state == CIRCUIT_STATE_OPEN) {
- tor_assert(!c->onionskin);
+ tor_assert(!c->n_conn_onionskin);
if (or_circ) {
tor_assert(or_circ->n_crypto);
tor_assert(or_circ->p_crypto);
Modified: tor/trunk/src/or/command.c
===================================================================
--- tor/trunk/src/or/command.c 2008-02-05 23:20:44 UTC (rev 13393)
+++ tor/trunk/src/or/command.c 2008-02-05 23:20:49 UTC (rev 13394)
@@ -262,11 +262,11 @@
circ->_base.purpose = CIRCUIT_PURPOSE_OR;
circuit_set_state(TO_CIRCUIT(circ), CIRCUIT_STATE_ONIONSKIN_PENDING);
if (cell->command == CELL_CREATE) {
- circ->_base.onionskin = tor_malloc(ONIONSKIN_CHALLENGE_LEN);
- memcpy(circ->_base.onionskin, cell->payload, ONIONSKIN_CHALLENGE_LEN);
+ char *onionskin = tor_malloc(ONIONSKIN_CHALLENGE_LEN);
+ memcpy(onionskin, cell->payload, ONIONSKIN_CHALLENGE_LEN);
/* hand it off to the cpuworkers, and then return. */
- if (assign_to_cpuworker(NULL, CPUWORKER_TASK_ONION, circ) < 0) {
+ if (assign_onionskin_to_cpuworker(NULL, circ, onionskin) < 0) {
log_warn(LD_GENERAL,"Failed to hand off onionskin. Closing.");
circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_INTERNAL);
return;
Modified: tor/trunk/src/or/cpuworker.c
===================================================================
--- tor/trunk/src/or/cpuworker.c 2008-02-05 23:20:44 UTC (rev 13393)
+++ tor/trunk/src/or/cpuworker.c 2008-02-05 23:20:49 UTC (rev 13394)
@@ -387,15 +387,16 @@
process_pending_task(connection_t *cpuworker)
{
or_circuit_t *circ;
+ char *onionskin = NULL;
tor_assert(cpuworker);
/* for now only process onion tasks */
- circ = onion_next_task();
+ circ = onion_next_task(&onionskin);
if (!circ)
return;
- if (assign_to_cpuworker(cpuworker, CPUWORKER_TASK_ONION, circ) < 0)
+ if (assign_onionskin_to_cpuworker(cpuworker, circ, onionskin))
log_warn(LD_OR,"assign_to_cpuworker failed. Ignoring.");
}
@@ -433,28 +434,22 @@
/** If cpuworker is defined, assert that he's idle, and use him. Else,
* look for an idle cpuworker and use him. If none idle, queue task onto
* the pending onion list and return.
- * If question_type is CPUWORKER_TASK_ONION then task is a circ.
- * No other question_types are allowed.
+ * DOCDOC this function is now less general
*/
int
-assign_to_cpuworker(connection_t *cpuworker, uint8_t question_type,
- void *task)
+assign_onionskin_to_cpuworker(connection_t *cpuworker,
+ or_circuit_t *circ, char *onionskin)
{
- or_circuit_t *circ;
+ char qbuf[1];
char tag[TAG_LEN];
- tor_assert(question_type == CPUWORKER_TASK_ONION);
-
cull_wedged_cpuworkers();
spawn_enough_cpuworkers();
- if (question_type == CPUWORKER_TASK_ONION) {
- circ = task;
- tor_assert(circ->_base.onionskin);
-
+ if (1) {
if (num_cpuworkers_busy == num_cpuworkers) {
log_debug(LD_OR,"No idle cpuworkers. Queuing.");
- if (onion_pending_add(circ) < 0)
+ if (onion_pending_add(circ, onionskin) < 0)
return -1;
return 0;
}
@@ -467,6 +462,7 @@
if (!circ->p_conn) {
log_info(LD_OR,"circ->p_conn gone. Failing circ.");
+ tor_free(onionskin);
return -1;
}
tag_pack(tag, circ->p_conn->_base.addr, circ->p_conn->_base.port,
@@ -479,11 +475,11 @@
cpuworker->timestamp_lastwritten = time(NULL);
num_cpuworkers_busy++;
- connection_write_to_buf((char*)&question_type, 1, cpuworker);
+ qbuf[0] = CPUWORKER_TASK_ONION;
+ connection_write_to_buf(qbuf, 1, cpuworker);
connection_write_to_buf(tag, sizeof(tag), cpuworker);
- connection_write_to_buf(circ->_base.onionskin, ONIONSKIN_CHALLENGE_LEN,
- cpuworker);
- tor_free(circ->_base.onionskin);
+ connection_write_to_buf(onionskin, ONIONSKIN_CHALLENGE_LEN, cpuworker);
+ tor_free(onionskin);
}
return 0;
}
Modified: tor/trunk/src/or/onion.c
===================================================================
--- tor/trunk/src/or/onion.c 2008-02-05 23:20:44 UTC (rev 13393)
+++ tor/trunk/src/or/onion.c 2008-02-05 23:20:49 UTC (rev 13394)
@@ -19,6 +19,7 @@
* to process a waiting onion handshake. */
typedef struct onion_queue_t {
or_circuit_t *circ;
+ char *onionskin;
time_t when_added;
struct onion_queue_t *next;
} onion_queue_t;
@@ -37,13 +38,14 @@
* if ol_list is too long, in which case do nothing and return -1.
*/
int
-onion_pending_add(or_circuit_t *circ)
+onion_pending_add(or_circuit_t *circ, char *onionskin)
{
onion_queue_t *tmp;
time_t now = time(NULL);
tmp = tor_malloc_zero(sizeof(onion_queue_t));
tmp->circ = circ;
+ tmp->onionskin = onionskin;
tmp->when_added = now;
if (!ol_tail) {
@@ -86,7 +88,7 @@
* NULL if the list is empty.
*/
or_circuit_t *
-onion_next_task(void)
+onion_next_task(char **onionskin_out)
{
or_circuit_t *circ;
@@ -97,6 +99,8 @@
tor_assert(ol_list->circ->p_conn); /* make sure it's still valid */
tor_assert(ol_length > 0);
circ = ol_list->circ;
+ *onionskin_out = ol_list->onionskin;
+ ol_list->onionskin = NULL; /* prevent free. */
onion_pending_remove(ol_list->circ);
return circ;
}
@@ -139,6 +143,7 @@
/* now victim points to the element that needs to be removed */
+ tor_free(victim->onionskin);
tor_free(victim);
}
@@ -448,6 +453,7 @@
while (ol_list) {
onion_queue_t *victim = ol_list;
ol_list = victim->next;
+ tor_free(victim->onionskin);
tor_free(victim);
}
ol_list = ol_tail = NULL;
Modified: tor/trunk/src/or/or.h
===================================================================
--- tor/trunk/src/or/or.h 2008-02-05 23:20:44 UTC (rev 13393)
+++ tor/trunk/src/or/or.h 2008-02-05 23:20:49 UTC (rev 13394)
@@ -1775,11 +1775,10 @@
* more. */
int deliver_window;
- /** For storage while passing to cpuworker (state
- * CIRCUIT_STATE_ONIONSKIN_PENDING), or while n_conn is pending
+ /** For storage while n_conn is pending
* (state CIRCUIT_STATE_OR_WAIT). When defined, it is always
* length ONIONSKIN_CHALLENGE_LEN. */
- char *onionskin;
+ char *n_conn_onionskin;
time_t timestamp_created; /**< When was this circuit created? */
time_t timestamp_dirty; /**< When the circuit was first used, or 0 if the
@@ -2979,8 +2978,9 @@
int connection_cpu_finished_flushing(connection_t *conn);
int connection_cpu_reached_eof(connection_t *conn);
int connection_cpu_process_inbuf(connection_t *conn);
-int assign_to_cpuworker(connection_t *cpuworker, uint8_t question_type,
- void *task);
+int assign_onionskin_to_cpuworker(connection_t *cpuworker,
+ or_circuit_t *circ,
+ char *onionskin);
/********************************* directory.c ***************************/
@@ -3398,8 +3398,8 @@
/********************************* onion.c ***************************/
-int onion_pending_add(or_circuit_t *circ);
-or_circuit_t *onion_next_task(void);
+int onion_pending_add(or_circuit_t *circ, char *onionskin);
+or_circuit_t *onion_next_task(char **onionskin_out);
void onion_pending_remove(or_circuit_t *circ);
int onion_skin_create(crypto_pk_env_t *router_key,
More information about the tor-commits
mailing list