[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: ra_serf crashes on Windows with AVG 2012 Surf-Shield

From: Greg Stein <gstein_at_gmail.com>
Date: Mon, 7 May 2012 20:29:54 -0400

On May 7, 2012 8:16 PM, "Lieven Govaerts" <svnlgo_at_mobsol.be> wrote:
>...
> The problem is in ra_serf/util.c svn_ra_serf__handle_xml_parser:
>
>   if (sl.code == 404 && ctx->ignore_errors == FALSE)
>     {
>       add_done_item(ctx);
>
>       err = svn_ra_serf__handle_server_error(request, response, pool);
>
>       SVN_ERR(svn_error_compose_create(
>         svn_ra_serf__handle_discard_body(request, response, NULL, pool),
>         err));
>
> When the response status of a PROPFIND request is 404, you see that the response body is discarded with calls to svn_ra_serf__handle_server_error and svn_ra_serf__handle_server_error.
>
> In your particular scenario, the status line of the response is already received, but the body is not. Reading from the response buckets returns EAGAIN status.
> Problem: the add_done_item(ctx) line ensures that the request is considered as handled, while the response body is still waiting on the socket to be read. ra_serf will only run the serf loop again with the next request. If the connection is not closed directly, which here it isn't, the next request will have a response that doesn't match.

Thanks for the excellent analysis of what Johan was running into.

> The fix is to ensure that the request is only marked as handled when a. the response body has been discarded completely or a b. read error was encountered resulting in serf setting up a new connection. I don't have a tested solution, as my Windows vm was so nice to reboot to install some updates while I was in the middle of a debug session, and I don't have time now to start over.

Not to worry. I've been working on exactly that stuff. In fact, the
code you quoted is one of my targets to fix.
svn_ra_serf__handle_server_error() is conceptually broken (and needs
to be removed for the reason you state), as I noted in the log message
of r1335217.

My intent is to replace the code you quoted with something basically
like: handler->server_error = alloc(). The core response handler will
then start processing the body as an error.

There are a couple similar cases. I'm looking at them now to ensure
the errors these things raise will propagate correctly, or to place
the error creation elsewhere. It should be fixed within a few days
(traveling tmw).

Cheers,
-g
Received on 2012-05-08 02:30:27 CEST

This is an archived mail posted to the Subversion Dev mailing list.