win32 file locking

[John A Meinel]

Andrew Bennetts wrote:

>On Wed, May 25, 2005 at 01:43:43AM -0500, John A Meinel wrote:
>[...]
>
>
>>    def __del__(self):
>>        """Abort if the file has not already been closed."""
>>-        self.close()
>>+        if not self.closed:
>>+            raise RuntimeError('AtomicFile must have
>>commit()/abort()/close() called before it expires.')
>>
>>    def commit(self):
>>        """Close the file and move to final name."""
>>
>>This way AtomicFile will throw an exception if it is not used correctly.
>>
>>
>
>There's not much point raising an exception in a __del__ -- who's going to
>catch it?  There's no caller other than the garbage collector, which will
>just log a warning about it.  Just explicitly log a warning instead :)
>
>(See, another reason not to use __del__ <wink>)
>
>
>
Well, it depends when it would happen. I'm using __del__ for stuff like
Branch('.').revno(), Which dies right away.

>>The problem for Branch() is that I don't think anyone is going to go
>>around and call "unlock()" when they are done with it, meaning that all
>>branches are locked for the lifetime of bzrlib. Since bzrlib is meant to
>>be a library, potentially used by a GUI, this could be a long time.
>>
>>The problem is that Branch() is commonly used as a temporary object. For
>>instance the 'bzr revno' command is done like:
>>
>>print Branch('.').revno()
>>
>>How do you unlock a branch if you never have an object to call
>>'unlock()' on?
>>
>>
>
>It would be nice if this could just work.  I don't have an easy answer.  If
>you can be confident that Branch holds references to things that will never
>lead to a reference cycle, __del__ is ok, but I think that's unrealistic.
>
>
Okay, I guess I didn't realize that python handles objects with a
__del__ differently than objects without. I thought there was a default
__del__ and that you just override that.

Anyway, I'm okay with the BranchLocker/Unlocker setup. I think Aaron
Bentley's idea of having a wrapper object is okay, but it isn't quite as
clean.

If we do the wrapper, I would say it should look like this:

class BranchLockWrapper(object):
    def __init__(self, path, lockmode='r'):
       self.path = path
       self._lockmode = lockmode
       super(BranchLockWrapper, self).__init__()
       self._attach()
       self._stealfuncs()

    def _attach(self):
       self._lockedbranch = Branch(self.path, lockmode=self._lockmode)

    def _stealfuncs(self):
       for k in self._lockedbranch.__dict__:
          if k[:1] != '_' and k != 'unlock':
             def _localfunc(self, *args, **kwargs):
                if not self._lockedbranch:
                   self._attach()
                getattr(self._lockedbranch, k)(self._lockedbranch,
*args, **kwargs)
            setattr(self, k, _localfunc)

    def unlock(self):
        if self._lockedbranch:
            self._lockedbranch.unlock()
            self._lockedbranch = None

I think that properly proxies all of the functions on Branch, and allows
you to add new entries without worrying about them locking. In fact, if
you put this right after the "class Branch" declaration, you can do:

Branch = BranchLockWrapper

So that everytime someone asks for Branch, they actually get the wrapper.

You could also do this with appropriate subclassing & use of super().

>At a glance, it looks like the DebugInfo idea could be adapted to unlocking
>the Branch:
>
>
>
>
...

>The key is to avoid reference cycles with objects with __del__.  It's hard
>to prevent that with objects in a public API, like Branch.  It's possible
>with private objects, like BranchUnlocker.
>
>What do you think?
>
>
>
I misunderstood the problem with __del__. Your suggestion seems fine to me.

>-Andrew.
>
>
John
=:->

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 253 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20050525/b6a475a4/attachment.pgp