[RFC] making TreeTransform more resistant to unexpected failures / writing problems

[Kuno Meyer]

On 07.08.2007 06:21, Aaron Bentley wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Kuno Meyer wrote:
>>   https://bugs.launchpad.net/bzr/+bug/67699
> 
> I am infuriated by the way Windows conflates opening a file with locking
> it, and this is exacerbated by Visual Studio's habit of opening files at
> random.

Nothing to be infuriated. Visual Studio just holds locks on files that 
don't deserve version control (binaries and generated intermediate files 
with cached data), and versioning such files is clearly a mistake.

But it is certainly true, that *some* popular applications (like the MS 
Office products) don't close the file handle of their document after 
reading it in order to prevent concurrent editing.

It occasionally may happen that a user forgets to close such documents 
when running bzr (as this may happen in other situations as well where 
file locks are being hold during a long time). Bazaar doesn't cope well 
even with the static case of locks constantly being hold.

And bug 67699 talks of write-protection flags as well.


> 
>> One option to solve this behaviour is to check the assumed preconditions
>> before actually performing the queued operations.
> 
> That doesn't solve the behavior.  That creates a race condition, in
> which sometimes the behavior is solved, and sometimes it is not.  I
> expect that in the Visual Studio case, Visual Studio will open a new
> file before the transform completes.

Yes, that's true in the case of having applications seeking file locks 
actively.

> 
>> 1) The easiest way to determine the writability would be to effectively
>> trying to write into the file (or when necessary, temporarily creating it).
>>
>> - This would be quite easy to implement and
>> - would cover all necessary issues like writing permissions on
>> directories on unix and exclusive file access.
>> - The cost of this operation is not that high
> 
> This would double the cost of tree creation, which I consider an
> unacceptable cost.  We are trying to make tree creation faster, not slower.
> 
>> 2) Another possibility would be to use the platform-dependent locking
>> techniques (fcntl.flock / win32file.LockFileEx) to check writability,
>> but then we have the problem with the directory permission flags under
>> Unix.
> 
> I think holding open locks on all affected files during the transform is
> an unacceptable resource waste.
> 
> Failing to hold all the locks during the transform would lead to races.
> 
>> What is your opinion?
> 
> I think that you can't predict whether this failure mode will occur.
> The only thing you can hope to do is recover gracefully when it does
> occur, e.g. by rolling back the transform or retrying the operation.

I'm sorry to say this, but implementing a rollback is way too much work 
for the time I have available for bazaar, based on a) my knowledge of 
cross-platform development, b) my knowledge of the internals of Bazaar 
and c) the complexity of TreeTransform. The TreeTransform class is in my 
opinion a bit underdocumented as well.

---

So, I am considering to implement at least an improved treatment of 
write-protection flags:

1) Write-protected files shall not break TreeTransform.finalize() (where 
the limbo dir gets cleared)

2) Newly created or modified files shall not have the write protection 
flag set (enforce chmod u+w)

Would this be acceptable an acceptable behaviour?

Kuno