[evolution-patches] possible fix for the soup connection_death loop

[Dan Winship <danw ximian com>]

I was trying to fix 42468, which turned out to be our old friend the
connection_death loop. There are actually 3 fixes in this patch:

     1. Only set a death watch on the connection when in_use is FALSE,
        so that we can always destroy it when connection_death is
        called, rather than returning FALSE and getting into an infinite
        loop.
     2. Fix a double free situation in soup-transfer made more common by
        that fix
     3. Fix the new infinite loop that 42468 turns into with that fix
        because the connection was never getting marked as not-new.

With those 3 fixes... 42468 *still* gets into an infinite loop, now
because of bugs in the NTLM logic. (It keeps forgetting that it's in the
middle of authenticating and starts over.) I'm still working on that
part, and I'm not going to commit this until I've figured that out too,
but those fixes should be orthogonal to these ones.


Index: soup-context.c
===================================================================
RCS file: /cvs/gnome/libsoup/libsoup/soup-context.c,v
retrieving revision 1.48
diff -u -r1.48 soup-context.c
--- soup-context.c	11 Apr 2003 17:02:29 -0000	1.48
+++ soup-context.c	7 May 2003 22:55:40 -0000
@@ -260,7 +260,8 @@
 	g_io_channel_unref (conn->channel);
 	soup_context_unref (conn->context);
 	soup_socket_unref (conn->socket);
-	g_source_remove (conn->death_tag);
+	if (conn->death_tag)
+		g_source_remove (conn->death_tag);
 	g_free (conn);
 
 	connection_count--;
