sftp locks can get stuck

[John A Meinel]

Robey Pointer wrote:
> 
> On 23 Dec 2005, at 5:59, John A Meinel wrote:
> 
>> Robey Pointer wrote:
>>>
>>> I think simpler is better here:  We record tracking info in the write
>>> lock (email, IP, timestamp) and provide a way to explicitly break the
>>> lock.  Maybe if an operation fails because of a write lock, and that
>>> lock is pretty old (by some arbitrary standard), bzr could hint to the
>>> user:
>>>
>>>     The branch was locked by <robey at lag.net> 5 hours ago.
>>>     If the lock is stale, you can break it with: bzr break-lock
>>>
>>> But no magic of trying to guess if the lock should be automatically
>>> broken, etc.  Stale locks should be relatively rare.
>>
>> I'm +1 for adding the username, and not automatically breaking the lock.
>>  I don't think the ip address helps much, but it is easy to record. I
>> don't think we need the timestamp, because we can just stat() the lock
>> file to figure out what time it was taken out. I suppose we could add
>> the UCT timestamp to help with handling time-zone shift.
> 
> Exactly, yeah.
> 
> So probably what we should do is move the locking outward (to Branch?) 
> and have Transport just provide something like
> 
>     create_lock_file(path, text_content)
> 
> and raise an exception if the lock file couldn't be created (via O_EXCL 
> or whatever the transport's equivalent is).
> 
> If that sounds okay I can probably cook up a patch.
> 
> robey
> 
> 

Well, transports already provide "lock_read()" and "lock_write()" which 
are supposed to 'lock' a specific file. (sftp implements this as 
creating a file with O_EXCL, local implements this as asking the OS to 
lock the file).

The question is how do we really want to do this. Do we want to give up 
the auto-unlock properties of OS locks, because of SFTP. Or do we just 
want SFTPTransport to add extra information to the lock.

It is nice to know that we'll never have dangling locks on local 
filesystems. Though we probably should at least check for a .write_lock 
file, so that we can warn if we think sftp is locking the branch.

If we are okay playing the least-common-denominator game, then I would 
say we change the Transport.lock_write() call, so that it asks for a 
text_content string, and then LocalTransport will act like SFTPTransport 
and create an O_EXCL file, and fill it with the content.

You could create the patch, I don't think it would be very hard. But at 
this point, we probably just need to discuss if it is worth it. Which a 
final decision should probably wait until Martin gets back from vacation 
in a week or two.

John
=:->