subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Branko ─îibej <br...@wandisco.com>
Subject Re: client SEGV with --enable-runtime-module-search
Date Fri, 01 May 2015 17:18:05 GMT
On 01.05.2015 18:44, Philip Martin wrote:
> Philip Martin <philip.martin@wandisco.com> writes:
>
>> The problem is that the shared struct created in fs_serialized_init
>> doesn't live long enough:
>>
>> ==25549== Invalid read of size 8
>> ==25549==    at 0x8102166: init_lock_baton (fs_fs.c:265)
>> ==25549==    by 0x81022D4: create_lock_baton (fs_fs.c:317)
>> ==25549==    by 0x81023B6: svn_fs_fs__with_write_lock (fs_fs.c:365)
>> ==25549==    by 0x8106366: svn_fs_fs__change_rev_prop (fs_fs.c:2144)
>> ==25549==    by 0x7CE00EF: svn_fs_change_rev_prop2 (fs-loader.c:1577)
>> ==25549==    by 0x7AA8CC2: svn_repos_fs_change_rev_prop4 (fs-wrap.c:404)
>> ==25549==    by 0x7887C7C: svn_ra_local__change_rev_prop (ra_plugin.c:750)
>> ==25549==    by 0x4E3DC5D: svn_ra_change_rev_prop2 (ra_loader.c:576)
>> ==25549==    by 0x4E409EE: svn_ra__get_operational_lock (util.c:250)
>> ==25549==    by 0x402A60: get_lock (svnsync.c:399)
>> ==25549==    by 0x402AEA: with_locked (svnsync.c:457)
>> ==25549==    by 0x403C7C: initialize_cmd (svnsync.c:909)
>> ==25549==  Address 0x74be778 is 24 bytes inside a block of size 56 free'd
>> ==25549==    at 0x4C29E90: free (vg_replace_malloc.c:473)
>> ==25549==    by 0x55C7096: pool_clear_debug (apr_pools.c:1576)
>> ==25549==    by 0x55C71EA: pool_destroy_debug (apr_pools.c:1638)
>> ==25549==    by 0x55C6FAA: pool_clear_debug (apr_pools.c:1550)
>> ==25549==    by 0x55C71EA: pool_destroy_debug (apr_pools.c:1638)
>> ==25549==    by 0x55C72D3: apr_pool_destroy_debug (apr_pools.c:1680)
>> ==25549==    by 0x4E3D696: svn_ra_open4 (ra_loader.c:431)
>> ==25549==    by 0x403F57: open_target_session (svnsync.c:996)
>> ==25549==    by 0x403BE6: initialize_cmd (svnsync.c:905)
>> ==25549==    by 0x407384: sub_main (svnsync.c:2380)
>> ==25549==    by 0x407463: main (svnsync.c:2414)
>>
>> This is with 1.9.x.
> This worked in 1.8 and the breaking change is r1667829:
>
> Merge the r1664078 group from trunk:
>
>  * r1664078,r1664080,r1664187,r1664191,r1664200,r1664344,r1664588,r1664927,r1665886
>    Instead of making more changes to the auth batons from ra sessions, reduce
>    the number of changes by introducing an internal slave auth baton feature.
>    Justification:
>      Without this patch (or a complete redesign of the auth layer), the
>      ra sessions cache (currently on a feature branch), will open the ra
>      sessions from outside configuration changes caused by opening other
>      ra sessions. This patch not only reverts the additional changes to the
>      auth baton on init that are new in 1.9, but also removes cases where we
>      already applied similar changes inside specific ra providers.
>    Notes:
>      The reason I group this under release blockers, is to avoid the behavior
>      change introduced in r1609499 from reaching released versions. The changes
>      itself are safe for a later backport as it only affects ra-session
>      internal state.
>    Votes:
>      +1: rhuijben, brane, philip
>
> in particular passing scratch_pool to the RA initfunc.  Reverting to
> passing the parent sesspool like 1.8:
>
> Index: ../src-1.9/subversion/libsvn_ra/ra_loader.c
> ===================================================================
> --- ../src-1.9/subversion/libsvn_ra/ra_loader.c	(revision 1677108)
> +++ ../src-1.9/subversion/libsvn_ra/ra_loader.c	(working copy)
> @@ -355,7 +355,7 @@ svn_error_t *svn_ra_open4(svn_ra_session_t **sessi
>              /* Library not found. */
>              continue;
>  
> -          SVN_ERR(initfunc(svn_ra_version(), &vtable, scratch_pool));
> +          SVN_ERR(initfunc(svn_ra_version(), &vtable, sesspool));
>  
>            SVN_ERR(check_ra_version(vtable->get_version(), scheme));
>
> allows the initialize to work again.

It gets worse, actually. The FSFS 'common pool' in which that shared
data is allocated is created as a subpool of whatever is sent to the
initfunc (in svn_fs_initialize). That's really broken IMO, because a
second session created from a different pool will get its FS struct
pulled from under it, if I'm reading the code correctly.

See libsvn_fs/fs-loader.c lines 390 to 420.

-- Brane

Mime
View raw message