zopefoundation / zope.interface

Interfaces for Python
http://zopeinterface.readthedocs.io/
Other
330 stars 71 forks source link

How to handle ConnectionStateErrors when looking up for adapters and closed connections #271

Open ale-rt opened 1 year ago

ale-rt commented 1 year ago

When we look up for adapters:

https://github.com/zopefoundation/zope.interface/blob/405017fef489ff9d517d0e86e6cb0e6a14fb9bd6/src/zope/interface/adapter.py#L818

We might have:

  1. a closed connection
  2. and a persistent adapter registry

See:

(Pdb++) self._registry.ro[0]
<zope.component.persistentregistry.PersistentAdapterRegistry object at 0x7fcbd097b3f0 oid 0x969d3f9388a51b2 in <Connection at 7fcbd22f1010>>
(Pdb++) self._registry.ro[0]._subscribers.__class__
<class 'persistent.list.PersistentList'>

This causes the _uncached_subscriptions to raise a ConnectionStateError.

The issue happens since zope.component 5, before the persistent adapter registry was storing a list, see:

What would be the best way to handle this issue? Adding a:

try:
    ...
except ConnectionStateError:
    continue

might be an idea?

Originally discovered while debugging:

https://github.com/plone/plone.testing/issues/83

What version of Python and Zope/Addons I am using:

Tested on Plone 6 with:

ale-rt commented 1 year ago

CCing @jamadden because he worked on https://github.com/zopefoundation/zope.component/pull/53

d-maurer commented 1 year ago

Alessandro Pisa wrote at 2023-6-8 03:04 -0700:

When we look up for adapters:

https://github.com/zopefoundation/zope.interface/blob/405017fef489ff9d517d0e86e6cb0e6a14fb9bd6/src/zope/interface/adapter.py#L818

We might have:

  1. a closed connection
  2. and a persistent adapter registry

When you access a persistent object with a closed connection, you do something fundamentally wrong: the object might be a ghost and then it cannot be loaded when the connection is not open.

You therefore must never store a persistent object at module or class level (e.g. in a cache). Any activity (e.g. thread) can access a persistent object only either directly via the connection (e.g. the ZODB root) or indirectly via object attributes of a persistent object it has itself accessed. In those cases, the connection is not closed.

ale-rt commented 1 year ago

I agree that this is not good. Probably that PersistentList does not even have what I am looking for :) But they point is that with the previous implementation (which used a list instead of a PersistentList) it was working.

d-maurer commented 1 year ago

Alessandro Pisa wrote at 2023-6-8 08:18 -0700:

I agree that this is not good. Probably that PersistentList does not even have what I am looking for :) But they point is that with the previous implementation (which used a list instead of a PersistentList) it was working.

Bad access to a persistent object is not detected reliably -- only when the object happens to be a ghost.

I am quite sure that the problem you now observe has been there already