"Hyrum K. Wright" <hyrum_wri...@mail.utexas.edu> writes:
> [ sorry it's taken a few days to followup with this ]
>
> On Nov 20, 2009, at 1:00 PM, Philip Martin wrote:
>> How do we fix it? I suppose we could simply make it not an error to
>> release locks from directories that don't exist. That would have
>> the drawback that some errors such as releasing the wrong lock
>> wouldn't get caught. We could attempt to second-guess which
>> directories get removed and so restrict which lock releases we allow
>> to fail. That doesn't sound very good either, one bit of code is
>> second-guessing another. Also what happens if something creates a
>> new directory with a name matching the removed one in the gap
>> between the directory removal and the attempted lock release? We
>> need to be very careful not to release a lock we didn't acquire.
>>
>> Another solution would be to track the directory removal and record
>> the "missing" lock until it gets released. We could have
>> svn_wc__db_temp_forget_directory leave an in-memory pdh structure
>> that exists until the lock is released. I think this is a better
>> solution, it also seems closer to the single DB per WC case where it
>> will be possible to hold onto the locks until released.
>>
>> Do you agree there is a problem? Do any of these solutions look
>> like the right thing to do?
>
> I do agree that it is a problem. The long-term (read:
> post-centralized-metadata) solution is to have a lock at any level
> imply an infinitely recursive lock for levels beneath that. Since we
> have not yet centralized metadata, wc_db doesn't understand this
> notion quite yet. svn_wc__acquire_write_lock() and
> svn_wc__release_write_lock() attempt to emulate it with (apparent)
> limited success.
With 1.6 a non-recursive write-lock on a directory and doesn't stop
some other process taking a write-lock on a directory underneath the
locked directory. Are we going to stop supporting that?
>
> A possible solution would be to make the svn_wc__db_* functions
> smarter in that when a lock for a given location is requested, they
> would then apply the lock to that location, and when queried about an
> lock, it would look up the tree to determine if any of the parents had
> a lock. With our SQL schema, that turns out to be pretty easy in the
> centralized environment; with the current paradigm, not so much.
>
> Does this make sense?
It doesn't really answer my question about what should happen when
attempting to release a lock in a db that no longer exists. As I
described in my earlier mail above, we either allow such errors
indiscriminately or we track the names of the directories that have
been removed.
The current locking appears to be broken, there are paths that modify
the metadata without verifying that a lock is held. I don't know how
widespread this is but the first case I looked at is reverting a
schedule add directory: the parent doesn't get locked and yet the
shedule add child gets removed via
revert_entry
svn_wc__internal_remove_from_revision_control
svn_wc__entry_remove
svn_wc__db_temp_op_remove_entry
--
Philip
