[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1552324 - Introduce svn_ra_session_dup()

From: Bert Huijben <bert_at_qqmail.nl>
Date: Fri, 20 Dec 2013 12:47:53 +0000

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
Received on 2013-12-20 13:55:46 CET

This is an archived mail posted to the Subversion Dev mailing list.