[tor-commits] [tor/maint-0.2.4] Always call circuit_n_chan_done(chan, 0) from channel_closed()

[nickm at torproject.org]

commit 938ee9b24dfda03a68a71d43b88e9fb00a90f9c8
Author: Andrea Shepard <andrea at torproject.org>
Date:   Mon Sep 23 09:37:26 2013 -0700

    Always call circuit_n_chan_done(chan, 0) from channel_closed()
---
 changes/bug9776        |    5 +++++
 src/or/channel.c       |    9 ++++-----
 src/or/connection_or.c |   10 ++++++++++
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/changes/bug9776 b/changes/bug9776
new file mode 100644
index 0000000..ea3a96a
--- /dev/null
+++ b/changes/bug9776
@@ -0,0 +1,5 @@
+   o Normal bugfixes:
+     - Always call circuit_n_chan_done(chan, 0) from channel_closed(), so we
+       can't leak pending circuits in some cases where
+       run_connection_housekeeping() calls connection_or_close_normally().
+       Fixes bug #9776; bugfix on 0.2.4.17.
diff --git a/src/or/channel.c b/src/or/channel.c
index 602797d..1fb39b8 100644
--- a/src/or/channel.c
+++ b/src/or/channel.c
@@ -1292,11 +1292,10 @@ channel_closed(channel_t *chan)
   if (chan->state == CHANNEL_STATE_CLOSED ||
       chan->state == CHANNEL_STATE_ERROR) return;
 
-  if (chan->reason_for_closing == CHANNEL_CLOSE_FOR_ERROR) {
-    /* Inform any pending (not attached) circs that they should
-     * give up. */
-    circuit_n_chan_done(chan, 0);
-  }
+  /* Inform any pending (not attached) circs that they should
+   * give up. */
+  circuit_n_chan_done(chan, 0);
+
   /* Now close all the attached circuits on it. */
   circuit_unlink_all_from_channel(chan, END_CIRC_REASON_CHANNEL_CLOSED);
 
diff --git a/src/or/connection_or.c b/src/or/connection_or.c
index 0e9d06a..3d16e14 100644
--- a/src/or/connection_or.c
+++ b/src/or/connection_or.c
@@ -1160,6 +1160,16 @@ connection_or_connect(const tor_addr_t *_addr, uint16_t port,
 
 /** Mark orconn for close and transition the associated channel, if any, to
  * the closing state.
+ *
+ * It's safe to call this and connection_or_close_for_error() any time, and
+ * channel layer will treat it as a connection closing for reasons outside
+ * its control, like the remote end closing it.  It can also be a local
+ * reason that's specific to connection_t/or_connection_t rather than
+ * the channel mechanism, such as expiration of old connections in
+ * run_connection_housekeeping().  If you want to close a channel_t
+ * from somewhere that logically works in terms of generic channels
+ * rather than connections, use channel_mark_for_close(); see also
+ * the comment on that function in channel.c.
  */
 
 void