On Mon, Jan 9, 2012 at 10:16 AM, Paul Burba <ptburba_at_gmail.com> wrote:
> On Thu, Jan 5, 2012 at 2:37 PM, Stefan Küng <tortoisesvn_at_gmail.com> wrote:
>> Hi,
>>
>> From a crash report dump sent for TSVN for an update the attached stack
>> trace happened.
>>
>> libsvn_wc\props.c, svn_wc__merge_props()
>> to_val = incoming_change->value
>> ? svn_string_dup(incoming_change->value, result_pool) : NULL;
>> ....
>> if (! from_val) /* adding a new property */
>> SVN_ERR(apply_single_prop_add(state, &conflict_remains,
>> db, local_abspath,
>> left_version, right_version,
>> is_dir, actual_props,
>> propname, base_val, to_val,
>> conflict_func, conflict_baton,
>> dry_run, result_pool, iterpool));
>> and then in apply_single_prop_add():
>> ...
>> if (svn_string_compare(working_val, new_val))
>> crashes, because new_val is NULL.
>>
>> the property is "svn:mergeinfo".
>
>
> Hi Stefan,
>
> I saw 'mergeinfo' so took a quick look (happily it has nothing to do
> with mergeinfo per se, any property will do ;-)
>
>> Could this happen if someone manually removes the svn:mergeinfo property and
>> then the next update (from another working copy) gets the to_val as NULL?
>
> I'm not sure if this is what you mean, but I see one way to replicate
> this problem: Create a file external mapping to a file with some
> arbitrary property. Remap the same external to a file without that
> property. Boom.
>
> I filed an issue (issue #4093) which gives the full replication
> recipe. I also added a regression test in r1228340.
Hi Bert,
I traced this regression back to your change in r1124782:
> Modified: subversion/trunk/subversion/libsvn_wc/externals.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/externals.c?rev=1124782&r1=1124781&r2=1124782&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_wc/externals.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/externals.c Thu May 19 13:53:20 2011
.
<snip>
.
> @@ -607,41 +607,17 @@ close_file(void *file_baton,
> svn_skel_t *all_work_items = NULL;
> svn_skel_t *work_item;
> apr_hash_t *base_props = NULL;
base_props set to null...
> apr_hash_t *actual_props = NULL;
> apr_hash_t *new_pristine_props = NULL;
> apr_hash_t *new_actual_props = NULL;
> apr_hash_t *new_dav_props = NULL;
> const svn_checksum_t *new_checksum = NULL;
> const svn_checksum_t *original_checksum = NULL;
> - svn_revnum_t changed_rev = SVN_INVALID_REVNUM;
> - apr_time_t changed_date = 0;
> - const char *changed_author = NULL;
> +
> svn_boolean_t added = !SVN_IS_VALID_REVNUM(eb->original_revision);
> const char *repos_relpath = svn_uri_is_child(eb->repos_root_url,
> eb->url, pool);
>
> if (! added)
> {
> - svn_boolean_t had_props;
> - svn_boolean_t props_mod;
> + new_checksum = eb->original_checksum;
>
> - SVN_ERR(svn_wc__db_external_read(NULL, NULL, NULL, NULL, NULL, NULL,
> - &changed_rev, &changed_date,
> - &changed_author, &original_checksum,
> - NULL, NULL, NULL, NULL, NULL, NULL,
> - NULL, NULL, NULL, &had_props,
> - &props_mod,
> - eb->db, eb->local_abspath,
> - eb->wri_abspath,
> - pool, pool));
> -
> - new_checksum = original_checksum;
> -
> - if (had_props)
> - SVN_ERR(svn_wc__db_external_read_pristine_props(&base_props, eb->db,
> - eb->local_abspath,
> - eb->wri_abspath,
> - pool, pool));
but we no longer populate base_props...
> - if (props_mod)
> - SVN_ERR(svn_wc__db_external_read_props(&actual_props, eb->db,
> - eb->local_abspath,
> - eb->wri_abspath,
> - pool, pool));
> + SVN_ERR(svn_wc__db_base_get_props(&actual_props, eb->db,
> + eb->local_abspath, pool, pool));
> }
>
> if (!base_props)
> base_props = apr_hash_make(pool);
So base_props is unconditionally an empty hash. Which causes the
segfault when close_file calls svn_wc__merge_props with an empty hash
as the PRISTINE_PROPS argument. svn_wc__merge_props has a prop
deletion, but the empty hash makes it think it has a prop addition:
[[[
svn_error_t *
svn_wc__merge_props(svn_skel_t **work_items,
svn_wc_notify_state_t *state,
apr_hash_t **new_pristine_props,
apr_hash_t **new_actual_props,
svn_wc__db_t *db,
const char *local_abspath,
svn_kind_t kind,
const svn_wc_conflict_version_t *left_version,
const svn_wc_conflict_version_t *right_version,
apr_hash_t *server_baseprops,
apr_hash_t *pristine_props,
apr_hash_t *actual_props,
const apr_array_header_t *propchanges,
svn_boolean_t base_merge,
svn_boolean_t dry_run,
svn_wc_conflict_resolver_func2_t conflict_func,
void *conflict_baton,
svn_cancel_func_t cancel_func,
void *cancel_baton,
apr_pool_t *result_pool,
apr_pool_t *scratch_pool)
{
<snip>
if (!server_baseprops)
server_baseprops = pristine_props;
<snip>
/* Looping over the array of incoming propchanges we want to apply: */
iterpool = svn_pool_create(scratch_pool);
for (i = 0; i < propchanges->nelts; i++)
{
<snip>
to_val = incoming_change->value
? svn_string_dup(incoming_change->value, result_pool) : NULL;
from_val = apr_hash_get(server_baseprops, propname, APR_HASH_KEY_STRING);
base_val = apr_hash_get(pristine_props, propname, APR_HASH_KEY_STRING);
<snip>
if (! from_val) /* adding a new property */
SVN_ERR(apply_single_prop_add(state, &conflict_remains,
db, local_abspath,
left_version, right_version,
is_dir, actual_props,
propname, base_val, to_val,
### ^^^^
### This is obviously going to blow up since to_val is null!
conflict_func, conflict_baton,
dry_run, result_pool, iterpool));
]]]
So it seems we need to put
subversion/libsvn_wc/wc_db.c:svn_wc__db_external_read_pristine_props()
back. Does that seem right to you, or is there a better way?
Paul
Received on 2012-01-10 17:36:41 CET