You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "D.J. Heap" <dj...@shadyvale.net> on 2005/04/01 23:04:32 UTC
Win32 Tests -- locks-test
I'm having trouble with Win32 tests on trunk -- locks-test.exe hangs
with the following call stack:
ntdll.dll!_KiFastSystemCallRet@0
ntdll.dll!_NtLockFile@40
kernel32.dll!_LockFileEx@24
> libapr.dll!apr_file_lock
locks-test.exe!svn_io_file_lock2
locks-test.exe!svn_fs_fs__get_write_lock
locks-test.exe!svn_fs_fs__commit
locks-test.exe!svn_fs_fs__commit_txn
locks-test.exe!svn_fs_commit_txn
locks-test.exe!lock_credentials
locks-test.exe!do_test_num
locks-test.exe!main
locks-test.exe!mainCRTStartup
kernel32.dll!_BaseProcessStart@4
The tests ran fine about 2 weeks ago. It is blocking on LockFileEx
because the file (repos\write-lock) has already been locked once.
Can someone else confirm/deny this result on Win32? Is apr_file_lock's
'exclusive lock' behavior semantically different on Unixes or is this
test hanging on Linux also? I haven't deeply investigated yet, but will
try to this weekend. It seems like it could be caused by a previous
test function not cleaning everything, but that is just a guess.
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Win32 Tests -- locks-test
Posted by Greg Hudson <gh...@MIT.EDU>.
On Mon, 2005-04-04 at 09:18, Branko Čibej wrote:
> I'd definitely want to see this fixed and tested on trunk *before* we
> branch 1.2. This is a major bug, and it makes no sense to start the 1.2
> soak period while it's in the code.
While I agree that this bug should block the release, I don't agree that
it has to block the branch. If we have a known bug but expect the fix
to be short, we can branch with it; small bugfixes only reset the last
week of our four-week soak period, not the whole thing.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Win32 Tests -- locks-test
Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Mon, 4 Apr 2005, [UTF-8] Branko �^Libej wrote:
> Peter N. Lundblad wrote:
>
> >It's on trunk now. r13906. If someone could runtests on Windows, it'd be
> >great. (don't forget FS_TYPE=fsfs or whatever it is called in the windows
> >tests.:-) It passes on Linux (of course).
> >
> >
> Sorry, no go. locks-test still deadlocks, although a bit later (the
> first 9 tests pass). I don't have time right now to look at the details,
> though.
Thanks for testing! Erik H also ran the testa and came to the same
conclusion.
The problem is that auto-expiration tries to grab the write lock even if a
caller already has it. The has_write_lock parameter to svn_fs_fs__get_lock
doesn't always get set correctly.
Regards,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Win32 Tests -- locks-test
Posted by Branko Čibej <br...@xbc.nu>.
Peter N. Lundblad wrote:
>It's on trunk now. r13906. If someone could runtests on Windows, it'd be
>great. (don't forget FS_TYPE=fsfs or whatever it is called in the windows
>tests.:-) It passes on Linux (of course).
>
>
Sorry, no go. locks-test still deadlocks, although a bit later (the
first 9 tests pass). I don't have time right now to look at the details,
though.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Win32 Tests -- locks-test
Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Mon, 4 Apr 2005, [UTF-8] Branko �^Libej wrote:
> Peter N. Lundblad wrote:
>
> >On Sun, 3 Apr 2005, D.J. Heap wrote:
> >
> >
> >
> >>Peter N. Lundblad wrote:
> >>[snip]
> >>
> >>
> >>>Well, the pool is cleared, eventually. We don't clear subpools on error.
> >>>I'm going to change this particular case (holding the write lock in FSFS),
> >>>since we don't know when the caller will clear the pool and locks should
> >>>be released when they're not needed anymore.
> >>>
> >>>
> >>>
> >>Sounds good to me. There are 3 or so other places where
> >>svn_fs_fs__get_write_lock is called and probably needs the same
> >>treatment on errors, I would think. I can post a patch for those later,
> >>if you don't get them.
> >>
> >>
> >>
> >I am replacing get_write_lock with a new with_write_lock function taking
> >another function/baton pair. That's happening in all places, including
> >commit.
> >
> >I timed out yesterday, but I'll finish it tonight. Hopefully before we
> >merge for 1.2, if that's actually happening today.
> >
> >
> I'd definitely want to see this fixed and tested on trunk *before* we
> branch 1.2. This is a major bug, and it makes no sense to start the 1.2
> soak period while it's in the code.
>
It's on trunk now. r13906. If someone could runtests on Windows, it'd be
great. (don't forget FS_TYPE=fsfs or whatever it is called in the windows
tests.:-) It passes on Linux (of course).
Regards,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Win32 Tests -- locks-test
Posted by Branko Čibej <br...@xbc.nu>.
Peter N. Lundblad wrote:
>On Sun, 3 Apr 2005, D.J. Heap wrote:
>
>
>
>>Peter N. Lundblad wrote:
>>[snip]
>>
>>
>>>Well, the pool is cleared, eventually. We don't clear subpools on error.
>>>I'm going to change this particular case (holding the write lock in FSFS),
>>>since we don't know when the caller will clear the pool and locks should
>>>be released when they're not needed anymore.
>>>
>>>
>>>
>>Sounds good to me. There are 3 or so other places where
>>svn_fs_fs__get_write_lock is called and probably needs the same
>>treatment on errors, I would think. I can post a patch for those later,
>>if you don't get them.
>>
>>
>>
>I am replacing get_write_lock with a new with_write_lock function taking
>another function/baton pair. That's happening in all places, including
>commit.
>
>I timed out yesterday, but I'll finish it tonight. Hopefully before we
>merge for 1.2, if that's actually happening today.
>
>
I'd definitely want to see this fixed and tested on trunk *before* we
branch 1.2. This is a major bug, and it makes no sense to start the 1.2
soak period while it's in the code.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Win32 Tests -- locks-test
Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Sun, 3 Apr 2005, D.J. Heap wrote:
> Peter N. Lundblad wrote:
> [snip]
> > Well, the pool is cleared, eventually. We don't clear subpools on error.
> > I'm going to change this particular case (holding the write lock in FSFS),
> > since we don't know when the caller will clear the pool and locks should
> > be released when they're not needed anymore.
> >
> Sounds good to me. There are 3 or so other places where
> svn_fs_fs__get_write_lock is called and probably needs the same
> treatment on errors, I would think. I can post a patch for those later,
> if you don't get them.
>
I am replacing get_write_lock with a new with_write_lock function taking
another function/baton pair. That's happening in all places, including
commit.
I timed out yesterday, but I'll finish it tonight. Hopefully before we
merge for 1.2, if that's actually happening today.
Thanks,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Win32 Tests -- locks-test
Posted by "D.J. Heap" <dj...@shadyvale.net>.
Peter N. Lundblad wrote:
[snip]
> Well, the pool is cleared, eventually. We don't clear subpools on error.
> I'm going to change this particular case (holding the write lock in FSFS),
> since we don't know when the caller will clear the pool and locks should
> be released when they're not needed anymore.
>
> Regards,
> //Peter
>
Sounds good to me. There are 3 or so other places where
svn_fs_fs__get_write_lock is called and probably needs the same
treatment on errors, I would think. I can post a patch for those later,
if you don't get them.
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Win32 Tests -- locks-test
Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Sun, 3 Apr 2005, [UTF-8] Branko �^Libej wrote:
> D.J. Heap wrote:
>
> >On Sun, 2005-04-03 at 09:21 -0600, D.J. Heap wrote:
> >I checked the docs for apr_file_lock:
> >
> >"Establish a lock on the specified, open file. The lock may be advisory
> >or mandatory, at the discretion of the platform. The lock applies to the
> >file as a whole, rather than a specific range. Locks are established on
> >a per-thread/process basis; a second lock by the same thread will not
> >block."
> >
> >This is not how it behaves on Win32 with the way it uses LockFileEx.
> >
> >So, this appears to be an apr problem on Win32. I will investigate how
> >to make apr behave as documented on Win32, but probably Subversion will
> >need a workaround at least for a while.
> >
> >Any thoughts on a workaround? Only this one series of tests seems to be
> >a problem because of the way it shares the pool...perhaps the test could
> >be modified temporarily?
> >
> >
>
> I'd say that an exclusive lock is an exclusive lock. If the pool that
> "owns" the lock doesn't get cleared on error, then that's a bug. And a
> good thing that Windows exposes it.
>
Well, the pool is cleared, eventually. We don't clear subpools on error.
I'm going to change this particular case (holding the write lock in FSFS),
since we don't know when the caller will clear the pool and locks should
be released when they're not needed anymore.
Regards,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Win32 Tests -- locks-test
Posted by "D.J. Heap" <dj...@shadyvale.net>.
D.J. Heap wrote:
> Branko Čibej wrote:
> [snip]
>
>> I'd say that an exclusive lock is an exclusive lock. If the pool that
>> "owns" the lock doesn't get cleared on error, then that's a bug. And a
>> good thing that Windows exposes it.
>>
>> -- Brane
>
>
> So you think apr_file_lock just needs it's docs changed to indicate the
> difference in behavior on Win32? Or the docs and behavior on Unix
> changed to match Win32?
>
> DJ
>
>
Also, it seems like we should make bdb and fsfs behavior match too...
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Win32 Tests -- locks-test
Posted by Branko Čibej <br...@xbc.nu>.
D.J. Heap wrote:
> Branko Čibej wrote:
> [snip]
>
>> I'd say that an exclusive lock is an exclusive lock. If the pool that
>> "owns" the lock doesn't get cleared on error, then that's a bug. And
>> a good thing that Windows exposes it.
>>
>> -- Brane
>
>
> So you think apr_file_lock just needs it's docs changed to indicate
> the difference in behavior on Win32? Or the docs and behavior on Unix
> changed to match Win32?
As far as APR is concerned, the behaviour can be different, as long as
it's documented correctly.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Win32 Tests -- locks-test
Posted by "D.J. Heap" <dj...@shadyvale.net>.
Branko Čibej wrote:
[snip]
> I'd say that an exclusive lock is an exclusive lock. If the pool that
> "owns" the lock doesn't get cleared on error, then that's a bug. And a
> good thing that Windows exposes it.
>
> -- Brane
So you think apr_file_lock just needs it's docs changed to indicate the
difference in behavior on Win32? Or the docs and behavior on Unix
changed to match Win32?
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Win32 Tests -- locks-test
Posted by Branko Čibej <br...@xbc.nu>.
D.J. Heap wrote:
>On Sun, 2005-04-03 at 09:21 -0600, D.J. Heap wrote:
>[snip]
>
>
>>It seems like allowing the same process to re-lock could be bad for
>>threaded programs like svnserve/mod_dav_svn, but maybe threads count as
>>different processes on Unix?
>>
>>BDB obviously does not fail because it doesn't do locking this way, but
>>what does 'exclusive repo write lock' mean to it?
>>
>>DJ
>>
>>
>
>I checked the docs for apr_file_lock:
>
>"Establish a lock on the specified, open file. The lock may be advisory
>or mandatory, at the discretion of the platform. The lock applies to the
>file as a whole, rather than a specific range. Locks are established on
>a per-thread/process basis; a second lock by the same thread will not
>block."
>
>This is not how it behaves on Win32 with the way it uses LockFileEx.
>
>So, this appears to be an apr problem on Win32. I will investigate how
>to make apr behave as documented on Win32, but probably Subversion will
>need a workaround at least for a while.
>
>Any thoughts on a workaround? Only this one series of tests seems to be
>a problem because of the way it shares the pool...perhaps the test could
>be modified temporarily?
>
>
I'd say that an exclusive lock is an exclusive lock. If the pool that
"owns" the lock doesn't get cleared on error, then that's a bug. And a
good thing that Windows exposes it.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Win32 Tests -- locks-test
Posted by "D.J. Heap" <dj...@shadyvale.net>.
On Sun, 2005-04-03 at 09:21 -0600, D.J. Heap wrote:
[snip]
> It seems like allowing the same process to re-lock could be bad for
> threaded programs like svnserve/mod_dav_svn, but maybe threads count as
> different processes on Unix?
>
> BDB obviously does not fail because it doesn't do locking this way, but
> what does 'exclusive repo write lock' mean to it?
>
> DJ
I checked the docs for apr_file_lock:
"Establish a lock on the specified, open file. The lock may be advisory
or mandatory, at the discretion of the platform. The lock applies to the
file as a whole, rather than a specific range. Locks are established on
a per-thread/process basis; a second lock by the same thread will not
block."
This is not how it behaves on Win32 with the way it uses LockFileEx.
So, this appears to be an apr problem on Win32. I will investigate how
to make apr behave as documented on Win32, but probably Subversion will
need a workaround at least for a while.
Any thoughts on a workaround? Only this one series of tests seems to be
a problem because of the way it shares the pool...perhaps the test could
be modified temporarily?
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Win32 Tests -- locks-test
Posted by "D.J. Heap" <dj...@shadyvale.net>.
D.J. Heap wrote:
> I'm having trouble with Win32 tests on trunk -- locks-test.exe hangs
> with the following call stack:
[snip]
> Can someone else confirm/deny this result on Win32? Is apr_file_lock's
> 'exclusive lock' behavior semantically different on Unixes or is this
> test hanging on Linux also? I haven't deeply investigated yet, but will
> try to this weekend. It seems like it could be caused by a previous
> test function not cleaning everything, but that is just a guess.
>
> DJ
The problem seems to be (in fsfs repos only) that the subpool created by
fs_fs.c:svn_fs_fs__commit which holds the write lock is never cleared on
error conditions -- so the intentional failure on line 503 of
locks-test.c where we are 'nobody' orphans the write lock until the main
pool is cleared which doesn't happen until after all locks-test.c tests
are done.
Then the next locks-test.c test as 'hortense' hangs waiting for the
exclusive lock.
Probably just clearing the subpool on error conditions would fix this
specific problem, but why doesn't it fail on a Unix OS? Does exclusive
lock mean the same process/thread can re-lock on Unix? LockFileEx on
Windows is documented to not allow that.
It seems like allowing the same process to re-lock could be bad for
threaded programs like svnserve/mod_dav_svn, but maybe threads count as
different processes on Unix?
BDB obviously does not fail because it doesn't do locking this way, but
what does 'exclusive repo write lock' mean to it?
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org