diff options
| -rw-r--r-- | changes/bug4655 | 10 | ||||
| -rw-r--r-- | src/or/circuituse.c | 53 | ||||
| -rw-r--r-- | src/or/circuituse.h | 1 | ||||
| -rw-r--r-- | src/or/rendclient.c | 7 |
4 files changed, 49 insertions, 22 deletions
diff --git a/changes/bug4655 b/changes/bug4655 new file mode 100644 index 000000000..b91d87123 --- /dev/null +++ b/changes/bug4655 @@ -0,0 +1,10 @@ + o Minor bugfixes: + + - If we can't attach streams to a rendezvous circuit when we + finish connecting to a hidden service, clear the rendezvous + circuit's stream-isolation state and try to attach streams + again. Previously, we cleared rendezvous circuits' isolation + state either too early (if they were freshly built) or not at + all (if they had been built earlier and were cannibalized). + Bugfix on 0.2.3.3-alpha; fixes bug 4655. + diff --git a/src/or/circuituse.c b/src/or/circuituse.c index bdff75cd5..ef4ac6faa 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -1004,7 +1004,6 @@ circuit_testing_failed(origin_circuit_t *circ, int at_last_hop) void circuit_has_opened(origin_circuit_t *circ) { - int can_try_clearing_isolation = 0, tried_clearing_isolation = 0; control_event_circuit_status(circ, CIRC_EVENT_BUILT, 0); /* Remember that this circuit has finished building. Now if we start @@ -1012,20 +1011,18 @@ circuit_has_opened(origin_circuit_t *circ) * to consider its build time. */ circ->has_opened = 1; - again: - switch (TO_CIRCUIT(circ)->purpose) { case CIRCUIT_PURPOSE_C_ESTABLISH_REND: rend_client_rendcirc_has_opened(circ); - /* XXXX We'd like to set something like "can_try_clearing_isolation" - * here, so that we can change the isolation of this circuit (and maybe - * its purpose too) if it turns out that we no longer have any streams - * that want to use it. But connection_ap_attach_pending() doesn't - * actually attach streams to a C_ESTABLISH_REND circuit-- streams - * don't get attached until the circuit is in C_REND_JOINED... so - * we can't clear isolation now. - */ + /* Start building an intro circ if we don't have one yet. */ connection_ap_attach_pending(); + /* This isn't a call to circuit_try_attaching_streams because a + * circuit in _C_ESTABLISH_REND state isn't connected to its + * hidden service yet, thus we can't attach streams to it yet, + * thus circuit_try_attaching_streams would always clear the + * circuit's isolation state. circuit_try_attaching_streams is + * called later, when the rend circ enters _C_REND_JOINED + * state. */ break; case CIRCUIT_PURPOSE_C_INTRODUCING: rend_client_introcirc_has_opened(circ); @@ -1033,8 +1030,7 @@ circuit_has_opened(origin_circuit_t *circ) case CIRCUIT_PURPOSE_C_GENERAL: /* Tell any AP connections that have been waiting for a new * circuit that one is ready. */ - can_try_clearing_isolation = 1; - connection_ap_attach_pending(); + circuit_try_attaching_streams(circ); break; case CIRCUIT_PURPOSE_S_ESTABLISH_INTRO: /* at Bob, waiting for introductions */ @@ -1051,12 +1047,15 @@ circuit_has_opened(origin_circuit_t *circ) * This won't happen in normal operation, but might happen if the * controller did it. Just let it slide. */ } +} +/** If the stream-isolation state of <b>circ</b> can be cleared, clear + * it. Return non-zero iff <b>circ</b>'s isolation state was cleared. */ +static int +circuit_try_clearing_isolation_state(origin_circuit_t *circ) +{ if (/* The circuit may have become non-open if it was cannibalized.*/ circ->_base.state == CIRCUIT_STATE_OPEN && - /* Only if the purpose is clearable, and only if we haven't tried - * to clear isolation yet, do we try. */ - can_try_clearing_isolation && !tried_clearing_isolation && /* If !isolation_values_set, there is nothing to clear. */ circ->isolation_values_set && /* It's not legal to clear a circuit's isolation info if it's ever had @@ -1066,8 +1065,26 @@ circuit_has_opened(origin_circuit_t *circ) * we didn't manage to attach any streams to it, then we can * and should clear it and try again. */ circuit_clear_isolation(circ); - tried_clearing_isolation = 1; - goto again; + return 1; + } else { + return 0; + } +} + +/** Called when a circuit becomes ready for streams to be attached to + * it. */ +void +circuit_try_attaching_streams(origin_circuit_t *circ) +{ + /* Attach streams to this circuit if we can. */ + connection_ap_attach_pending(); + + /* The call to circuit_try_clearing_isolation_state here will do + * nothing and return 0 if we didn't attach any streams to circ + * above. */ + if (circuit_try_clearing_isolation_state(circ)) { + /* Maybe *now* we can attach some streams to this circuit. */ + connection_ap_attach_pending(); } } diff --git a/src/or/circuituse.h b/src/or/circuituse.h index 9867fd820..bc11fe5d9 100644 --- a/src/or/circuituse.h +++ b/src/or/circuituse.h @@ -29,6 +29,7 @@ void reset_bandwidth_test(void); int circuit_enough_testing_circs(void); void circuit_has_opened(origin_circuit_t *circ); +void circuit_try_attaching_streams(origin_circuit_t *circ); void circuit_build_failed(origin_circuit_t *circ); /** Flag to set when a circuit should have only a single hop. */ diff --git a/src/or/rendclient.c b/src/or/rendclient.c index 5429b6c7e..8aae8c5cb 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -892,10 +892,9 @@ rend_client_receive_rendezvous(origin_circuit_t *circ, const uint8_t *request, onion_append_to_cpath(&circ->cpath, hop); circ->build_state->pending_final_cpath = NULL; /* prevent double-free */ - /* XXXX023 This is a pretty brute-force approach. It'd be better to - * attach only the connections that are waiting on this circuit, rather - * than trying to attach them all. See comments bug 743. */ - connection_ap_attach_pending(); + + circuit_try_attaching_streams(circ); + memset(keys, 0, sizeof(keys)); return 0; err: |