[ZODB-Dev] Tracking down a freeze (deadlock?)
Dieter Maurer
dieter at handshake.de
Fri Feb 25 13:19:27 EST 2005
Tim Peters wrote at 2005-2-25 10:01 -0500:
> ...
>[Florent Guillaume]
>>> File "/opt/zope/lib/python/ZODB/Connection.py", line 257, in _setDB
>>> self._flush_invalidations()
>>> File "/opt/zope/lib/python/ZODB/Connection.py", line 552, in
>>> _flush_invalidations
>>> self._cache.invalidate(self._invalidated)
>>> File
>>>
>"/appli/zeo/zeocli-192.168.106.6-8080/Products/DICOD/DICODMailingList.py",
>line 125, in __del__
>>> File "/opt/zope/lib/python/ZODB/Connection.py", line 599, in setstate
>>> invalid = self._is_invalidated(obj)
>>> File "/opt/zope/lib/python/ZODB/Connection.py", line 617, in
>_is_invalidated
>>> self._inv_lock.acquire()
>>>
>>> Hm I think I can answer that one. A persistent object is not supposed to
>>> have a __del__ that accesses the ZODB right ? Otherwise, well, we see
>>> what happens.
>
>[Dieter Maurer]
>> On the other hand, it should not cause a deadlock.
>
>Why is that? As far as I'm concerned, ZODB doesn't support persistent
>objects with __del__ methods -- it was never intended to.
If it does not support objects with "__del__" method,
it should
1. clearly state so
2. raise an exception when it meats one
It is far too expensive to analyse a deadlock's cause
such that silently deadlocking is not "friendly"...
On the other hand, apparently not all "__del__" methods
seem to cause a deadlock (otherwise, the tests for
the infinite look would have revealed it).
Thus, maybe, Florent's analysis is not yet complete...
>2. Avoiding the deadlock is only the tip of the iceberg. Locks are
> meant to ensure invariants, and the latter are rarely documented
> in ZODB. To ensure that invariants aren't violated when switching
> to a reentrant lock requires trying to figure out what all the
> intended invariants actually are, then checking every possibly
> relevant code path to ensure that those invariants remain satisfied
> in reentrant cases.
In this special case, the lock's purpose is documented:
ensuring atomic processing of invalidations.
As such invalidations always come from a different thread,
a reentrant lock would not change anything for them.
>So, ya, it's a matter of seconds to change from a Lock to an RLock here, and
>a matter of perhaps 20 minutes to write and debug a good new test case(s),
>but every ZODB user would pay runtime expense for that forever after,
Yes, but we need to keep this in perspective:
This lock is usually acquired in connection with an
expensive operation (the loading of an object, the commit
of a transaction (maybe remote) and the opening a
connection). Compared to the "surrounding" operations,
the locking cost seem to be minor.
>and it
>may or may not introduce subtle new bugs (depending on the analysis in #2,
>and also on whether that analysis is wholly correct). If the benefit is to
>stop a deadlock in application code Florent is inclined to believe is
>incorrect anyway, it sounds like a losing set of tradeoffs to me.
If we really have the feeling that persistent objects
with "__del__" can cause deadlock, we should probably stamp
them out in a future ZODB version.
This would give a clear exception rather than a difficult to
analyse deadlock -- okay: at the cost of an additional test.
--
Dieter
More information about the ZODB-Dev
mailing list