svn commit: r1665609 - /subversion/trunk/subversion/libsvn_ra_serf/util.c

[rh...@apache.org]

Author: rhuijben
Date: Tue Mar 10 16:35:36 2015
New Revision: 1665609

URL: http://svn.apache.org/r1665609
Log:
In ra-serf: When running a single request and the request is still scheduled
when the handler reports itself as done: make sure the request is cancelled.

This avoids continuing handling the request while the pool containing
the request is not destroyed yet. This code is exercised by the merge code
that cancels a log request after the first results arrived.

* subversion/libsvn_ra_serf/util.c
  (svn_ra_serf__unschedule_handler): New function, +- extracted from
    handler_cleanup.
  (svn_ra_serf__context_run_one): If the handler is still scheduled when
    returing, properly unschedule.
  (handler_cleanup): Call svn_ra_serf__unschedule_handler.
    Remove now unneeded check, that made this problem mostly invisible.

Modified:
    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=1665609&r1=1665608&r2=1665609&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/util.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/util.c Tue Mar 10 16:35:36 2015
@@ -947,6 +947,22 @@ svn_ra_serf__context_run_wait(svn_boolea
   return SVN_NO_ERROR;
 }
 
+/* Ensure that a handler is no longer scheduled on the connection.
+
+   Eventually serf will have a reliable way to cancel existing requests,
+   but currently it doesn't even have a way to relyable identify a request
+   after rescheduling, for auth reasons.
+
+   So the only thing we can do today is reset the connection, which
+   will cancel all outstanding requests and prepare the connection
+   for re-use.
+*/
+static void
+svn_ra_serf__unschedule_handler(svn_ra_serf__handler_t *handler)
+{
+  serf_connection_reset(handler->conn->conn);
+  handler->scheduled = FALSE;
+}
 
 svn_error_t *
 svn_ra_serf__context_run_one(svn_ra_serf__handler_t *handler,
@@ -960,6 +976,15 @@ 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);
+
+  if (handler->scheduled)
+    {
+      /* We reset the connection (breaking  pipelining, etc.), as
+         if we didn't the next data would still be handled by this handler,
+         which is done as far as our caller is concerned. */
+      svn_ra_serf__unschedule_handler(handler);
+    }
+
   return svn_error_trace(err);
 }
 
@@ -1880,14 +1905,14 @@ response_done(serf_request_t *request,
    handlers registered in no freed memory.
 
    This fallback kills the connection for this case, which will make serf
-   unregister any*/
+   unregister any outstanding requests on it. */
 static apr_status_t
 handler_cleanup(void *baton)
 {
   svn_ra_serf__handler_t *handler = baton;
-  if (handler->scheduled && handler->conn && handler->conn->conn)
+  if (handler->scheduled)
     {
-      serf_connection_reset(handler->conn->conn);
+      svn_ra_serf__unschedule_handler(handler);
     }
 
   return APR_SUCCESS;