Found while reviewing #1149 (which fixes a different session-sharing defect). This one is on main today and is reachable by any two concurrent web requests.
Symptom
A request fails partway through with one of:
InvalidRequestError: Instance '<Books at 0x...>' is not persistent within this Session
InvalidRequestError: Object <_Row> cannot be converted to 'persistent' state,
as this identity map is no longer valid. Has the owning Session been closed?
Intermittent, load-dependent, and the traceback points at whatever the request happened to be doing — so it looks like a different bug each time. Same family as the reports on #1048 and #1121.
Root cause
Two facts combine:
cps/server.py runs gevent.pywsgi.WSGIServer with a Pool, and there is no monkey.patch_all(). So every request greenlet runs on the same OS thread and therefore shares one calibre_db.session.shutdown_session (cps/__init__.py:481) runs calibre_db.session_factory.remove() on every request teardown. scoped_session.remove() calls Session.close(), which expunges every loaded object.
So when greenlet A finishes, its teardown closes the Session that greenlet B is still using. B's already-loaded ORM objects become detached, and its next attribute access, expire(), or streaming fetch raises.
Reproduced against main:
obj = db.session.query(T).first() # greenlet B, mid-request
print(obj.name) # -> "book"
CalibreDB.session_factory.remove() # greenlet A's Flask teardown fires
db.session.expire(obj)
# InvalidRequestError: Instance '<T at 0x...>' is not persistent within this Session
The busier the instance, the likelier the overlap — which matches reports that these errors show up on large libraries and during ingest, when requests take longer and overlap more.
Why it is not the same as #1121
#1121 is about a background WorkerThread (a real OS thread) sharing a Session with request handlers. This is about request greenlets sharing a Session with each other and tearing it down per-request. #1149 fixes the first and, as written, makes this one sharper — the property returns a brand-new Session after remove(), so calibre_db.session stops being stable within a single request. Details in the review on #1149.
Direction
Needs a real answer rather than a patch, and the two issues should be solved together since they both come down to "what is the lifetime of a Session here":
- Scope the session per greenlet (
scoped_session(scopefunc=gevent.getcurrent)) instead of per OS thread, so each request genuinely gets its own and per-request teardown is correct.
- Or stop calling
remove() per request and tie teardown to something that is actually request-scoped.
Option 1 looks closer to correct, but it interacts with the StaticPool constraint recorded in notes/fix-udf-gil-deadlock-DESIGN.md (one shared DBAPI connection, transaction boundaries process-global), so it needs designing rather than a one-liner.
No fix in this issue — filing so the finding isn't lost and so #1149's design pass covers both.
Found while reviewing #1149 (which fixes a different session-sharing defect). This one is on
maintoday and is reachable by any two concurrent web requests.Symptom
A request fails partway through with one of:
Intermittent, load-dependent, and the traceback points at whatever the request happened to be doing — so it looks like a different bug each time. Same family as the reports on #1048 and #1121.
Root cause
Two facts combine:
cps/server.pyrunsgevent.pywsgi.WSGIServerwith aPool, and there is nomonkey.patch_all(). So every request greenlet runs on the same OS thread and therefore shares onecalibre_db.session.shutdown_session(cps/__init__.py:481) runscalibre_db.session_factory.remove()on every request teardown.scoped_session.remove()callsSession.close(), which expunges every loaded object.So when greenlet A finishes, its teardown closes the Session that greenlet B is still using. B's already-loaded ORM objects become detached, and its next attribute access,
expire(), or streaming fetch raises.Reproduced against
main:The busier the instance, the likelier the overlap — which matches reports that these errors show up on large libraries and during ingest, when requests take longer and overlap more.
Why it is not the same as #1121
#1121 is about a background
WorkerThread(a real OS thread) sharing a Session with request handlers. This is about request greenlets sharing a Session with each other and tearing it down per-request. #1149 fixes the first and, as written, makes this one sharper — the property returns a brand-new Session afterremove(), socalibre_db.sessionstops being stable within a single request. Details in the review on #1149.Direction
Needs a real answer rather than a patch, and the two issues should be solved together since they both come down to "what is the lifetime of a Session here":
scoped_session(scopefunc=gevent.getcurrent)) instead of per OS thread, so each request genuinely gets its own and per-request teardown is correct.remove()per request and tie teardown to something that is actually request-scoped.Option 1 looks closer to correct, but it interacts with the
StaticPoolconstraint recorded innotes/fix-udf-gil-deadlock-DESIGN.md(one shared DBAPI connection, transaction boundaries process-global), so it needs designing rather than a one-liner.No fix in this issue — filing so the finding isn't lost and so #1149's design pass covers both.