Bert, On Mon, Jan 13, 2014 at 12:59 PM, <rhuij...@apache.org> wrote: > Author: rhuijben > Date: Mon Jan 13 11:59:56 2014 > New Revision: 1557686 > > URL: http://svn.apache.org/r1557686 > Log: > Make ra_serf use a pool cleanup handler on its request handlers to allow > reusing the ra session in cases that before this patch would cause a segfault > because it wasn't cleaned up. > > If the ra session would schedule a new request before timeout, the context > would continue delivering data to old handlers. After this patch that won't > happen again. > > * subversion/libsvn_ra_serf/ra_serf.h > (svn_ra_serf__handler_t): Add boolean. > > * subversion/libsvn_ra_serf/util.c > (svn_ra_serf__context_run_one): Remove specialized handling of > SVN_ERR_CEASE_INVOCATION that was already not required after recent > error handling cleanups, but is now completely unneeded. > (handle_response): Unregister on request errors. > (handle_response_cb): Unregister on EOF and/or errors. > (svn_ra_serf__request_create): Check for double scheduling. Update comment > to match current reality. > > (handler_cleanup): New function. > (svn_ra_serf__create_handler): Register handler_cleanup. > > Modified: > subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h > subversion/trunk/subversion/libsvn_ra_serf/util.c > [..]
> Modified: subversion/trunk/subversion/libsvn_ra_serf/util.c > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/util.c?rev=1557686&r1=1557685&r2=1557686&view=diff > ============================================================================== > --- subversion/trunk/subversion/libsvn_ra_serf/util.c (original) > +++ subversion/trunk/subversion/libsvn_ra_serf/util.c Mon Jan 13 11:59:56 2014 > @@ -930,23 +930,6 @@ svn_ra_serf__context_run_one(svn_ra_serf > /* Wait until the response logic marks its DONE status. */ > err = svn_ra_serf__context_run_wait(&handler->done, handler->session, > scratch_pool); > - > - /* A callback invocation has been canceled. In this simple case of > - context_run_one, we can keep the ra-session operational by resetting > - the connection. > - > - If we don't do this, the next context run will notice that the > connection > - is still in the error state and will just return > SVN_ERR_CEASE_INVOCATION > - (=the last error for the connection) again */ > - if (err && err->apr_err == SVN_ERR_CEASE_INVOCATION) > - { > - apr_status_t status = serf_connection_reset(handler->conn->conn); > - > - if (status) > - err = svn_error_compose_create(err, > - svn_ra_serf__wrap_err(status, NULL)); > - } > - > return svn_error_trace(err); > } > > @@ -1200,6 +1183,8 @@ handle_response(serf_request_t *request, > if (!response) > { > /* Uh-oh. Our connection died. */ > + handler->scheduled = FALSE; > + > if (handler->response_error) > { > /* Give a handler chance to prevent request requeue. */ > @@ -1423,6 +1408,7 @@ handle_response_cb(serf_request_t *reque > { > svn_ra_serf__session_t *sess = handler->session; > handler->done = TRUE; > + handler->scheduled = FALSE; > outer_status = APR_EOF; > > /* We use a cached handler->session here to allow handler to free the > @@ -1436,7 +1422,8 @@ handle_response_cb(serf_request_t *reque > { > handler->discard_body = TRUE; /* Discard further data */ > handler->done = TRUE; /* Mark as done */ > - return APR_EAGAIN; /* Exit context loop */ > + handler->scheduled = FALSE; > + outer_status = APR_EAGAIN; /* Exit context loop */ This looks wrong: why are you hiding the error status from serf? Is there an issue in serf that you want to work around here? [..] Lieven
