You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Küng <to...@gmail.com> on 2012/03/26 08:22:47 UTC

SVN_ERR_ASSERT with reintegrate

Hi,

There's an SVN_ERR_ASSERT when reintegrating. From the crash statistics 
I get this happens quite often. The stack trace:

libsvn_tsvn!svn_ra_get_location_segments+0x12
libsvn_tsvn!svn_client__repos_location_segments+0x9f
libsvn_tsvn!svn_client_merge4+0xff8
libsvn_tsvn!svn_client_merge4+0x172e
libsvn_tsvn!svn_client_merge_reintegrate+0x113
tortoiseproc!SVN::MergeReintegrate+0x12c

in ra_loader.c, function svn_ra_get_location_segments() the
   SVN_ERR_ASSERT(*path != '/');
is hit.

the crash dumps I got (8 of them as of now) are not very clear: I'm not 
sure if it's the assert that's triggered or whether 'path' is actually 
NULL. Because the crash dump result in:
0xC0000005: Access violation reading location 0x0000000000000000.

 From looking at the code I think this would happen if someone tries to 
merge from or to a repo root. So instead of asserting or segfaulting, 
maybe returning an error like "you can't merge from/to a repo root" or 
something like that would be much better.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net

Re: SVN_ERR_ASSERT with reintegrate

Posted by Julian Foad <ju...@btopenworld.com>.
Stefan Küng wrote:

> On 27.03.2012 12:06, Julian Foad wrote:
>>  Stefan Küng<to...@gmail.com>  wrote:
>> 
>>>  There's an SVN_ERR_ASSERT when reintegrating. [...]
>>> 
>>>  libsvn_tsvn!svn_ra_get_location_segments+0x12
>>>  libsvn_tsvn!svn_client__repos_location_segments+0x9f
>>>  libsvn_tsvn!svn_client_merge4+0xff8
>>>  libsvn_tsvn!svn_client_merge4+0x172e
>>>  libsvn_tsvn!svn_client_merge_reintegrate+0x113
>>>  tortoiseproc!SVN::MergeReintegrate+0x12c
>>> 
>>>  in ra_loader.c, function svn_ra_get_location_segments() the
>>>     SVN_ERR_ASSERT(*path != '/');
>>>  is hit.
[...]
>>>   From looking at the code I think this would happen if someone
>>> tries to  merge  from or to a repo root. [...]
>> 
>>  Yup.  In trunk there is such a check in find_reintegrate_merge:
[...]
>>  First included in release 1.6.10. [...]
> 
> Strange. The crash dumps are for the latest 1.7.4 release (TSVN 1.7.6). 
> So the libraries are from that version as well.

OK.  In that case I suspect that it's not being caused by a merge to or from the repo root, but by some other conditions in which reintegrate tries to calculate a relative path to 

I suspect the invalid rel-path to svn_client__repos_location_segments() must occur in one of these two places (quoting from 1.7.4, abbreviating):

static svn_error_t *
find_unmerged_mergeinfo(... source_catalog ... source_repos_rel_path ...)
{ ...
      for (hi = apr_hash_first(scratch_pool, source_catalog); ...)
        {
          source_path = svn__apr_hash_index_key(hi);
          source_path_rel_to_session =
            svn_relpath_skip_ancestor(source_repos_rel_path, source_path);
          ...
            /* Get the source path's natural history and convert it to
               mergeinfo.  Then merge that natural history into source
               path's explicit or inherited mergeinfo. */
            svn_client__repos_location_segments(
              ..., source_path_rel_to_session, ...));
}

