[Tim] >> _p_oid = Attribute( >> """The object id. >> ... >> Non-None object ids must be non-empty strings.
[Florent] > Ok then I guess this comment in Connection doesn't apply anymore: > > # It sucks that we have to hold the lock to read _invalidated. > # Normally, _invalidated is written by calling dict.update, which > # will execute atomically by virtue of the GIL. But some storage > # might generate oids where hash or compare invokes Python code. In > # that case, the GIL can't save us. Possibly, but I don't know. The specific _reason_ it gives clearly doesn't apply, but I never take code comments as being wholly accurate, and especially not when they're claiming to be an exhaustive account of threading insecurities. For example, it's easy to imagine that some other reason(s) for holding the lock also exist(s), but that nobody noticed it simply because holding the lock has prevented all other possible related races from occurring. Mucking with locks is dangerous, and it's rarely clear what the true intent was. For example, exactly which subset of the four existing ._inv_lock release/acquire pairs do you think that comment is talking about? Did those four even all exist at the time the comment was written? Regardless, why do other accesses of ._invalidated in Connection.py not lock at all? Is there code in Zope2 or Zope3 that reaches into Connection and mucks with .inv_lock and/or ._invalidated too (the leading "I'm private" underscore typically doesn't inhibit Zope2 coders ;-))? It generally takes me hours of poking and thinking before I can answer all questions "like that" with any confidence, and the potential costs of introducing a new threading problem usually far outweigh the potential benefits of removing a critical section. All I could make time for here now is to add another blurb to that comment, like "Note: oids are always strings, so that reasoning may no longer apply; it's unclear. TODO: figure this out, and remove the unnecessary critical sections (if any)." Of course I wanted to nail the type of oids precisely so that "mystery code" like this _could_ be cleaned up. Sometimes progress is slow ;-) _______________________________________________ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org http://mail.zope.org/mailman/listinfo/zodb-dev