On Tue, Aug 24, 2010 at 10:06:16AM +0100, Guido Trotter wrote:
> On Mon, Aug 23, 2010 at 6:38 PM, Michael Hanselmann <[email protected]> wrote:
>
> Hi,
>
> LGTM, a couple of notes below:
>
> > + # Note: to avoid unintentional race conditions, no references to
> > + # modifiable objects should be returned unless they were created in
> > this
> > + # function.
> > + for fname in fields:
> > + if fname == "name":
> > + info.append(self.name)
>
> self.name is theoretically modifiable, though, right?
Yes, but we never do it, I think.
> > + elif fname == "deleted":
> > + info.append(self.__deleted)
>
> Should this be reported as the "mode", I wonder?
>
> > +class LockMonitor(object):
> > + _LOCK_ATTR = "_lock"
> > +
> > + def __init__(self):
> > + """Initializes this class.
> > +
> > + """
> > + self._lock = SharedLock("LockMonitor")
> > +
> > + # Tracked locks. Weak references are used to avoid issues with circular
> > + # references and deletion.
> > + self._locks = weakref.WeakKeyDictionary()
> > +
> > + �...@ssynchronized(_LOCK_ATTR)
> > + def RegisterLock(self, lock):
> > + """Registers a new lock.
> > +
> > + """
> > + logging.debug("Registering lock %s", lock.name)
> > + assert lock not in self._locks, "Duplicate lock registration"
> > + assert not compat.any(lock.name == i.name for i in
> > self._locks.keys()), \
> > + "Found duplicate lock name"
> > + self._locks[lock] = None
> > +
>
> Why do we keep a dict with locks as keys and "None" as values? Would a
> set do? We seem to never use the value!
read the comment above about weak references.
iustin
