More fixes/debugging attempts for 17659

src/or/circuituse.c

@@ -1986,6 +1986,7 @@ circuit_get_open_circ_or_launch(entry_connection_t *conn,
                  "No intro points for '%s': re-fetching service descriptor.",
                  safe_str_client(rend_data->onion_address));
         rend_client_refetch_v2_renddesc(rend_data);
+        connection_ap_mark_as_non_pending_circuit(conn);
         ENTRY_TO_CONN(conn)->state = AP_CONN_STATE_RENDDESC_WAIT;
         return 0;
       }

src/or/connection.c

@@ -1597,6 +1597,8 @@ connection_init_accepted_conn(connection_t *conn,
           break;
         case CONN_TYPE_AP_TRANS_LISTENER:
           TO_ENTRY_CONN(conn)->is_transparent_ap = 1;
+          /* XXXX028 -- is this correct still, with the addition of
+           * pending_entry_connections ? */
           conn->state = AP_CONN_STATE_CIRCUIT_WAIT;
           return connection_ap_process_transparent(TO_ENTRY_CONN(conn));
         case CONN_TYPE_AP_NATD_LISTENER:

src/or/connection_edge.c

@@ -771,7 +771,7 @@ connection_ap_rescan_and_attach_pending(void)
                "adding it.",
                pending_entry_connections);
       untried_pending_connections = 1;
-      smartlist_add(pending_entry_connections, entry_conn);
+      connection_ap_mark_as_pending_circuit(entry_conn);
     }
 
   } SMARTLIST_FOREACH_END(conn);
@@ -827,8 +827,11 @@ connection_ap_attach_pending(int retry)
         conn->type != CONN_TYPE_AP ||
         conn->state != AP_CONN_STATE_CIRCUIT_WAIT) {
       SMARTLIST_DEL_CURRENT(pending_entry_connections, entry_conn);
+      continue;
     }
 
+    tor_assert(conn->magic == ENTRY_CONNECTION_MAGIC);
+
   } SMARTLIST_FOREACH_END(entry_conn);
 
   untried_pending_connections = 0;
