Still more code to make sure we send the right number and kind of RELAY END cells

svn:r3723

doc/TODO

@@ -72,8 +72,8 @@ R . HTTPS proxy for OR CONNECT stuff. (For outgoing SSL connections to
   o Changes for forward compatibility
     o If a version is later than the last in its series, but a version
       in the next series is recommended, that doesn't mean it's bad.
-N . Do end reasons better
-    . Start using RESOURCELIMIT more.
+  o Do end reasons better
+    o Start using RESOURCELIMIT more.
     o Try to use MISC a lot less.
       o bug: if the exit node fails to create a socket (e.g. because it
         has too many open), we will get a generic stream end response.
@@ -82,11 +82,11 @@ N . Do end reasons better
         relay end feature.
     o Realize that unrecognized end reasons are probably features rather than
       bugs. (backport to 009x)
-    - Check for anyplace where we can close an edge connection without
+    o Push the work of sending the end cell deeper into package_raw_inbuf.
+      (Turns out, if package_raw_inbuf fails, it *can't* send an end cell.)
+    o Check for any place where we can close an edge connection without
       sending an end; see if we should send an end.
-    - Get some kind of sane return code from package_raw_inbuf, or maybe
-      push the work of sending the end cell deeper into package_raw_inbuf.
-N . Feed end reason back into SOCK5 as reasonable.
+N   . Feed end reason back into SOCK5 as reasonable.
 R o cache .foo.exit names better, or differently, or not.
 N - make !advertised_server_mode() ORs fetch dirs less often.
 N - Clean up NT service code even more.  Document it. Enable it by default.

src/or/circuitlist.c

@@ -396,8 +396,12 @@ int _circuit_mark_for_close(circuit_t *circ) {
   while (circ->resolving_streams) {
     conn = circ->resolving_streams;
     circ->resolving_streams = conn->next_stream;
-    if (!conn->marked_for_close)
+    if (!conn->marked_for_close) {
+      /* The other side will see a DESTROY, and infer that the connections
+       * are closing because the circuit is getting torn down.  No need
+       * to send an end cell*/
       connection_mark_for_close(conn);
+    }
   }
   if (circ->p_conn)
     connection_send_destroy(circ->p_circ_id, circ->p_conn);

src/or/connection_edge.c

@@ -78,8 +78,7 @@ int connection_edge_process_inbuf(connection_t *conn, int package_partial) {
     case AP_CONN_STATE_OPEN:
     case EXIT_CONN_STATE_OPEN:
       if (connection_edge_package_raw_inbuf(conn, package_partial) < 0) {
-        /* XXXX We can't tell *why* package failed. -NM */
-        connection_edge_end(conn, END_STREAM_REASON_MISC, conn->cpath_layer);
+        /* (We already sent an end cell if possible) */
         connection_mark_for_close(conn);
         return -1;
       }
@@ -275,6 +274,7 @@ void connection_ap_expire_beginning(void) {
     if (!circ) { /* it's vanished? */
       log_fn(LOG_INFO,"Conn is waiting (address %s), but lost its circ.",
              conn->socks_request->address);
+      conn->has_sent_end = 1; /* No circuit to receive end cell. */
       connection_mark_for_close(conn);
       continue;
     }

src/or/main.c

@@ -358,6 +358,8 @@ conn_read_callback(int fd, short event, void *_conn)
       tor_assert(0);
 #endif
 #endif
+      if (CONN_IS_EDGE(conn))
+        connection_edge_end_errno(conn, conn->cpath_layer);
       connection_mark_for_close(conn);
     }
   }

src/or/relay.c

@@ -683,8 +683,7 @@ connection_edge_process_relay_cell_not_open(
     conn->socks_request->has_finished = 1;
     /* handle anything that might have queued */
     if (connection_edge_package_raw_inbuf(conn, 1) < 0) {
-      /* XXXX we can't tell why package failed. -NM */
-      connection_edge_end(conn, END_STREAM_REASON_MISC, conn->cpath_layer);
+      /* (We already sent an end cell if possible) */
       connection_mark_for_close(conn);
       return 0;
     }
@@ -903,7 +902,12 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
       conn->package_window += STREAMWINDOW_INCREMENT;
       log_fn(LOG_DEBUG,"stream-level sendme, packagewindow now %d.", conn->package_window);
       connection_start_reading(conn);
-      connection_edge_package_raw_inbuf(conn, 1); /* handle whatever might still be on the inbuf */
+      /* handle whatever might still be on the inbuf */
+      if (connection_edge_package_raw_inbuf(conn, 1) < 0) {
+        /* (We already sent an end cell if possible) */
+        connection_mark_for_close(conn);
+        return 0;
+      }
       return 0;
     case RELAY_COMMAND_RESOLVE:
       if (layer_hint) {
@@ -952,7 +956,8 @@ uint64_t stats_n_data_bytes_received = 0;
  * and the appropriate package windows aren't empty, grab a cell
  * and send it down the circuit.
  *
- * Return -1 if conn should be marked for close, else return 0.
+ * Return -1 (and send a RELAY_END cell if necessary) if conn should
+ * be marked for close, else return 0.
  */
 int connection_edge_package_raw_inbuf(connection_t *conn, int package_partial) {
   size_t amount_to_process, length;
@@ -966,7 +971,7 @@ repeat_connection_edge_package_raw_inbuf:
 
   circ = circuit_get_by_conn(conn);
   if (!circ) {
-    log_fn(LOG_INFO,"conn has no circuits! Closing.");
+    log_fn(LOG_INFO,"conn has no circuit! Closing.");
     return -1;
   }
 
@@ -1006,7 +1011,8 @@ repeat_connection_edge_package_raw_inbuf:
 
   if (connection_edge_send_command(conn, circ, RELAY_COMMAND_DATA,
                                    payload, length, conn->cpath_layer) < 0)
-    return 0; /* circuit is closed, don't continue */
+    /* circuit got marked for close, don't continue, don't need to mark conn */
+    return 0;
 
   if (!conn->cpath_layer) { /* non-rendezvous exit */
     tor_assert(circ->package_window > 0);
@@ -1089,7 +1095,11 @@ circuit_resume_edge_reading_helper(connection_t *conn,
         (layer_hint && conn->package_window > 0 && conn->cpath_layer == layer_hint)) {
       connection_start_reading(conn);
       /* handle whatever might still be on the inbuf */
-      connection_edge_package_raw_inbuf(conn, 1);
+      if (connection_edge_package_raw_inbuf(conn, 1)<0) {
+        /* (We already sent an end cell if possible) */
+        connection_mark_for_close(conn);
+        continue;
+      }
 
       /* If the circuit won't accept any more data, return without looking
        * at any more of the streams. Any connections that should be stopped