[MERGE][#335180] LockableFiles.__del__ must die (was Re: Do you need to close a branch? (if so how))

Michael Hudson michael.hudson at canonical.com
Wed Mar 11 21:49:03 GMT 2009


John Arbash Meinel wrote:
> Michael Hudson wrote:
>> Robert Collins wrote:
>>> Is the list with the value needed? 
>>>
>>> That gives LF->lock_value and LF->warner
>>> and warner->lock_value
>>> and warner has the __del__
>>>
>>> Looks to me like putting the lock value in the warner should work too
>>> and would be a little less convoluted.
>> Yes, that would make more sense :)
> 
>> Try the attached instead.
> 
>> Cheers,
>> mwh
> 
> 
> I'm a bit concerned, as there are some bits of code that inspect
> "_lock_count" to check whether things are locked. We really should have
> a simple api for that anyway, but the member was private, so code that
> did that gets to handle fixing itself when we break it.
>
> Anyway, I would just recommend adding a comment in NEWS, to make it
> easier for people to realize why it broke, and how to fix the plugin.

My patch adds a _lock_count property to LockableFiles, so there should
be no breakage.

> BB:tweak
> 
> And if someone could else could merge this, as I'm offline for the next
> couple of days.

Attached is a bundle that changes the name back to _LockWarner and fixes
the comment Andrew complained about.  It doesn't add a NEWS entry.

I would really like to see this land in both bzr.dev and bzr.1.13
because, astonishingly, I think it is contributing to operational
problems for Launchpad!  When I push to Launchpad, the 'bzr-notify'
process opens the remote branch so it can pop up a message about the new
revision[1].  The bug under discussion means that this socket is never
closed, which means that there is a bzr serve process hanging around on
the server.  Multiply this by a few hundred users of Bazaar and
Launchpad, and these processes use a _lot_ of ram.  So this issue just
became more urgent in my eyes at least.

I haven't merged bzr.dev into my branch, so this bundle should apply
fine to the 1.13 branch.

Cheers,
mwh.

[1] This is kinda stupid, but ...
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: no-LockableFiles.__del__.bundle
Url: https://lists.ubuntu.com/archives/bazaar/attachments/20090312/f5249765/attachment.diff 


More information about the bazaar mailing list