[tor-commits] [tor/master] Add, use a bufferevent-safe connection_flush()

[nickm at torproject.org]

commit e617a34d589ac8ef26203ae90443a0e98f7032b1
Author: Nick Mathewson <nickm at torproject.org>
Date:   Tue Jun 21 10:22:54 2011 -0400

    Add, use a bufferevent-safe connection_flush()
    
    A couple of places in control.c were using connection_handle_write()
    to flush important stuff (the response to a SIGNAL command, an
    ERR-level status event) before Tor went down.  But
    connection_handle_write() isn't meaningful for bufferevents, so we'd
    crash.
    
    This patch adds a new connection_flush() that works for all connection
    backends, and makes control.c use that instead.
    
    Fix for bug 3367; bugfix on 0.2.3.1-alpha.
---
 changes/bug3367     |    4 ++++
 src/or/connection.c |   19 +++++++++++++++++++
 src/or/connection.h |    2 ++
 src/or/control.c    |    4 ++--
 4 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/changes/bug3367 b/changes/bug3367
new file mode 100644
index 0000000..8a69778
--- /dev/null
+++ b/changes/bug3367
@@ -0,0 +1,4 @@
+  o Minor bugfixes
+    - Fix a crash when handling the SIGNAL controller command or
+      reporting ERR-level status events with bufferevents enabled. Found
+      by Robert Ransom. Fixes bug 3367; bugfix on 0.2.3.1-alpha.
diff --git a/src/or/connection.c b/src/or/connection.c
index 317bfb1..12d93ce 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -3314,6 +3314,25 @@ connection_handle_write(connection_t *conn, int force)
     return res;
 }
 
+/**
+ * Try to flush data that's waiting for a write on <b>conn</b>.  Return
+ * -1 on failure, 0 on success.
+ *
+ * Don't use this function for regular writing; the buffers/bufferevents
+ * system should be good enough at scheduling writes there.  Instead, this
+ * function is for cases when we're about to exit or something and we want
+ * to report it right away.
+ */
+int
+connection_flush(connection_t *conn)
+{
+  IF_HAS_BUFFEREVENT(conn, {
+      int r = bufferevent_flush(conn->bufev, EV_WRITE, BEV_FLUSH);
+      return (r < 0) ? -1 : 0;
+  });
+  return connection_handle_write(conn, 1);
+}
+
 /** OpenSSL TLS record size is 16383; this is close. The goal here is to
  * push data out as soon as we know there's enough for a TLS record, so
  * during periods of high load we won't read entire megabytes from
diff --git a/src/or/connection.h b/src/or/connection.h
index 94ae645..8dc0112 100644
--- a/src/or/connection.h
+++ b/src/or/connection.h
@@ -79,6 +79,8 @@ int connection_fetch_from_buf_http(connection_t *conn,
 int connection_wants_to_flush(connection_t *conn);
 int connection_outbuf_too_full(connection_t *conn);
 int connection_handle_write(connection_t *conn, int force);
+int connection_flush(connection_t *conn);
+
 void _connection_write_to_buf_impl(const char *string, size_t len,
                                    connection_t *conn, int zlib);
 static void connection_write_to_buf(const char *string, size_t len,
diff --git a/src/or/control.c b/src/or/control.c
index 5e71a2e..e65cd26 100644
--- a/src/or/control.c
+++ b/src/or/control.c
@@ -600,7 +600,7 @@ send_control_event_string(uint16_t event, event_format_t which,
         else if (event == EVENT_STATUS_SERVER)
           is_err = !strcmpstart(msg, "STATUS_SERVER ERR ");
         if (is_err)
-          connection_handle_write(TO_CONN(control_conn), 1);
+          connection_flush(TO_CONN(control_conn));
       }
     }
   } SMARTLIST_FOREACH_END(conn);
@@ -1257,7 +1257,7 @@ handle_control_signal(control_connection_t *conn, uint32_t len,
   send_control_done(conn);
   /* Flush the "done" first if the signal might make us shut down. */
   if (sig == SIGTERM || sig == SIGINT)
-    connection_handle_write(TO_CONN(conn), 1);
+    connection_flush(TO_CONN(conn));
 
   process_signal(sig);