On 01.05.2015 18:44, Philip Martin wrote: > Philip Martin <philip.mar...@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
