Skip to content

Commit

Permalink
PsqlDosBackend: ensure sqla sessions are garbage-collected on `clos…
Browse files Browse the repository at this point in the history
…e` (#5728)

When calling `Manager.unload_profile`, the `close` method is called on
the `StorageBackend`. For the `PsqlDosBackend` this means cleaning up
all SqlAlchemy connections and sessions. This was implemented, however,
SqlAlchemy would often still be holding on to at least one session in
the `WeakrefValueDictionary` of `sqlalchemy.orm.sessions._sessions`. It
turns out however that the reference was already cleared in fact, as
calling the garbage collector explicitly cleans the session references.
  • Loading branch information
ltalirz authored Nov 1, 2022
1 parent 470aba4 commit 798ded5
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 0 deletions.
5 changes: 5 additions & 0 deletions aiida/storage/psql_dos/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
# pylint: disable=missing-function-docstring
from contextlib import contextmanager, nullcontext
import functools
import gc
import pathlib
from typing import TYPE_CHECKING, Iterator, List, Optional, Sequence, Set, Union

Expand Down Expand Up @@ -144,6 +145,10 @@ def close(self) -> None:
self._session_factory.close()
self._session_factory = None

# Without this, sqlalchemy keeps a weakref to a session
# in sqlalchemy.orm.session._sessions
gc.collect()

def _clear(self, recreate_user: bool = True) -> None:
from aiida.storage.psql_dos.models.settings import DbSetting
from aiida.storage.psql_dos.models.user import DbUser
Expand Down
21 changes: 21 additions & 0 deletions tests/storage/psql_dos/test_backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,3 +144,24 @@ def mock_get_info(self, detailed=False, **kwargs): # pylint: disable=unused-arg
assert 'extra_value' in repository_info_out
assert repository_info_out['value'] == 42
assert repository_info_out['extra_value'] == 0


@pytest.mark.usefixtures('aiida_profile_clean')
def test_unload_profile():
"""Test that unloading the profile closes all sqla sessions.
This is a regression test for #5506.
"""
from sqlalchemy.orm.session import _sessions # pylint: disable=import-outside-toplevel

# Just running the test suite itself should have opened at least one session
assert len(_sessions) != 0, str(_sessions)

manager = get_manager()
profile_name = manager.get_profile().name

try:
manager.unload_profile()
assert len(_sessions) == 0, str(_sessions)
finally:
manager.load_profile(profile_name)

0 comments on commit 798ded5

Please sign in to comment.