[or-cvs] r11882: Fix bug 451. This was a nasty bug, so let's fix it twice: fi (in tor/trunk: . src/or)
nickm at seul.org
nickm at seul.org
Thu Oct 11 20:45:27 UTC 2007
Author: nickm
Date: 2007-10-11 16:45:26 -0400 (Thu, 11 Oct 2007)
New Revision: 11882
Modified:
tor/trunk/
tor/trunk/ChangeLog
tor/trunk/src/or/connection.c
tor/trunk/src/or/or.h
Log:
r15689 at catbus: nickm | 2007-10-11 16:40:25 -0400
Fix bug 451. This was a nasty bug, so let's fix it twice: first, by banning recursive calls to connection_handle_write from connection_flushed_some; and second, by not calling connection_finished_flushing() on a closed connection. Backport candidate.
Property changes on: tor/trunk
___________________________________________________________________
svk:merge ticket from /tor/trunk [r15689] on 8246c3cf-6607-4228-993b-4d95d33730f1
Modified: tor/trunk/ChangeLog
===================================================================
--- tor/trunk/ChangeLog 2007-10-11 20:31:57 UTC (rev 11881)
+++ tor/trunk/ChangeLog 2007-10-11 20:45:26 UTC (rev 11882)
@@ -35,7 +35,19 @@
string which usually wasn't there, once for every routerinfo
we read, just scan lines forward until we find one we like.
Bugfix on 0.2.0.1.
+ - When we add data to a write buffer in response to the data on that
+ write buffer getting low because of a flush, do not consider the newly
+ added data as a candidate for immediate flushing, but rather make it
+ wait until the next round of writing. Otherwise, we flush and refill
+ recursively, and a single greedy TLS connection can eat all of our
+ bandwidth. Bugfix on 0.1.2.7-alpha.
+ o Major bugfixes (crashes):
+ - If a connection is shut down abruptly because of something that
+ happened inside connection_flushed_some(), do not call
+ connection_finished_flushing(). Should fix bug 451. Bugfix on
+ 0.1.2.7-alpha.
+
o Minor features (v3 authority system):
- Add more ways for tools to download the votes that lead to the
current consensus.
Modified: tor/trunk/src/or/connection.c
===================================================================
--- tor/trunk/src/or/connection.c 2007-10-11 20:31:57 UTC (rev 11881)
+++ tor/trunk/src/or/connection.c 2007-10-11 20:45:26 UTC (rev 11882)
@@ -529,6 +529,11 @@
}
}
+/** Return true iff connection_close_immediate has been called on this
+ * connection */
+#define CONN_IS_CLOSED(c) \
+ ((c)->linked ? ((c)->linked_conn_is_closed) : ((c)->s < 0))
+
/** Close the underlying socket for <b>conn</b>, so we don't try to
* flush it. Must be used in conjunction with (right before)
* connection_mark_for_close().
@@ -537,7 +542,7 @@
connection_close_immediate(connection_t *conn)
{
assert_connection_ok(conn,0);
- if (conn->s < 0 && !conn->linked) {
+ if (CONN_IS_CLOSED(conn)) {
log_err(LD_BUG,"Attempt to close already-closed connection.");
tor_fragile_assert();
return;
@@ -554,6 +559,8 @@
if (conn->s >= 0)
tor_close_socket(conn->s);
conn->s = -1;
+ if (conn->linked)
+ conn->linked_conn_is_closed = 1;
if (!connection_is_listener(conn)) {
buf_clear(conn->outbuf);
conn->outbuf_flushlen = 0;
@@ -2046,6 +2053,11 @@
if (conn->marked_for_close || conn->s < 0)
return 0; /* do nothing */
+ if (conn->in_flushed_some) {
+ log_warn(LD_BUG, "called recursively from inside conn->in_flushed_some()");
+ return 0;
+ }
+
conn->timestamp_lastwritten = now;
/* Sometimes, "writable" means "connected". */
@@ -2207,6 +2219,7 @@
_connection_write_to_buf_impl(const char *string, size_t len,
connection_t *conn, int zlib)
{
+ /* XXXX This function really needs to return -1 on failure. */
int r;
size_t old_datalen;
if (!len)
@@ -2248,9 +2261,18 @@
int extra = 0;
conn->outbuf_flushlen += len;
+ /* Should we try flushing the outbuf now? */
+ if (conn->in_flushed_some) {
+ /* Don't flush the outbuf when the reason we're writing more stuff is
+ * _because_ we flushed the outbuf. That's unfair. */
+ return;
+ }
+
if (conn->type == CONN_TYPE_OR &&
conn->outbuf_flushlen-len < MIN_TLS_FLUSHLEN &&
conn->outbuf_flushlen >= MIN_TLS_FLUSHLEN) {
+ /* We just pushed outbuf_flushelen to MIN_TLS_FLUSHLEN or above;
+ * we can send out a full TLS frame now if we like. */
extra = conn->outbuf_flushlen - MIN_TLS_FLUSHLEN;
conn->outbuf_flushlen = MIN_TLS_FLUSHLEN;
} else if (conn->type == CONN_TYPE_CONTROL &&
@@ -2625,13 +2647,17 @@
static int
connection_flushed_some(connection_t *conn)
{
+ int r = 0;
+ tor_assert(!conn->in_flushed_some);
+ conn->in_flushed_some = 1;
if (conn->type == CONN_TYPE_DIR &&
- conn->state == DIR_CONN_STATE_SERVER_WRITING)
- return connection_dirserv_flushed_some(TO_DIR_CONN(conn));
- else if (conn->type == CONN_TYPE_OR)
- return connection_or_flushed_some(TO_OR_CONN(conn));
- else
- return 0;
+ conn->state == DIR_CONN_STATE_SERVER_WRITING) {
+ r = connection_dirserv_flushed_some(TO_DIR_CONN(conn));
+ } else if (conn->type == CONN_TYPE_OR) {
+ r = connection_or_flushed_some(TO_OR_CONN(conn));
+ }
+ conn->in_flushed_some = 0;
+ return r;
}
/** We just finished flushing bytes from conn-\>outbuf, and there
@@ -2645,6 +2671,10 @@
{
tor_assert(conn);
+ /* If the connection is don't try to do anything more here. */
+ if (CONN_IS_CLOSED(conn))
+ return 0;
+
// log_fn(LOG_DEBUG,"entered. Socket %u.", conn->s);
switch (conn->type) {
Modified: tor/trunk/src/or/or.h
===================================================================
--- tor/trunk/src/or/or.h 2007-10-11 20:31:57 UTC (rev 11881)
+++ tor/trunk/src/or/or.h 2007-10-11 20:45:26 UTC (rev 11882)
@@ -744,9 +744,6 @@
/* The next fields are all one-bit booleans. Some are only applicable to
* connection subtypes, but we hold them here anyway, to save space.
- * (Currently, they all fit into a single byte. If they ever need more than
- * one byte, we can shave some bits off type, state, and purpose above, none
- * of which is ever over 31.)
*/
unsigned read_blocked_on_bw:1; /**< Boolean: should we start reading again
* once the bandwidth throttler allows it? */
@@ -769,6 +766,9 @@
/** For AP connections only. If 1, and we fail to reach the chosen exit,
* stop requiring it. */
unsigned int chosen_exit_optional:1;
+ /** Set to 1 when we're inside connection_flushed_some to keep us from
+ * calling connection_handle_write() recursively. */
+ unsigned int in_flushed_some:1;
/* For linked connections:
*/
@@ -781,6 +781,9 @@
/** True iff we're currently able to read on the linked conn, and our
* read_event should be made active with libevent. */
unsigned int active_on_link:1;
+ /** True iff we've called connection_close_immediate on this linked
+ * connection */
+ unsigned int linked_conn_is_closed:1;
int s; /**< Our socket; -1 if this connection is closed, or has no
* socket. */
More information about the tor-commits
mailing list