Bert,
On Mon, Jan 13, 2014 at 12:59 PM, <rhuijben_at_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
Received on 2014-06-22 10:33:37 CEST