From: Nick Mathewson <nickm@torproject.org>
Date: Fri, 25 Mar 2011 21:57:15 +0000 (-0400)
Subject: Fix handling of StreamID exhaustion.
X-Git-Tag: tor-0.2.2.25-alpha~19^2^2~3
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=432734279d3688fafb466a23f43585ff509ff693;p=thirdparty%2Ftor.git

Fix handling of StreamID exhaustion.

Since svn r1475/git 5b6099e8 in tor-0.0.6, we have responded to an
exhaustion of all 65535 stream IDs on a circuit by marking that
circuit for close.  That's not the right response.  Instead, we
should mark the circuit as "too dirty for new circuits".

Of course in reality this isn't really right either.  If somebody
has managed to cram 65535 streams onto a circuit, the circuit is
probably not going to work well for any of those streams, so maybe
we should be limiting the number of streams on an origin circuit
concurrently.

Also, closing the stream in this case is probably the wrong thing to
do as well, but fixing that can also wait.
---

diff --git a/changes/full_ap_circuits b/changes/full_ap_circuits
new file mode 100644
index 0000000000..379a1a1b73
--- /dev/null
+++ b/changes/full_ap_circuits
@@ -0,0 +1,6 @@
+  o Minor bugfixes
+    - When a client finds that an origin circuit has run out of 16-bit
+      stream IDs, we now mark it as unusable for new streams.
+      Previously, we would try to close the entire circuit.  Bugfix on
+      Tor version 0.0.6.
+
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index af0cfbe14c..72e2c8a409 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -2164,9 +2164,14 @@ connection_ap_handshake_send_begin(edge_connection_t *ap_conn)
 
   ap_conn->stream_id = get_unique_stream_id_by_circ(circ);
   if (ap_conn->stream_id==0) {
+    /* XXXX023 Instead of closing this stream, we should make it get
+     * retried on another circuit. */
     connection_mark_unattached_ap(ap_conn, END_STREAM_REASON_INTERNAL);
-    /*XXXX022 _close_ the circuit because it's full?  That sounds dumb. */
-    circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_RESOURCELIMIT);
+
+    /* Mark this circuit "unusable for new streams". */
+    /* XXXX023 this is a kludgy way to do this. */
+    tor_assert(circ->_base.timestamp_dirty);
+    circ->_base.timestamp_dirty -= get_options()->MaxCircuitDirtiness;
     return -1;
   }
 
@@ -2224,9 +2229,14 @@ connection_ap_handshake_send_resolve(edge_connection_t *ap_conn)
 
   ap_conn->stream_id = get_unique_stream_id_by_circ(circ);
   if (ap_conn->stream_id==0) {
+    /* XXXX023 Instead of closing this stream, we should make it get
+     * retried on another circuit. */
     connection_mark_unattached_ap(ap_conn, END_STREAM_REASON_INTERNAL);
-    /*XXXX022 _close_ the circuit because it's full?  That sounds dumb. */
-    circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_RESOURCELIMIT);
+
+    /* Mark this circuit "unusable for new streams". */
+    /* XXXX023 this is a kludgy way to do this. */
+    tor_assert(circ->_base.timestamp_dirty);
+    circ->_base.timestamp_dirty -= get_options()->MaxCircuitDirtiness;
     return -1;
   }