[RFC] Implement bzrlib.lock with CreateFile rather than LockFileEx on win32

[Alexander Belchenko]

Martin (gzlist) пишет:
> Thanks for the feedback.

Thanks to your attempt on this.

> 2009/6/16 Martin Pool <mbp at sourcefrog.net>:
>> 2009/6/10 Martin (gzlist) <gzlist at googlemail.com>:
>>> Questions:
>>> # Would a change along these lines be acceptable?
>> I think these a change along these lines would be fine, assuming it
>> pases tests (or doesn't cause any new failures.)  We should also
>> manually check interoperation with old versions, to make sure they
>> correctly exclude each other.
> 
> Can test that just on the patch above. It basically works, the old
> two-step process gets Failed vs. Contention confused though, eg:
> 
>>>> from bzrlib.lock import _pywin32_CreateFile_WriteLock, _w32c_WriteLock
>>>> a = _pywin32_CreateFile_WriteLock("/tmp.lock")
>>>> b = _w32c_WriteLock("/tmp.lock")
> Traceback (most recent call last):
>   File "<stdin>", line 1, in ?
>   File "C:\bzr\bzr\lock_on_create\bzrlib\lock.py", line 426, in __init__
>     self._lock(filename, 'rb+', LOCK_EX + LOCK_NB)
>   File "C:\bzr\bzr\lock_on_create\bzrlib\lock.py", line 372, in _lock
>     self._open(filename, openmode)
>   File "C:\bzr\bzr\lock_on_create\bzrlib\lock.py", line 125, in _open
>     raise errors.LockFailed(self.filename, str(e))
> bzrlib.errors.LockFailed: Cannot lock C:/tmp.lock: [Errno 13]
> Permission denied: u'C:/tmp.lock'
>>>> a.unlock()
>>>> b = _w32c_WriteLock("/tmp.lock")
>>>> a = _pywin32_CreateFile_WriteLock("/tmp.lock")
> Traceback (most recent call last):
>   File "<stdin>", line 1, in ?
>   File "C:\bzr\bzr\lock_on_create\bzrlib\lock.py", line 342, in __init__
>     os.O_RDWR, "rb+")
>   File "C:\bzr\bzr\lock_on_create\bzrlib\lock.py", line 325, in _open
>     raise errors.LockContention(filename, e)
> bzrlib.errors.LockContention: Could not acquire lock "/tmp.lock": (32,
> 'CreateFile', 'The process cannot access the file because it is being
> used by another process.')
> 
>>> # Are there any expectations of the lock classes that aren't exercised in tests?
>> Possibly.  The main limitations seem to me to be:
>>
>> 1. We don't have very good code for testing locking across different
>> unrelated processes, different threads, etc -- and the behaviour of
>> the locks we use there varies, which is bad.  Eventually we want to
>> get away from using OS file locks.
> 
> I think you said something similar in Alexander's bug from 2007 that I
> linked. File locks don't seem to be going anywhere fast.
> 
> Have been bothered before by unittest not doing multiprocess well, do
> you know of any work that's been done to get around that?

May be I don't understand what Martin Pool means but for me it's enough to run command in the
subprocess to force reliable inter-processes behaviour re locks (even on Linux).

So I'd suggest this.

>>> # Should the existing win32 classes be left in place for the moment at least?
>> I don't think so.  Do you?  If it turns out to be a problem we can
>> revert the change or (in the short term) people can downgrade.
> 
> This seems fair, if new-over-old testing as per above is sufficient.
> 
>>> # Would altering the design to avoid all the code duplication be warrented?
>> Feel free too, but maybe as a separate patch.
> 
> Will look at as a later change as you suggest.
> 
>>> # Are locks only taken on byte string filenames, or is unicode used as well?
>> I'm not sure tbh.  It can probably be either.
> 
> Looking at it again, it seems _win32_realpath in bzrlib.osutils is
> effectively ensuring the filename is unicode (and that str input is an
> ascii, or it throws a UnicodeDecodeError).

Because tree could be placed in non-ascii directory I'd say it's most likely there is possible to
have unicode path to lock file.

>>> # How *are* you meant to do unicode exceptions in bazaar...
>> What do you mean?
> 
> Sorry, I explained in a bit more detail in the message with the patch
> preceeding this one. Basically, windows gives error messages in the
> user's language, which pywin32 and ctypes expose as byte strings, that
> may well not be in the same encoding as the terminal/whatever bzr
> wants to write to the log. I wanted to know the right way to encode
> the message to get it to display correctly.
> 
> Having looked at the bzrlib.errors code now though, it seems there is
> no right way. Even simple things will end up like:
> LockFailed(Unprintable exception LockFailed: dict={'msg': '', 'lock':
> u'somefile', 'why': 'O acesso \xe9 negado.'}, fmt='Cannot lock
> %(lock)s: %(why)s', error=<exceptions.UnicodeDecodeError instance at
> 0x00E33378>)
> Ensuring all params are unicode doesn't help much either, as the repr
> ends up as UTF-8 and thus mangled on the console. For
> non-ascii-superset languages, this means they're not going to be able
> to read the error messages at all.
> 
> As it's now obvious this isn't a problem with an easy-but-undocumented
> solution, I'll ignore it for the moment.

Well, you can try to decode ErrorMessage with osutils.user_encoding() because this is most likely
the encoding of system messages. And fallback to repr() instead. This is really hard to do right.