static svn_error_t *
calculate_left_hand_side(... target_repos_rel_path,
                         subtrees_with_mergeinfo,
                         target_repos_root, ...)
{ ...
  /* Get the history (segments) for TARGET_ABSPATH and any of its subtrees
     with explicit mergeinfo. */
  for (hi = apr_hash_first(scratch_pool, subtrees_with_mergeinfo); ...)
    {
      absolute_path = svn__apr_hash_index_key(hi);
      svn_client__path_relative_to_root(
        &path_rel_to_root, ..., absolute_path, target_repos_root, ...)
      path_rel_to_session = svn_relpath_skip_ancestor(
                              target_repos_rel_path,path_rel_to_root);
      svn_client__repos_location_segments(... path_rel_to_session ...)
}

But I can't (yet) figure out by inspection what merge inputs would cause that.

In trunk, some time in the last few months I changed these code paths so that this can't happen.  Instead of calculating a relpath (relative to the session), instead the code now reparents the session to the desired URL and passes a literal "" as the relpath argument to svn_ra_get_location_segments().  My intention was just to make the code simpler to read, but the change may have had the effect of eliminating some latent bugs as well.  (Of course I accept that it could have introduced bugs as well, or just changed the result from an assertion failure to quietly doing something wrong.)

We ought to back-port some sort of fix or improved failure mode to 1.7.x when we figure it out.  If nothing else, we could throw an error if the relpath comes out as NULL in the two code places I quoted above.

- Julian

Re: SVN_ERR_ASSERT with reintegrate

Posted by Stefan Küng <to...@gmail.com>.
On 27.03.2012 12:06, Julian Foad wrote:
> Stefan Küng<to...@gmail.com>  wrote:
>
>> There's an SVN_ERR_ASSERT when reintegrating. From the crash statistics I
>> get this happens quite often. The stack trace:
>>
>> libsvn_tsvn!svn_ra_get_location_segments+0x12
>> libsvn_tsvn!svn_client__repos_location_segments+0x9f
>> libsvn_tsvn!svn_client_merge4+0xff8
>> libsvn_tsvn!svn_client_merge4+0x172e
>> libsvn_tsvn!svn_client_merge_reintegrate+0x113
>> tortoiseproc!SVN::MergeReintegrate+0x12c
>>
>> in ra_loader.c, function svn_ra_get_location_segments() the
>>    SVN_ERR_ASSERT(*path != '/');
>> is hit.
>
> Hi Stefan.  Thanks for that.  I'll take a look.
>
>> the crash dumps I got (8 of them as of now) are not very clear: I'm not sure
>> if it's the assert that's triggered or whether 'path' is
>> actually NULL. Because the crash dump result in:
>> 0xC0000005: Access violation reading location 0x0000000000000000.
>
> That pretty much guarantees that 'path' was NULL, I would say.
>
>>  From looking at the code I think this would happen if someone tries to merge
>> from or to a repo root. So instead of asserting or segfaulting, maybe returning
>> an error like "you can't merge from/to a repo root" or something
>> like that would be much better.
>
> Yup.  In trunk there is such a check in find_reintegrate_merge:
>
>    /* Can't reintegrate to or from the root of the repository. */
>    if (strcmp(source_loc->url, source_loc->repo->url) == 0
>        || strcmp(target->loc.url, target->loc.repo->url) == 0)
>      return svn_error_createf(SVN_ERR_CLIENT_NOT_READY_TO_MERGE, NULL,
>                               _("Neither the reintegrate source nor target "
>                                 "can be the root of the repository"));
>
> This was added in r877459 - pburba 20-Apr-2009:
>    Return an error if attempting to reintegrate from/to the repository root.
>
>    * subversion/libsvn_client/merge.c
>      (svn_client_merge_reintegrate): Error out as soon as an attempt is made to
>      reintegrate to or from the repository root rather than attempting this
>      and getting a cryptic assertion, see
>      http://svn.haxx.se/dev/archive-2009-04/0623.shtml.
>
> Merged to 1.6.x in r877630 | hwright | 2009-05-04.
>
> First included in release 1.6.10.
>
> Maybe this is not in the version that these users are running.
>
> What version(s) of Subversion libraries are these crashes in?

Strange. The crash dumps are for the latest 1.7.4 release (TSVN 1.7.6). 
So the libraries are from that version as well.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net

Re: SVN_ERR_ASSERT with reintegrate

Posted by Julian Foad <ju...@btopenworld.com>.
Stefan Küng <to...@gmail.com> wrote:

> There's an SVN_ERR_ASSERT when reintegrating. From the crash statistics I 
> get this happens quite often. The stack trace:
> 
> libsvn_tsvn!svn_ra_get_location_segments+0x12
> libsvn_tsvn!svn_client__repos_location_segments+0x9f
> libsvn_tsvn!svn_client_merge4+0xff8
> libsvn_tsvn!svn_client_merge4+0x172e
> libsvn_tsvn!svn_client_merge_reintegrate+0x113
> tortoiseproc!SVN::MergeReintegrate+0x12c
> 
> in ra_loader.c, function svn_ra_get_location_segments() the
>   SVN_ERR_ASSERT(*path != '/');
> is hit.

Hi Stefan.  Thanks for that.  I'll take a look.

> the crash dumps I got (8 of them as of now) are not very clear: I'm not sure 
> if it's the assert that's triggered or whether 'path' is 
> actually NULL. Because the crash dump result in:
> 0xC0000005: Access violation reading location 0x0000000000000000.

That pretty much guarantees that 'path' was NULL, I would say.

> From looking at the code I think this would happen if someone tries to merge 
> from or to a repo root. So instead of asserting or segfaulting, maybe returning 
> an error like "you can't merge from/to a repo root" or something 
> like that would be much better.

Yup.  In trunk there is such a check in find_reintegrate_merge:

  /* Can't reintegrate to or from the root of the repository. */
  if (strcmp(source_loc->url, source_loc->repo->url) == 0
      || strcmp(target->loc.url, target->loc.repo->url) == 0)
    return svn_error_createf(SVN_ERR_CLIENT_NOT_READY_TO_MERGE, NULL,
                             _("Neither the reintegrate source nor target "
                               "can be the root of the repository"));

This was added in r877459 - pburba 20-Apr-2009:
  Return an error if attempting to reintegrate from/to the repository root.

  * subversion/libsvn_client/merge.c
    (svn_client_merge_reintegrate): Error out as soon as an attempt is made to
    reintegrate to or from the repository root rather than attempting this
    and getting a cryptic assertion, see
    http://svn.haxx.se/dev/archive-2009-04/0623.shtml.

Merged to 1.6.x in r877630 | hwright | 2009-05-04.

First included in release 1.6.10.

Maybe this is not in the version that these users are running.

What version(s) of Subversion libraries are these crashes in?

- Julian