I'd go one better and say this might be appropriate for 1.5.x, too.
On Tue, Mar 31, 2009 at 23:25, Ben Collins-Sussman <sussman_at_red-bean.com> wrote:
> Definitely worth a 1.6.1 backport, I think. ra_serf currently fails
> to commit against googlecode.com due to some google-specific
> infrastructure latency in responses, which then triggers this infinite
> loop in ra_serf.
>
> On Tue, Mar 31, 2009 at 4:22 PM, Ben Collins-Sussman
> <sussman_at_red-bean.com> wrote:
>> Author: sussman
>> Date: Tue Mar 31 14:22:56 2009
>> New Revision: 36913
>>
>> Log:
>> Fix race condition in ra_serf which can cause infinite-loop during a commit.
>> Diagnosis and patch by Jon Trowbridge <jon_at_trowbridge.org>.
>>
>> After initializing the parser, we were attempting to make it parse
>> response data. Apparently up till now, the server response was always
>> "fast enough" such that 100% of the response was waiting in the
>> buffer, ready to be parsed, and so __handle_xml_parser() always
>> succeeded.
>>
>> But if you introduce latency into the server's response then parsing
>> might fail (the data may not be available yet), which means
>> 'parser.done' doesn't get set. But then we *never* re-enter the
>> initialization block on subsequent calls, which means 'done' never
>> gets set, which means we're in an infinite loop waiting for 'done'.
>>
>> * subversion/libsvn_ra_serf/util.c
>> (svn_ra_serf__handle_multistatus_only): separate parser
>> initialization from the "is parser done?" check. And actually examine
>> the status result of __handle_xml_parser()!
>>
>> Modified:
>> trunk/subversion/libsvn_ra_serf/util.c
>>
>> Modified: trunk/subversion/libsvn_ra_serf/util.c
>> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_ra_serf/util.c?pathrev=36913&r1=36912&r2=36913
>> ==============================================================================
>> --- trunk/subversion/libsvn_ra_serf/util.c Tue Mar 31 13:39:01 2009 (r36912)
>> +++ trunk/subversion/libsvn_ra_serf/util.c Tue Mar 31 14:22:56 2009 (r36913)
>> @@ -846,6 +846,7 @@ svn_ra_serf__handle_multistatus_only(ser
>> svn_ra_serf__simple_request_context_t *ctx = baton;
>> svn_ra_serf__server_error_t *server_err = &ctx->server_error;
>>
>> + /* If necessary, initialize our XML parser. */
>> if (server_err && !server_err->init)
>> {
>> serf_bucket_t *hdrs;
>> @@ -867,16 +868,7 @@ svn_ra_serf__handle_multistatus_only(ser
>> server_err->parser.cdata = cdata_207;
>> server_err->parser.done = &ctx->done;
>> server_err->parser.ignore_errors = TRUE;
>> -
>> - status = svn_ra_serf__handle_xml_parser(request, response,
>> - &server_err->parser, pool);
>> -
>> - if (ctx->done && server_err->error->apr_err == APR_SUCCESS)
>> - {
>> - svn_error_clear(server_err->error);
>> - server_err->error = SVN_NO_ERROR;
>> - }
>> - }
>> + }
>> else
>> {
>> ctx->done = TRUE;
>> @@ -884,6 +876,30 @@ svn_ra_serf__handle_multistatus_only(ser
>> }
>> }
>>
>> + /* If server_err->error still contains APR_SUCCESS, it means that we
>> + have not successfully parsed the XML yet. */
>> + if (server_err && server_err->error
>> + && server_err->error->apr_err == APR_SUCCESS)
>> + {
>> + status = svn_ra_serf__handle_xml_parser(request, response,
>> + &server_err->parser, pool);
>> +
>> + /* APR_EOF will be returned when parsing is complete. If we see
>> + any other error, return it immediately. In practice the only
>> + other error we expect to see is APR_EAGAIN, which indicates that
>> + we could not parse the XML because the contents are not yet
>> + available to be read. */
>> + if (!APR_STATUS_IS_EOF(status))
>> + {
>> + return status;
>> + }
>> + else if (ctx->done && server_err->error->apr_err == APR_SUCCESS)
>> + {
>> + svn_error_clear(server_err->error);
>> + server_err->error = SVN_NO_ERROR;
>> + }
>> + }
>> +
>> status = svn_ra_serf__handle_discard_body(request, response,
>> NULL, pool);
>>
>> ------------------------------------------------------
>> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageId=1499817
>>
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1499834
>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1506592
Received on 2009-04-01 14:33:17 CEST