Re: segfault when deleting folders in 1.6.2

On Sun, May 31, 2009 at 7:35 AM, Stefan Küng <tortoisesvn_at_gmail.com> wrote:
> On Thu, May 21, 2009 at 10:22, Stefan Küng <tortoisesvn_at_gmail.com> wrote:
>> Hi,
>>
>> In the file libsvn_wc/adm_ops.c, function
>> svn_wc_remove_from_revision_control(), line 2628:
>>
>>        dir_entry = apr_hash_get(parent_entries, base_name,
>>                                                         APR_HASH_KEY_STRING);
>>        if (dir_entry->depth != svn_depth_exclude)
>>          {
>>                svn_wc__entry_remove(parent_entries, base_name);
>>                SVN_ERR(svn_wc__entries_write(parent_entries, parent_access, pool));
>>
>>          }
>>
>> the 'dir_entry' is NULL, and accessing it in the if-clause causes a
>> segfault.
>> From the crash dump I got, I can't see anything special about the path
>> (it was F:\dev\svn\name\name), so no root path, no UNC path or anything
>> else special I can think of.
>>
>> But the delete was called with --keep-local, so maybe that has something
>> to do with it?
>
> Some more information:
> the folder which should be deleted is tree-conflicted (deleted in
> repository, still present locally).
>
> Stefan

Attached is a patch that adds a new update test to replicate this
segfault, along with the very simple fix to address it.

I'm going to hold off on committing though for two reasons:

First, while this fixes the problem for 1.6.x, when running this new
test on trunk it triggers an assert on line 2051 in
entries.c:write_entry():

  2048 if (entry->keep_local)
  2049 {
  2050 SVN_ERR_ASSERT(working_node != NULL);
  2051 SVN_ERR_ASSERT(entry->schedule == svn_wc_schedule_delete);
  2052 working_node->keep_local = TRUE;
  2053 }

This assert was added in r36105, but I can't figure out what its
purpose is. Putting aside tree conflicts for a moment we can trigger
this assert by simply deleting a path with a locally added subtree,
e.g.:

trunk_at_37912>svn st -v
                 1 1 jrandom .
                 1 1 jrandom A
                 1 1 jrandom A\mu
                 1 1 jrandom A\B
                 1 1 jrandom A\B\lambda
                 1 1 jrandom A\B\E
                 1 1 jrandom A\B\E\alpha
                 1 1 jrandom A\B\E\beta
                 1 1 jrandom A\B\F
                 1 1 jrandom A\C
                 1 1 jrandom A\D
                 1 1 jrandom A\D\gamma
                 1 1 jrandom A\D\G
                 1 1 jrandom A\D\G\rho
                 1 1 jrandom A\D\G\pi
                 1 1 jrandom A\D\G\tau
                 1 1 jrandom A\D\H
                 1 1 jrandom A\D\H\chi
                 1 1 jrandom A\D\H\omega
                 1 1 jrandom A\D\H\psi
                 1 1 jrandom iota

trunk_at_37912>svn copy A\D A\C
A A\C\D

trunk_at_37912>svn delete A\C --keep-local
D A\C\D\gamma
D A\C\D\G\rho
D A\C\D\G\pi
D A\C\D\G\tau
D A\C\D\G
D A\C\D\H\chi
D A\C\D\H\omega
D A\C\D\H\psi
D A\C\D\H
..\..\..\subversion\libsvn_wc\wc_db.c:2240: (apr_err=235000)
svn: In file '..\..\..\subversion\libsvn_wc\entries.c' line 2051:
assertion failed (entry->schedule == svn_wc_schedule_delete)

Hyrum, I'm ccing you in the hope you can explain the purpose of this
assert (apologies if I'm being dense!).

~~~~~

Second, I'm wondering what additional limitations we should consider
putting on operations on a tree conflicted directory before that TC is
resolved or reverted. Obviously we can't commit a tree conflicted WC,
but should we allow other operations, copies, moves, deletes, etc., of
WC paths which are either a subtree of a tree conflicted directory or
have a subtree that is tree conflicted. Or should all this be allowed
and should it "work" (however that might be defined)? I'm CCing you
TC guys in the hopes of prying some wisdom from your brains on this
;-)

Paul

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2358905