Re: [evolution-patches] soup patch for connection_death loop

[Jeffrey Stedfast <fejj ximian com>]

is this anything like the vulcan_death grip?

Jeff

On Wed, 2003-04-23 at 15:24, Dan Winship wrote:
> This fixes a bug I started encountering yesterday, which might also be
> 41754. It might also be Joe's soup infinite loop. (It's at least very
> similar).
> 
> The patch to soup-transfer.c is unfortunate, but that's soup-transfer
> for you.
> 
> Joe: I'm currently running with both this and my earlier uncommitted
> soup patch to split conn->in_use into a tri-state enum. I'm curious if
> (a) this patch fixes your loop, and (b) if you need the other patch too
> or not.
> 
> 
> 
> ______________________________________________________________________
> Index: ChangeLog
> ===================================================================
> RCS file: /cvs/gnome/libsoup/ChangeLog,v
> retrieving revision 1.297
> diff -u -r1.297 ChangeLog
> --- ChangeLog	21 Apr 2003 16:06:28 -0000	1.297
> +++ ChangeLog	23 Apr 2003 18:34:18 -0000
> @@ -1,3 +1,15 @@
> +2003-04-23  Dan Winship  <danw ximian com>
> +
> +	* libsoup/soup-message.c (soup_message_cleanup): Don't set up the
> +	soup-transfer callbacks to keep reading off the connection unless
> +	we're actually going to keep the connection around afterward.
> +	Otherwise we can just close it.
> +
> +	* libsoup/soup-transfer.c: Re-kludge the awful IGNORE_CANCEL
> +	thingy so that it's possible to cancel a read from inside a
> +	callback so that the above change actually works instead of just
> +	crashing.
> +
>  2003-04-20  Rodney Dawes  <dobey ximian com>
>  
>  	* configure.in: Up version to 1.99.18
> Index: libsoup/soup-message.c
> ===================================================================
> RCS file: /cvs/gnome/libsoup/libsoup/soup-message.c,v
> retrieving revision 1.53
> diff -u -r1.53 soup-message.c
> --- libsoup/soup-message.c	31 Mar 2003 15:32:39 -0000	1.53
> +++ libsoup/soup-message.c	23 Apr 2003 18:34:18 -0000
> @@ -171,6 +171,7 @@
>  	g_return_if_fail (req != NULL);
>  
>  	if (req->connection && 
> +	    soup_connection_is_keep_alive (req->connection) &&
>  	    req->priv->read_tag &&
>  	    req->status == SOUP_STATUS_READING_RESPONSE) {
>  		soup_transfer_read_set_callbacks (req->priv->read_tag,
> Index: libsoup/soup-transfer.c
> ===================================================================
> RCS file: /cvs/gnome/libsoup/libsoup/soup-transfer.c,v
> retrieving revision 1.27
> diff -u -r1.27 soup-transfer.c
> --- libsoup/soup-transfer.c	7 Mar 2003 23:02:26 -0000	1.27
> +++ libsoup/soup-transfer.c	23 Apr 2003 18:34:18 -0000
> @@ -73,7 +73,7 @@
>  	 */
>  	gboolean               callback_issued;
>  
> -	gboolean               processing;
> +	gboolean              *cancelled;
>  
>  	GByteArray            *recv_buf;
>  	guint                  header_len;
> @@ -96,7 +96,7 @@
>  	guint                   write_tag;
>  	guint                   err_tag;
>  
> -	gboolean                processing;
> +	gboolean               *cancelled;
>  
>  	SoupTransferEncoding    encoding;
>  	GByteArray             *write_buf;
> @@ -111,15 +111,18 @@
>  	gpointer                user_data;
>  } SoupWriter;
>  
> -#define IGNORE_CANCEL(t) (t)->processing = TRUE;
> -#define UNIGNORE_CANCEL(t) (t)->processing = FALSE;
> +#define IGNORE_CANCEL(t, cancel_p) (t)->cancelled = cancel_p;
> +#define UNIGNORE_CANCEL(t) (t)->cancelled = NULL;
>  
>  void
>  soup_transfer_read_cancel (guint tag)
>  {
>  	SoupReader *r = GINT_TO_POINTER (tag);
>  
> -	if (r->processing) return;
> +	if (r->cancelled) {
> +		*(r->cancelled) = TRUE;
> +		return;
> +	}
>  
>  	if (r->read_tag)
>  		g_source_remove (r->read_tag);
> @@ -170,9 +173,7 @@
>  		g_source_remove (r->err_tag);
>  		r->read_tag = r->err_tag = 0;
>  
> -		IGNORE_CANCEL (r);
>  		(*r->read_done_cb) (&buf, r->user_data);
> -		UNIGNORE_CANCEL (r);
>  	}
>  }
>  
> @@ -182,7 +183,9 @@
>  			     SoupReader *r)
>  {
>  	gboolean body_started = r->recv_buf->len > r->header_len;
> +	gboolean cancelled = FALSE;
>  
> +	IGNORE_CANCEL (r, &cancelled);
>  	/*
>  	 * Closing the connection to signify EOF is valid if content length is
>  	 * unknown, but only if headers have been sent.
> @@ -192,7 +195,6 @@
>  		goto CANCELLED;
>  	}
>  
> -	IGNORE_CANCEL (r);
>  	if (r->error_cb) (*r->error_cb) (body_started, r->user_data);
>  	UNIGNORE_CANCEL (r);
>  
> @@ -314,14 +316,12 @@
>  
>  		r->callback_issued = TRUE;
>  
> -		IGNORE_CANCEL (r);
> +		IGNORE_CANCEL (r, cancelled);
>  		cont = (*r->read_chunk_cb) (&buf, r->user_data);
>  		UNIGNORE_CANCEL (r);
>  
>  		if (cont == SOUP_TRANSFER_END)
>  			*cancelled = TRUE;
> -		else
> -			*cancelled = FALSE;
>  	}
>  }
>  
> @@ -471,14 +471,14 @@
>  			strncpy (str.str, r->recv_buf->data, index);
>  			str.str [index] = '\0';
>  
> -			IGNORE_CANCEL (r);
> +			IGNORE_CANCEL (r, &cancelled);
>  			ret = (*r->headers_done_cb) (&str, 
>  						     &r->encoding, 
>  						     &r->content_length, 
>  						     r->user_data);
>  			UNIGNORE_CANCEL (r);
>  
> -			if (ret == SOUP_TRANSFER_END) 
> +			if (ret == SOUP_TRANSFER_END || cancelled) 
>  				goto FINISH_READ;
>  		}
>  
> @@ -510,7 +510,9 @@
>  		goto READ_AGAIN;
>  	}
>  
> +	IGNORE_CANCEL (r, &cancelled);
>  	issue_final_callback (r);
> +	UNIGNORE_CANCEL (r);
>  
>   FINISH_READ:
>  	soup_transfer_read_cancel (GPOINTER_TO_INT (r));
> @@ -560,7 +562,10 @@
>  {
>  	SoupWriter *w = GINT_TO_POINTER (tag);
>  
> -	if (w->processing) return;
> +	if (w->cancelled) {
> +		*(w->cancelled) = TRUE;
> +		return;
> +	}
>  
>  	if (w->write_tag)
>  		g_source_remove (w->write_tag);
> @@ -577,7 +582,9 @@
>  			      SoupWriter *w)
>  {
>  	if (w->error_cb) {
> -		IGNORE_CANCEL (w);
> +		gboolean cancelled;
> +
> +		IGNORE_CANCEL (w, &cancelled);
>  		(*w->error_cb) (w->headers_done, w->user_data);
>  		UNIGNORE_CANCEL (w);
>  	}
> @@ -588,11 +595,11 @@
>  }
>  
>  static gboolean 
> -get_header (SoupWriter *w)
> +get_header (SoupWriter *w, gboolean *cancelled)
>  {
>  	GString *header = NULL;
>  
> -	IGNORE_CANCEL (w);
> +	IGNORE_CANCEL (w, cancelled);
>  	(*w->get_header_cb) (&header, w->user_data);
>  	UNIGNORE_CANCEL (w);
>  
> @@ -628,12 +635,12 @@
>  }
>  
>  static void
> -get_next_chunk (SoupWriter *w)
> +get_next_chunk (SoupWriter *w, gboolean *cancelled)
>  {
>  	SoupTransferStatus ret = SOUP_TRANSFER_END;
>  	SoupDataBuffer buf = { 0 , NULL, 0 };
>  
> -	IGNORE_CANCEL (w);
> +	IGNORE_CANCEL (w, cancelled);
>  	ret = (*w->get_chunk_cb) (&buf, w->user_data);
>  	UNIGNORE_CANCEL (w);
>  
> @@ -673,16 +680,23 @@
>  	GIOError error;
>  	gpointer pipe_handler;
>  	gsize bytes_written = 0;
> +	gboolean cancelled = FALSE;
>  
>  	/*
>  	 * Get the header and first data chunk (if available).
>  	 */
>  	if (w->get_header_cb) {
> -		if (!get_header (w)) 
> +		if (!get_header (w, &cancelled)) {
> +			if (cancelled)
> +				goto CANCEL;
>  			return TRUE;
> +		}
>  
> -		if (w->get_chunk_cb)
> -			get_next_chunk (w);
> +		if (w->get_chunk_cb) {
> +			get_next_chunk (w, &cancelled);
> +			if (cancelled)
> +				goto CANCEL;
> +		}
>  	}
>  
>  	IGNORE_PIPE (pipe_handler);
> @@ -721,7 +735,10 @@
>  	 * Get the next data chunk and try again, or quit if paused.
>  	 */
>  	if (w->get_chunk_cb) {
> -		get_next_chunk (w);
> +		get_next_chunk (w, &cancelled);
> +
> +		if (cancelled)
> +			goto CANCEL;
>  
>  		if (!w->write_tag)
>  			goto DONE_WRITING;
> @@ -730,11 +747,12 @@
>  	}
>  
>  	if (w->write_done_cb) {
> -		IGNORE_CANCEL (w);
> +		IGNORE_CANCEL (w, &cancelled);
>  		(*w->write_done_cb) (w->user_data);
>  		UNIGNORE_CANCEL (w);
>  	}
>  
> + CANCEL:
>  	soup_transfer_write_cancel (GPOINTER_TO_INT (w));
>  
>   DONE_WRITING:
-- 
Jeffrey Stedfast
Evolution Hacker - Ximian, Inc.
fejj ximian com  - www.ximian.com