@@ -847,6 +850,7 @@ connection_ap_mark_as_pending_circuit_(entry_connection_t *entry_conn,
 {
   connection_t *conn = ENTRY_TO_CONN(entry_conn);
   tor_assert(conn->state == AP_CONN_STATE_CIRCUIT_WAIT);
+  tor_assert(conn->magic == ENTRY_CONNECTION_MAGIC);
   if (conn->marked_for_close)
     return;
 
@@ -866,6 +870,15 @@ connection_ap_mark_as_pending_circuit_(entry_connection_t *entry_conn,
   smartlist_add(pending_entry_connections, entry_conn);
 }
 
+/** Mark <b>entry_conn</b> as no longer waiting for a circuit. */
+void
+connection_ap_mark_as_non_pending_circuit(entry_connection_t *entry_conn)
+{
+  if (PREDICT_UNLIKELY(NULL == pending_entry_connections))
+    return;
+  smartlist_remove(pending_entry_connections, entry_conn);
+}
+
 /** Tell any AP streams that are waiting for a one-hop tunnel to
  * <b>failed_digest</b> that they are going to fail. */
 /* XXX024 We should get rid of this function, and instead attach
@@ -986,6 +999,7 @@ connection_ap_detach_retriable(entry_connection_t *conn,
     circuit_detach_stream(TO_CIRCUIT(circ),ENTRY_TO_EDGE_CONN(conn));
     connection_ap_mark_as_pending_circuit(conn);
   } else {
+    CONNECTION_AP_EXPECT_NONPENDING(conn);
     ENTRY_TO_CONN(conn)->state = AP_CONN_STATE_CONTROLLER_WAIT;
     circuit_detach_stream(TO_CIRCUIT(circ),ENTRY_TO_EDGE_CONN(conn));
   }
@@ -1038,6 +1052,7 @@ connection_ap_rewrite_and_attach_if_allowed(entry_connection_t *conn,
   const or_options_t *options = get_options();
 
   if (options->LeaveStreamsUnattached) {
+    CONNECTION_AP_EXPECT_NONPENDING(conn);
     ENTRY_TO_CONN(conn)->state = AP_CONN_STATE_CONTROLLER_WAIT;
     return 0;
   }
@@ -1689,6 +1704,7 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn,
      * Also, a fetch could have been requested if the onion address was not
      * found in the cache previously. */
     if (refetch_desc || !rend_client_any_intro_points_usable(entry)) {
+      connection_ap_mark_as_non_pending_circuit(conn);
       base_conn->state = AP_CONN_STATE_RENDDESC_WAIT;
       log_info(LD_REND, "Unknown descriptor %s. Fetching.",
           safe_str_client(rend_data->onion_address));

src/or/connection_edge.h

@@ -70,6 +70,14 @@ void connection_ap_mark_as_pending_circuit_(entry_connection_t *entry_conn,
                                            const char *file, int line);
 #define connection_ap_mark_as_pending_circuit(c) \
   connection_ap_mark_as_pending_circuit_((c), __FILE__, __LINE__)
+void connection_ap_mark_as_non_pending_circuit(entry_connection_t *entry_conn);
+#define CONNECTION_AP_EXPECT_NONPENDING(c) do {                         \
+    if (ENTRY_TO_CONN(c)->state == AP_CONN_STATE_CIRCUIT_WAIT) {        \
+      log_warn(LD_BUG, "At %s:%d: %p was unexpectedly in circuit_wait.", \
+               __FILE__, __LINE__, (c));                                \
+      connection_ap_mark_as_non_pending_circuit(c);                     \
+    }                                                                   \
+  } while (0)
 void connection_ap_fail_onehop(const char *failed_digest,
                                cpath_build_state_t *build_state);
 void circuit_discard_optional_exit_enclaves(extend_info_t *info);

src/or/control.c

@@ -3011,6 +3011,7 @@ handle_control_attachstream(control_connection_t *conn, uint32_t len,
     edge_conn->end_reason = 0;
     if (tmpcirc)
       circuit_detach_stream(tmpcirc, edge_conn);
+    CONNECTION_AP_EXPECT_NONPENDING(ap_conn);
     TO_CONN(edge_conn)->state = AP_CONN_STATE_CONTROLLER_WAIT;
   }
 

src/or/dnsserv.c

@@ -125,6 +125,7 @@ evdns_server_callback(struct evdns_server_request *req, void *data_)
   /* Make a new dummy AP connection, and attach the request to it. */
   entry_conn = entry_connection_new(CONN_TYPE_AP, AF_INET);
   conn = ENTRY_TO_EDGE_CONN(entry_conn);
+  CONNECTION_AP_EXPECT_NONPENDING(entry_conn);
   TO_CONN(conn)->state = AP_CONN_STATE_RESOLVE_WAIT;
   conn->is_dns_request = 1;
 
@@ -199,6 +200,7 @@ dnsserv_launch_request(const char *name, int reverse,
   /* Make a new dummy AP connection, and attach the request to it. */
   entry_conn = entry_connection_new(CONN_TYPE_AP, AF_INET);
   conn = ENTRY_TO_EDGE_CONN(entry_conn);
+  CONNECTION_AP_EXPECT_NONPENDING(entry_conn);
   conn->base_.state = AP_CONN_STATE_RESOLVE_WAIT;
 
   tor_addr_copy(&TO_CONN(conn)->addr, &control_conn->base_.addr);

src/or/relay.c

@@ -1304,6 +1304,7 @@ connection_edge_process_relay_cell_not_open(
              "Got 'connected' while not in state connect_wait. Dropping.");
       return 0;
     }
+    CONNECTION_AP_EXPECT_NONPENDING(entry_conn);
     conn->base_.state = AP_CONN_STATE_OPEN;
     log_info(LD_APP,"'connected' received for circid %u streamid %d "
              "after %d seconds.",

src/or/rendclient.c

@@ -173,6 +173,7 @@ rend_client_send_introduction(origin_circuit_t *introcirc,
       while ((conn = connection_get_by_type_state_rendquery(CONN_TYPE_AP,
                        AP_CONN_STATE_CIRCUIT_WAIT,
                        introcirc->rend_data->onion_address))) {
+        connection_ap_mark_as_non_pending_circuit(TO_ENTRY_CONN(conn));
         conn->state = AP_CONN_STATE_RENDDESC_WAIT;
       }
     }
@@ -1055,9 +1056,11 @@ rend_client_report_intro_point_failure(extend_info_t *failed_intro,
     rend_client_refetch_v2_renddesc(rend_query);
 
     /* move all pending streams back to renddesc_wait */
+    /* NOTE: We can now do this faster, if we use pending_entry_connections */
     while ((conn = connection_get_by_type_state_rendquery(CONN_TYPE_AP,
                                    AP_CONN_STATE_CIRCUIT_WAIT,
                                    rend_query->onion_address))) {
+      connection_ap_mark_as_non_pending_circuit(TO_ENTRY_CONN(conn));
       conn->state = AP_CONN_STATE_RENDDESC_WAIT;
     }