On Mon, Jan 9, 2012 at 10:16 AM, Paul Burba <ptbu...@gmail.com> wrote: > On Thu, Jan 5, 2012 at 2:37 PM, Stefan Küng <tortoise...@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
