Browse Source

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.
Nick Mathewson 13 years ago
parent
commit
e617a34d58
4 changed files with 27 additions and 2 deletions
  1. 4 0
      changes/bug3367
  2. 19 0
      src/or/connection.c
  3. 2 0
      src/or/connection.h
  4. 2 2
      src/or/control.c

+ 4 - 0
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.

+ 19 - 0
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

+ 2 - 0
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,

+ 2 - 2
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);