Ben Collins-Sussman <sussman@collab.net> writes:
> On Jul 7, 2005, at 4:34 PM, Philip Martin wrote:
>>
>>
>> I don't suppose it matters a great deal if the error gets returned by
>> svn_fs_lock instead of svn_repos_fs_lock, but the fact that the race
>> exists means that the code is suspicious. It's a bit like libsvn_wc
>> doing stat() to check a file exists before open() rather than simply
>> handling the open() error.
>
> I had a similar discussion with kfogel today. I pointed out that in
> the case of a commit, the commit-transaction may have been built
> successfully with no just-in-time conflicts, and then 'pre-commit'
> runs... but there's still a race, right? 'pre-commit' runs, and yet
> there's still no promise that svn_fs_commit() will succeed. So this
> doesn't seem any more suspicious to me: a bit of just-in-time
> checking from libsvn_repos, doing the best it can.
I suppose it's similar, but it's not really the same. Yes, the
pre-commit *hook* can reject the commit, but that's not the same as
having the libsvn_repos pre-commit code reject the commit.
> Karl feels that if we don't add this change, we should probably
> rename 'pre-lock' to 'start-lock', that there's a subtle difference
> between those things. 'start-foo' means "you're making an attempt to
> do foo", whereas 'pre-foo' means "you've attempted foo, and it's just
> about to happen now".
>
> In any case, the motivation here was that on the users@ list,
> somebody was trying to write a hook that notified someone, "hey, this
> user stole your lock". It turns out that it's impossible to do right
> now. 'post-lock' doesn't know who the original lock-owner was -- the
> original lock is long gone. 'pre-lock' works,
No, it doesn't work. The pre-lock hook cannot ever report "this user
stole your lock" it can only report "this user is trying to steal your
lock", and r15290 doesn't change that.
> but it turns out the
> user was getting two emails: first, when the stealer ran 'svn lock'
> (and failed), and again when the stealer successfully retried with
> '--force'.
Since pre-lock can only report the attempt, not the outcome, two
emails is correct as it indicates two attempts.
Only post-lock can definitively claim a new lock. I suppose there
could be cooperation between pre-lock and post-lock to report the
stolen lock, but I think that would still have races (and would
probably need a post-lock-failed hook as well). As far as I can see
the only way to report stolen locks without races is to have
svn_fs_lock return any such lock, and have libsvn_repos pass that to
the post-lock hook.
Here's an example of a race that causes a pre-lock hook to fail to
report a stolen lock:
- Joe habitually runs 'svn lock --force' because he always wants his
locks to succeed.
- File 'foo' is unlocked.
- Joe decides to lock 'foo' and runs 'svn lock --force foo'.
- Sally decides to lock 'foo' and runs 'svn lock foo'.
- Joe's lock gets as far as running pre-lock but 'foo' is unlocked so
no "stolen" email is sent.
- Sally's lock wins the race to svn_fs_lock and locks 'foo'.
- Joe's lock gets to svn_fs_lock and the --force flag steals Sally's
lock.
The result: Sally gets a lock and then Joe steals it, but no "stolen"
email gets sent. Now that race is improbable, but in a version
control system it should be impossible not improbable.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Jul 8 03:52:27 2005