[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