[PATCH] fix for issue 2576 ( was: RE: [BUG] regression: 'svn commit -N folder folder' will crash svn 1.4 and trunk)

[Lieven Govaerts <lg...@mobsol.be>]

Hi, 

this issue is now known as issue #2576:
http://subversion.tigris.org/issues/show_bug.cgi?id=2576.

I think what happens is this:

example: svn ci -m "log msg" -N commit_tests-35 commit_tests-35\iota
commit_tests-35\A.

Normally (without -N) svn will calculate the shared base folder
(commit_tests-35), lock that folder and automatically lock all subfolders. A
and iota are considered as relative paths, and svn just checks if there need
to be other locks set.

With the -N option, that calculation of the shared base folder is not
executed, so svn will see three 'relative' paths:
commit_tests-35, commit_tests-35\iota and commit_tests-35\A. svn will add
both commit_tests-35 and commit_tests-35\A to the list of to-be-locked
folders. Problem is that folder commit_tests-35 is already locked (because
it's the base folder) and then locked again because it's in the list
dirs_to_lock. Locking a folder twice fails.

To make it worse, the safety measure that aborts the while loop to find the
parent folders doesn't work on Windows, because the root folder '/' is not
recognized. So on Linux this code just aborts, on Windows it will loop until
we get a stack overflow:
if (target[0] == '/' && target[1] == '\0')                    
  abort();

Attached is a patch that solves this issue. Please review thoroughly,
because I'm not familiar with this code. 

regards,

Lieven.
[[[
Fix for issue 2576: Don't lock the base folder twice when
committing overlapping targets non-recursively.

* subversion/libsvn_client/commit.c
  (svn_client_commit3): add extra checks to avoid adding the
   base folder to dirs_to_lock. 
   Catch a potential stack overflow on Windows, abort()
   instead.

* subversion/tests/cmdline/commit_tests.py
  (commit_same_folder_in_targets): New test
  (test_list): Run the new commit test.

]]]

> -----Original Message-----
> From: Lieven Govaerts [mailto:lgo@mobsol.be] 
> Sent: zondag 25 juni 2006 8:49
> To: dev@subversion.tigris.org
> Cc: 'Nicolás Lichtmaier'
> Subject: [BUG] regression: 'svn commit -N folder folder' will 
> crash svn 1.4 and trunk (was: Crash in 1.4 with "svn commit 
> -N . something")
> 
> Hi, 
> 
> 
> a few days ago Nicolás reported a bug on this list
> (http://svn.haxx.se/dev/archive-2006-06/0689.shtml) that 
> seems to be a regression compared to 1.3. I'm reposting it to 
> make sure we solve it in 1.4.
> 
> The problem:
> 1. checkout a branch in /tmp/trunk
> 2. alter a file test in the trunk wc
> 3. svn commit -N /tmp/trunk /tmp/trunk/test
> 
> On my machine (windows xp) with svn 1.4_RC1 and trunk, the 
> svn process will start using huge amounts of memory 
> (increasing 100MB per sec) before it crashes. Server is 
> apache 2.0.55 with svn trunk modules.
> 
> Attached is a python test that triggers this problem. Please 
> do not commit this to the repository before the issue is 
> solved! The original mail from Nicolás also contains a short 
> analysis of the problem and a patch.
> 
> Nicolás, can you report this in the issue tracker? Otherwise 
> I'll do it.
> 
> regards,
> 
> Lieven.
> 
> [[[
> Test committing two overlapping targets non-recursively.
> 
> * subversion/tests/cmdline/commit_tests.py
>   (commit_same_folder_in_targets): New test
>   (test_list): Run the new commit test.
> ]]]
> 

Re: [PATCH] fix for issue 2576 ( was: RE: [BUG] regression: 'svn commit -N folder folder' will crash svn 1.4 and trunk)

[Garrett Rooney <ro...@electricjellyfish.net>]

On 6/26/06, Lieven Govaerts <lg...@mobsol.be> wrote:
> Hi,
>
> this issue is now known as issue #2576:
> http://subversion.tigris.org/issues/show_bug.cgi?id=2576.
>
> I think what happens is this:
>
> example: svn ci -m "log msg" -N commit_tests-35 commit_tests-35\iota
> commit_tests-35\A.
>
> Normally (without -N) svn will calculate the shared base folder
> (commit_tests-35), lock that folder and automatically lock all subfolders. A
> and iota are considered as relative paths, and svn just checks if there need
> to be other locks set.
>
> With the -N option, that calculation of the shared base folder is not
> executed, so svn will see three 'relative' paths:
> commit_tests-35, commit_tests-35\iota and commit_tests-35\A. svn will add
> both commit_tests-35 and commit_tests-35\A to the list of to-be-locked
> folders. Problem is that folder commit_tests-35 is already locked (because
> it's the base folder) and then locked again because it's in the list
> dirs_to_lock. Locking a folder twice fails.
>
> To make it worse, the safety measure that aborts the while loop to find the
> parent folders doesn't work on Windows, because the root folder '/' is not
> recognized. So on Linux this code just aborts, on Windows it will loop until
> we get a stack overflow:
> if (target[0] == '/' && target[1] == '\0')
>   abort();
>
> Attached is a patch that solves this issue. Please review thoroughly,
> because I'm not familiar with this code.

Seems fine to me.  Applied in r20276.  Thanks!

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org