You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2014/03/07 17:48:18 UTC

svn commit: r1575319 - in /subversion/trunk/subversion/libsvn_ra_serf: options.c ra_serf.h serf.c

Author: rhuijben
Date: Fri Mar  7 16:48:18 2014
New Revision: 1575319

URL: http://svn.apache.org/r1575319
Log:
Introduce a subpool in the ra_serf open session handling, to reduce memory
usage in the session pool and to work around a bug in ra serf, where it
doesn't remember to tell us that a request failed/was cancelled.

* subversion/libsvn_ra_serf/options.c
  (svn_ra_serf__exchange_capabilities): Use result, scratch pool.

* subversion/libsvn_ra_serf/ra_serf.h
  (svn_ra_serf__exchange_capabilities): Use result, scratch pool.

* subversion/libsvn_ra_serf/serf.c
  (svn_ra_serf__open): Introduce a subpool. Update caller.

Modified:
    subversion/trunk/subversion/libsvn_ra_serf/options.c
    subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
    subversion/trunk/subversion/libsvn_ra_serf/serf.c

Modified: subversion/trunk/subversion/libsvn_ra_serf/options.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/options.c?rev=1575319&r1=1575318&r2=1575319&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/options.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/options.c Fri Mar  7 16:48:18 2014
@@ -481,14 +481,16 @@ svn_ra_serf__v1_get_activity_collection(
 svn_error_t *
 svn_ra_serf__exchange_capabilities(svn_ra_serf__session_t *serf_sess,
                                    const char **corrected_url,
-                                   apr_pool_t *pool)
+                                   apr_pool_t *result_pool,
+                                   apr_pool_t *scratch_pool)
 {
   options_context_t *opt_ctx;
 
   /* This routine automatically fills in serf_sess->capabilities */
-  SVN_ERR(create_options_req(&opt_ctx, serf_sess, serf_sess->conns[0], pool));
+  SVN_ERR(create_options_req(&opt_ctx, serf_sess, serf_sess->conns[0],
+                             scratch_pool));
 
-  SVN_ERR(svn_ra_serf__context_run_one(opt_ctx->handler, pool));
+  SVN_ERR(svn_ra_serf__context_run_one(opt_ctx->handler, scratch_pool));
 
   /* If our caller cares about server redirections, and our response
      carries such a thing, report as much.  We'll disregard ERR --
@@ -496,7 +498,7 @@ svn_ra_serf__exchange_capabilities(svn_r
      successfully parsing as XML or somesuch. */
   if (corrected_url && (opt_ctx->handler->sline.code == 301))
     {
-      *corrected_url = opt_ctx->handler->location;
+      *corrected_url = apr_pstrdup(result_pool, opt_ctx->handler->location);
       return SVN_NO_ERROR;
     }
 

Modified: subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h?rev=1575319&r1=1575318&r2=1575319&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h Fri Mar  7 16:48:18 2014
@@ -1582,7 +1582,8 @@ svn_ra_serf__get_mergeinfo(svn_ra_sessio
 svn_error_t *
 svn_ra_serf__exchange_capabilities(svn_ra_serf__session_t *serf_sess,
                                    const char **corrected_url,
-                                   apr_pool_t *pool);
+                                   apr_pool_t *result_pool,
+                                   apr_pool_t *scratch_pool);
 
 /* Implements svn_ra__vtable_t.has_capability(). */
 svn_error_t *

Modified: subversion/trunk/subversion/libsvn_ra_serf/serf.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/serf.c?rev=1575319&r1=1575318&r2=1575319&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/serf.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/serf.c Fri Mar  7 16:48:18 2014
@@ -489,6 +489,7 @@ svn_ra_serf__open(svn_ra_session_t *sess
   apr_uri_t url;
   const char *client_string = NULL;
   svn_error_t *err;
+  apr_pool_t *subpool;
 
   if (corrected_url)
     *corrected_url = NULL;
@@ -584,12 +585,16 @@ svn_ra_serf__open(svn_ra_session_t *sess
 
   session->priv = serf_sess;
 
-  err = svn_ra_serf__exchange_capabilities(serf_sess, corrected_url, pool);
+  subpool = svn_pool_create(pool);
+
+  err = svn_ra_serf__exchange_capabilities(serf_sess, corrected_url,
+                                           pool, subpool);
 
   /* serf should produce a usable error code instead of APR_EGENERAL */
   if (err && err->apr_err == APR_EGENERAL)
     err = svn_error_createf(SVN_ERR_RA_DAV_REQUEST_FAILED, err,
                             _("Connection to '%s' failed"), session_URL);
+  svn_pool_clear(subpool);
   SVN_ERR(err);
 
   /* We have set up a useful connection (that doesn't indication a redirect).
@@ -598,7 +603,9 @@ svn_ra_serf__open(svn_ra_session_t *sess
      problems in any proxy.  */
   if ((corrected_url == NULL || *corrected_url == NULL)
       && serf_sess->detect_chunking && !serf_sess->http10)
-    SVN_ERR(svn_ra_serf__probe_proxy(serf_sess, pool));
+    SVN_ERR(svn_ra_serf__probe_proxy(serf_sess, subpool));
+
+  svn_pool_destroy(subpool);
 
   return SVN_NO_ERROR;
 }



Re: svn commit: r1575319 - in /subversion/trunk/subversion/libsvn_ra_serf: options.c ra_serf.h serf.c

Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 7 March 2014 20:48,  <rh...@apache.org> wrote:
> Author: rhuijben
> Date: Fri Mar  7 16:48:18 2014
> New Revision: 1575319
>
> URL: http://svn.apache.org/r1575319
> Log:
> Introduce a subpool in the ra_serf open session handling, to reduce memory
> usage in the session pool and to work around a bug in ra serf, where it
> doesn't remember to tell us that a request failed/was cancelled.
>
Hi Bert,

Could you please document this hack in code where subpool created to
prevent accidental removal of subpool in future.

-- 
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com