@@ -271,12 +272,31 @@
 		  GIOCondition    condition,
 		  SoupConnection *conn)
 {
+	connection_free (conn);
+	return FALSE;
+}
+
+static void
+soup_connection_set_in_use (SoupConnection *conn, gboolean in_use)
+{
+	if (in_use == conn->in_use)
+		return;
+
+	conn->in_use = in_use;
 	if (!conn->in_use) {
-		connection_free (conn);
-		return FALSE;
-	}
+		GIOChannel *chan;
 
-	return TRUE;
+		chan = soup_connection_get_iochannel (conn);
+		conn->death_tag = 
+			g_io_add_watch (chan,
+					G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
+					(GIOFunc) connection_death,
+					conn);
+		g_io_channel_unref (chan);
+	} else {
+		g_source_remove (conn->death_tag);
+		conn->death_tag = 0;
+	}
 }
 
 struct SoupConnectData {
@@ -334,7 +354,6 @@
 	struct SoupConnectData *data = user_data;
 	SoupContext            *ctx = data->ctx;
 	SoupConnection         *new_conn;
-	GIOChannel             *chan;
 
 	switch (status) {
 	case SOUP_SOCKET_CONNECT_ERROR_NONE:
@@ -344,19 +363,12 @@
 		new_conn->port = ctx->uri->port;
 		new_conn->keep_alive = TRUE;
 		new_conn->in_use = TRUE;
+		new_conn->new = TRUE;
 		new_conn->last_used_id = 0;
 
 		new_conn->context = ctx;
 		soup_context_ref (ctx);
 
-		chan = soup_connection_get_iochannel (new_conn);
-		new_conn->death_tag = 
-			g_io_add_watch (chan,
-					G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
-					(GIOFunc) connection_death,
-					new_conn);
-		g_io_channel_unref (chan);
-
 		ctx->server->connections =
 			g_slist_prepend (ctx->server->connections, new_conn);
 
@@ -414,7 +426,7 @@
 		    conn->keep_alive == TRUE &&
 		    conn->port == (guint) ctx->uri->port) {
 			/* Set connection to in use */
-			conn->in_use = TRUE;
+			soup_connection_set_in_use (conn, TRUE);
 
 			/* Reset connection context */
 			soup_context_ref (ctx);
@@ -589,7 +601,7 @@
 
 	if (conn->keep_alive) {
 		conn->last_used_id = ++most_recently_used_id;
-		conn->in_use = FALSE;		
+		soup_connection_set_in_use (conn, FALSE);
 	} else
 		connection_free (conn);
 }
@@ -688,11 +700,25 @@
 }
 
 /**
+ * soup_connection_set_used:
+ * @conn: a %SoupConnection.
+ *
+ * Clears the "new" flag on @conn.
+ */
+void
+soup_connection_set_used (SoupConnection *conn)
+{
+	g_return_if_fail (conn != NULL);
+
+	conn->new = FALSE;
+}
+
+/**
  * soup_connection_is_new:
  * @conn: a %SoupConnection.
  *
  * Returns TRUE if this is the first use of @conn
- * (I.E. %soup_connection_release has not yet been called on it).
+ * (I.E. no response has been read from it yet)
  *
  * Return value: boolean representing whether this is the first time a
  * connection has been used.
@@ -701,7 +727,7 @@
 soup_connection_is_new (SoupConnection *conn)
 {
 	g_return_val_if_fail (conn != NULL, FALSE);
-	return conn->last_used_id == 0;
+	return conn->new;
 }
 
 
Index: soup-context.h
===================================================================
RCS file: /cvs/gnome/libsoup/libsoup/soup-context.h,v
retrieving revision 1.16
diff -u -r1.16 soup-context.h
--- soup-context.h	11 Apr 2003 17:02:29 -0000	1.16
+++ soup-context.h	7 May 2003 22:55:41 -0000
@@ -57,6 +57,7 @@
 
 gboolean       soup_connection_is_keep_alive  (SoupConnection       *conn);
 
+void           soup_connection_set_used       (SoupConnection       *conn);
 gboolean       soup_connection_is_new         (SoupConnection       *conn);
 
 void           soup_connection_release        (SoupConnection       *conn);
Index: soup-message.c
===================================================================
RCS file: /cvs/gnome/libsoup/libsoup/soup-message.c,v
retrieving revision 1.55
diff -u -r1.55 soup-message.c
--- soup-message.c	6 May 2003 13:00:52 -0000	1.55
+++ soup-message.c	7 May 2003 22:55:41 -0000
@@ -582,6 +582,7 @@
 	SoupMessage *msg = data->msg;
 	SoupConnection *conn = msg->connection;
 
+	soup_connection_set_used (msg->connection);
 	if (!soup_connection_is_keep_alive (msg->connection))
 		requeue_read_error (FALSE, data);
 	else {
Index: soup-private.h
===================================================================
RCS file: /cvs/gnome/libsoup/libsoup/soup-private.h,v
retrieving revision 1.53
diff -u -r1.53 soup-private.h
--- soup-private.h	15 Nov 2002 16:26:42 -0000	1.53
+++ soup-private.h	7 May 2003 22:55:41 -0000
@@ -82,6 +82,7 @@
 	guint         last_used_id;
 	gboolean      keep_alive;
 	guint         death_tag;
+	gboolean      new;
 };
 
 struct _SoupServer {
Index: soup-queue.c
===================================================================
RCS file: /cvs/gnome/libsoup/libsoup/soup-queue.c,v
retrieving revision 1.80
diff -u -r1.80 soup-queue.c
--- soup-queue.c	25 Apr 2003 17:34:08 -0000	1.80
+++ soup-queue.c	7 May 2003 22:55:41 -0000
@@ -112,6 +112,9 @@
 		soup_message_issue_callback (req);
 		break;
 	}
+
+	if (req->connection)
+		soup_connection_set_used (req->connection);
 }
 
 static SoupTransferDone
@@ -246,6 +249,8 @@
 			 gpointer              user_data)
 {
 	SoupMessage *req = user_data;
+
+	soup_connection_set_used (req->connection);
 
 	req->response.owner = data->owner;
 	req->response.length = data->length;
Index: soup-transfer.c
===================================================================
RCS file: /cvs/gnome/libsoup/libsoup/soup-transfer.c,v
retrieving revision 1.29
diff -u -r1.29 soup-transfer.c
--- soup-transfer.c	29 Apr 2003 16:02:24 -0000	1.29
+++ soup-transfer.c	7 May 2003 22:55:41 -0000
@@ -448,8 +448,10 @@
 
  PROCESS_READ:
 
-	if (r->header_len == 0 && total_read == 0)
-		goto FINISH_READ;
+	if (r->header_len == 0 && total_read == 0) {
+		soup_transfer_read_error_cb (iochannel, G_IO_HUP, r);
+		return FALSE;
+	}
 
 	if (r->header_len == 0) {
 		gint index;