svn_ra_session_open4 already uses some variables from the config without duplicating them to the internal pool, so in that way it is not a first. svn_ra_dup_session() documents that it will re-use the original arguments of the first Ra session, which includes references to things like batons, that can't be duplicated.
(And Ra serf already did some internal session duplication before this patch) No, it is not the cleanest way to implement this, but I don't see any other option of creating helper Ra sessions from the Ra layer for the compact shims. The way we use it now from libsvn_client is safe as we always construct these sessions based on the state in svn_client_ctx_t, which must outlive both Ra sessions. Bert Sent from Windows Mail From: Julian Foad Sent: Friday, December 20, 2013 1:12 PM To: Bert Huijben Cc: Subversion Development > URL: http://svn.apache.org/r1552324 [...] > Modified: subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h > ============================================================================== > struct svn_ra_serf__session_t { > /* Pool for allocations during this session */ > apr_pool_t *pool; > + apr_hash_t *config; /* For duplicating */ > > /* The current context */ > serf_context_t *context; > > Modified: subversion/trunk/subversion/libsvn_ra_serf/serf.c > ============================================================================== > --- subversion/trunk/subversion/libsvn_ra_serf/serf.c (original) > +++ subversion/trunk/subversion/libsvn_ra_serf/serf.c Thu Dec 19 16:09:26 2013 > @@ -494,6 +494,7 @@ svn_ra_serf__open(svn_ra_session_t *sess > > serf_sess = apr_pcalloc(pool, sizeof(*serf_sess)); > serf_sess->pool = svn_pool_create(pool); > + serf_sess->config = config; Is this safe -- storing the old pointer to 'config' ... > @@ -598,6 +599,181 @@ > +/* Implements svn_ra__vtable_t.dup_session */ > +static svn_error_t * > +ra_serf_dup_session(svn_ra_session_t *new_session, > + svn_ra_session_t *old_session, > + const char *new_session_url, > + apr_pool_t *result_pool, > + apr_pool_t *scratch_pool) > +{ [...] > + SVN_ERR(load_config(new_sess, old_sess->config, result_pool)); ... and then using it later? I don't see any lifetime guarantee. - Julian
