Skip to content

Commit

Permalink
Check for and cleanup dirty zope.sqlalchemy state at end of request
Browse files Browse the repository at this point in the history
`zope.sqlalchemy.register` adds a set of event listeners to a session
which trigger population of an internal `id(session) => state` map
whenever a session becomes dirty. This map is usually cleared when the
DB transaction is committed as a result of the transaction manager being
committed or aborted at the end of a web request by pyramid_tm.

If however `request.session` is accessed outside of pyramid_tm's tween
and DB changes are made, the event listeners will fire and references to
the session's ID will be left in the internal _SESSION_STATE map when
the request ends.

This commit adds a check for a non-empty map at the end of a request and
logs a warning, to help track down causes of DB writes that are causing
the issue, it then cleans up the dirty state.

Unfortunately there are no public APIs for deregistering a session [1], so
we clear zope.sqlalchemy's private internal map directly.

[1] zopefoundation/zope.sqlalchemy#9
  • Loading branch information
robertknight committed Jan 9, 2018
1 parent bcae58f commit 4408060
Showing 1 changed file with 13 additions and 0 deletions.
13 changes: 13 additions & 0 deletions h/db/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,19 @@ def close_the_sqlalchemy_session(request):
})
session.close()

# zope.sqlalchemy maintains an internal `id(session) => state` map for
# each active DB session which is registered with it.
#
# Ensure that this is empty at the end of the request as otherwise it
# can cause request <=> DB transaction integration to break in future
# requests if a future `Session` object reuses the same ID.
dm = zope.sqlalchemy.datamanager
if len(dm._SESSION_STATE) > 0:
log.warn('request ended with non-empty zope.sqlalchemy session', extra={
'state': dm._SESSION_STATE
})
dm._SESSION_STATE = {}

return session


Expand Down

0 comments on commit 4408060

Please sign in to comment.