diff --git a/changes/bug40834 b/changes/bug40834 new file mode 100644 index 0000000000..5c37e0c6a4 --- /dev/null +++ b/changes/bug40834 @@ -0,0 +1,5 @@ + o Major bugfixes (conflux): + - Fix a relay-side crash caused by side effects of the fix for bug + 40827. Reverts part of that fix that caused the crash and adds + additional log messages to help find the root cause. Fixes bug + 40834; bugfix on 0.4.8.3-rc. diff --git a/src/core/or/conflux.c b/src/core/or/conflux.c index eb004b3626..ab464f3446 100644 --- a/src/core/or/conflux.c +++ b/src/core/or/conflux.c @@ -467,8 +467,12 @@ conflux_decide_circ_for_send(conflux_t *cfx, * so these commands arrive in-order. */ if (!new_circ && relay_command != RELAY_COMMAND_DATA) { /* Curr leg should be set, because conflux_decide_next_circ() should - * have set it earlier. */ - tor_assert(cfx->curr_leg); + * have set it earlier. No BUG() here because the only caller BUG()s. */ + if (!cfx->curr_leg) { + log_warn(LD_BUG, "No current leg for conflux with relay command %d", + relay_command); + return NULL; + } return cfx->curr_leg->circ; } diff --git a/src/core/or/conflux_pool.c b/src/core/or/conflux_pool.c index 837c1da720..79fd6c1648 100644 --- a/src/core/or/conflux_pool.c +++ b/src/core/or/conflux_pool.c @@ -1509,49 +1509,6 @@ linked_update_stream_backpointers(circuit_t *circ) } } -/** This is called when this circuit is the last leg. - * - * The "on_circuit" pointer is nullified here so it is not given back to the - * conflux subsytem between the circuit mark for close step and actually - * freeing the circuit which is when streams are destroyed. - * - * Reason is that when the connection edge inbuf is packaged in - * connection_edge_package_raw_inbuf(), the on_circuit pointer is used and - * then passed on to conflux to learn which leg to use. However, if the circuit - * was marked prior but not yet freed, there are no more legs remaining which - * leads to a linked circuit being used without legs. No bueno. */ -static void -linked_detach_circuit(circuit_t *circ) -{ - tor_assert(circ); - tor_assert_nonfatal(circ->conflux); - - if (CIRCUIT_IS_ORIGIN(circ)) { - origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ); - tor_assert_nonfatal(circ->purpose == CIRCUIT_PURPOSE_CONFLUX_LINKED); - /* Iterate over stream list using next_stream pointer, until null */ - for (edge_connection_t *stream = ocirc->p_streams; stream; - stream = stream->next_stream) { - /* Update the circuit pointer of each stream */ - stream->on_circuit = NULL; - } - } else { - or_circuit_t *orcirc = TO_OR_CIRCUIT(circ); - /* Iterate over stream list using next_stream pointer, until null */ - for (edge_connection_t *stream = orcirc->n_streams; stream; - stream = stream->next_stream) { - /* Update the circuit pointer of each stream */ - stream->on_circuit = NULL; - } - /* Iterate over stream list using next_stream pointer, until null */ - for (edge_connection_t *stream = orcirc->resolving_streams; stream; - stream = stream->next_stream) { - /* Update the circuit pointer of each stream */ - stream->on_circuit = NULL; - } - } -} - /** Nullify all streams of the given circuit. */ static void linked_nullify_streams(circuit_t *circ) @@ -1592,7 +1549,6 @@ linked_circuit_closed(circuit_t *circ) if (CONFLUX_NUM_LEGS(circ->conflux) == 0) { /* Last leg, remove conflux object from linked set. */ linked_pool_del(circ->conflux->nonce, is_client); - linked_detach_circuit(circ); } else { /* If there are other circuits, update streams backpointers and * nullify the stream lists. We do not free those streams in circuit_free_. diff --git a/src/core/or/relay.c b/src/core/or/relay.c index 3af9435a76..893daa09fd 100644 --- a/src/core/or/relay.c +++ b/src/core/or/relay.c @@ -639,7 +639,8 @@ relay_send_command_from_edge_,(streamid_t stream_id, circuit_t *orig_circ, circ = conflux_decide_circ_for_send(orig_circ->conflux, orig_circ, relay_command); if (BUG(!circ)) { - log_warn(LD_BUG, "No circuit to send on for conflux"); + log_warn(LD_BUG, "No circuit to send for conflux for relay command %d, " + "called from %s:%d", relay_command, filename, lineno); circ = orig_circ; } else { /* Conflux circuits always send multiplexed relay commands to