David Glasser wrote:
> On Feb 1, 2008 4:31 PM, David Glasser <glasser_at_davidglasser.net> wrote:
>> This revision caused the segfault described in
>> http://svn.haxx.se/dev/archive-2008-02/0064.shtml.
>>
>> (Ah, binary search.)
>>
>> (Ah, pools.)
>
> I've tracked it down a little more, to svn_client_merge_peg3:
>
>>> @@ -4801,6 +4833,7 @@
>>> const char *wc_repos_root, *source_repos_root;
>>> svn_opt_revision_t working_rev;
>>> svn_ra_session_t *ra_session;
>>> + apr_pool_t *sesspool;
>>>
>>> /* No ranges to merge? No problem. */
>>> if (ranges_to_merge->nelts == 0)
>>> @@ -4828,9 +4861,10 @@
>>> &working_rev, adm_access, ctx, pool));
>>>
>>> /* Open an RA session to our source URL, and determine its root URL. */
>>> + sesspool = svn_pool_create(pool);
>>> SVN_ERR(svn_client__open_ra_session_internal(&ra_session,
>>> URL, NULL, NULL, NULL,
>>> - FALSE, FALSE, ctx, pool));
>>> + FALSE, TRUE, ctx, sesspool));
>>> SVN_ERR(svn_ra_get_repos_root(ra_session, &source_repos_root, pool));
>>>
>>> /* Normalize our merge sources. */
>>> @@ -4838,6 +4872,9 @@
>>> source_repos_root, peg_revision,
>>> ranges_to_merge, ra_session, ctx, pool));
>>>
>>> + /* We're done with our little RA session. */
>>> + svn_pool_destroy(sesspool);
>>> +
>>> /* Do the real merge! (We say with confidence that our merge
>>> sources are both ancestral and related.) */
>>> SVN_ERR(do_merge(merge_sources, target_wcpath, entry, adm_access,
>
> Swapping this svn_pool_destroy with the do_merge that follows it saves
> the day. Perhaps merge_sources contains something allocated in
> sesspool?
This might be another of those instances where the repos_root's pool is
prematurely destroyed. In another of a series of "let's design a
non-obvious API" decisions, svn_ra_get_repos_root() apparently returns its
value allocated in the session pool, *not* in the apparently unused
passed-in pool (like svn_ra_get_uuid does). I can almost guarantee that
this bit me in this change, and likely in other changes I've made, too.
The right fix, in my opinion, is to rev svn_ra_get_repos_root() and
svn_ra_get_repos_uuid() and promise to allocate the returned data in the
provided pool, which is what I think most folks would expect from an API
that actually accepts a pool parameter.
--
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Received on 2008-02-02 04:34:24 CET