Another try at fixing 17752

I believe that the final SMARTLIST_DEL_CURRENT was sometimes
double-removing items that had already been removed by
connection_mark_unattached_ap or
connection_ap_handshake_attach_circuit().

The fix here is to prevent iteration over the list that other
functions might be modifying.
This commit is contained in:
Nick Mathewson 2015-12-17 12:28:40 -05:00
parent 54d9632cdd
commit f1be33fc00

View File

@ -800,20 +800,23 @@ connection_ap_attach_pending(int retry)
if (untried_pending_connections == 0 && !retry) if (untried_pending_connections == 0 && !retry)
return; return;
SMARTLIST_FOREACH_BEGIN(pending_entry_connections, /* Don't allow modifications to pending_entry_connections while we are
* iterating over it. */
smartlist_t *pending = pending_entry_connections;
pending_entry_connections = smartlist_new();
SMARTLIST_FOREACH_BEGIN(pending,
entry_connection_t *, entry_conn) { entry_connection_t *, entry_conn) {
connection_t *conn = ENTRY_TO_CONN(entry_conn); connection_t *conn = ENTRY_TO_CONN(entry_conn);
tor_assert(conn && entry_conn); tor_assert(conn && entry_conn);
if (conn->marked_for_close) { if (conn->marked_for_close) {
UNMARK(); UNMARK();
SMARTLIST_DEL_CURRENT(pending_entry_connections, entry_conn);
continue; continue;
} }
if (conn->magic != ENTRY_CONNECTION_MAGIC) { if (conn->magic != ENTRY_CONNECTION_MAGIC) {
log_warn(LD_BUG, "%p has impossible magic value %u.", log_warn(LD_BUG, "%p has impossible magic value %u.",
entry_conn, (unsigned)conn->magic); entry_conn, (unsigned)conn->magic);
UNMARK(); UNMARK();
SMARTLIST_DEL_CURRENT(pending_entry_connections, entry_conn);
continue; continue;
} }
if (conn->state != AP_CONN_STATE_CIRCUIT_WAIT) { if (conn->state != AP_CONN_STATE_CIRCUIT_WAIT) {
@ -822,7 +825,6 @@ connection_ap_attach_pending(int retry)
entry_conn, entry_conn,
conn_state_to_string(conn->type, conn->state)); conn_state_to_string(conn->type, conn->state));
UNMARK(); UNMARK();
SMARTLIST_DEL_CURRENT(pending_entry_connections, entry_conn);
continue; continue;
} }
@ -832,18 +834,19 @@ connection_ap_attach_pending(int retry)
END_STREAM_REASON_CANT_ATTACH); END_STREAM_REASON_CANT_ATTACH);
} }
if (conn->marked_for_close || if (! conn->marked_for_close &&
conn->type != CONN_TYPE_AP || conn->type == CONN_TYPE_AP &&
conn->state != AP_CONN_STATE_CIRCUIT_WAIT) { conn->state == AP_CONN_STATE_CIRCUIT_WAIT) {
UNMARK(); if (!smartlist_contains(pending_entry_connections, entry_conn)) {
SMARTLIST_DEL_CURRENT(pending_entry_connections, entry_conn); smartlist_add(pending_entry_connections, entry_conn);
continue; continue;
} }
}
tor_assert(conn->magic == ENTRY_CONNECTION_MAGIC); UNMARK();
} SMARTLIST_FOREACH_END(entry_conn); } SMARTLIST_FOREACH_END(entry_conn);
smartlist_free(pending);
untried_pending_connections = 0; untried_pending_connections = 0;
} }