From 4cf9d933cad1f4e0b49db4ed7eb51667cf01ebc4 Mon Sep 17 00:00:00 2001 From: Francisco Ramirez Date: Fri, 21 Jan 2022 18:10:44 +0100 Subject: [PATCH] Add `ProfileAccessManager` to provide exclusive-access profile locks (#5270) With the new disk object store repository backend implementation, there is the need to perform maintenance operations on the repository from time to time. Some of these operations do not allow other processes to read from or write to the repository concurrently. Therefore it is necessary for the maintenance functionality to obtain an exlusive-access lock on the profile's backend. To this end the class `aiida.manage.profile_access.ProfileAccessManager` is added. From now on, when the storage backend of a profile is loaded it should call the `request_access` method of the manager. This will cause a PID file to be written to the `ACCESS_CONTROL_DIR` directory of the given profile. Since this access is non-exclusive, multiple processes can request access like this concurrently and a PID file will be written for each, where the command of the process is written to the file. This information is used in order to be able to determine which PID files may have gone stale. When the maintenance operation requires exclusive-access, it should use the `lock` context manager. If the profile is currently being accessed or already locked by another process, the lock request will fail. Otherwise, a lock file is created and this will guarantee that other processes won't be given a lock or be given normal access. The choice was made to use a custom implementation for the PID and lock files instead of using existing libraries such as `filelock`. The reason for this decision is because the use case is quite specific, where not only do we have exclusive as well as non-exclusive access, we also decided that we want to keep a record of the IDs of the processes that get access, such that when the request for a lock or access is denied, the error message can provide those PIDs. This will make it easier for the user to debug which (potentially dead) process is blocking the profile. The downside of this approach is of course that a custom implementation is more prone to bugs and cross-platform incompatibility compared to using a well tested library. Co-authored-by: Sebastiaan Huber --- aiida/backends/managers/__init__.py | 9 + aiida/common/__init__.py | 2 + aiida/common/exceptions.py | 15 +- aiida/manage/configuration/settings.py | 14 +- aiida/manage/manager.py | 3 + aiida/manage/profile_access.py | 222 +++++++++++++++ tests/backends/managers/__init__.py | 9 + tests/manage/test_profile_access.py | 363 +++++++++++++++++++++++++ tests/restapi/conftest.py | 3 +- 9 files changed, 637 insertions(+), 3 deletions(-) create mode 100644 aiida/backends/managers/__init__.py create mode 100644 aiida/manage/profile_access.py create mode 100644 tests/backends/managers/__init__.py create mode 100644 tests/manage/test_profile_access.py diff --git a/aiida/backends/managers/__init__.py b/aiida/backends/managers/__init__.py new file mode 100644 index 0000000000..2776a55f97 --- /dev/null +++ b/aiida/backends/managers/__init__.py @@ -0,0 +1,9 @@ +# -*- coding: utf-8 -*- +########################################################################### +# Copyright (c), The AiiDA team. All rights reserved. # +# This file is part of the AiiDA code. # +# # +# The code is hosted on GitHub at https://github.com/aiidateam/aiida-core # +# For further information on the license, see the LICENSE.txt file # +# For further information please visit http://www.aiida.net # +########################################################################### diff --git a/aiida/common/__init__.py b/aiida/common/__init__.py index 5a4963a697..fcb30cb980 100644 --- a/aiida/common/__init__.py +++ b/aiida/common/__init__.py @@ -57,6 +57,8 @@ 'LicensingException', 'LinkType', 'LoadingEntryPointError', + 'LockedProfileError', + 'LockingProfileError', 'MissingConfigurationError', 'MissingEntryPointError', 'ModificationNotAllowed', diff --git a/aiida/common/exceptions.py b/aiida/common/exceptions.py index fbae709d02..902feed9f2 100644 --- a/aiida/common/exceptions.py +++ b/aiida/common/exceptions.py @@ -17,7 +17,8 @@ 'PluginInternalError', 'ValidationError', 'ConfigurationError', 'ProfileConfigurationError', 'MissingConfigurationError', 'ConfigurationVersionError', 'IncompatibleDatabaseSchema', 'DbContentError', 'InputValidationError', 'FeatureNotAvailable', 'FeatureDisabled', 'LicensingException', 'TestsNotAllowedError', - 'UnsupportedSpeciesError', 'TransportTaskException', 'OutputParsingError', 'HashingError', 'DatabaseMigrationError' + 'UnsupportedSpeciesError', 'TransportTaskException', 'OutputParsingError', 'HashingError', 'DatabaseMigrationError', + 'LockedProfileError', 'LockingProfileError' ) @@ -260,3 +261,15 @@ class HashingError(AiidaException): """ Raised when an attempt to hash an object fails via a known failure mode """ + + +class LockedProfileError(AiidaException): + """ + Raised if attempting to access a locked profile + """ + + +class LockingProfileError(AiidaException): + """ + Raised if the profile can`t be locked + """ diff --git a/aiida/manage/configuration/settings.py b/aiida/manage/configuration/settings.py index 2408bdacbf..7595d46552 100644 --- a/aiida/manage/configuration/settings.py +++ b/aiida/manage/configuration/settings.py @@ -24,10 +24,12 @@ DEFAULT_CONFIG_INDENT_SIZE = 4 DEFAULT_DAEMON_DIR_NAME = 'daemon' DEFAULT_DAEMON_LOG_DIR_NAME = 'log' +DEFAULT_ACCESS_CONTROL_DIR_NAME = 'access' AIIDA_CONFIG_FOLDER: typing.Optional[pathlib.Path] = None DAEMON_DIR: typing.Optional[pathlib.Path] = None DAEMON_LOG_DIR: typing.Optional[pathlib.Path] = None +ACCESS_CONTROL_DIR: typing.Optional[pathlib.Path] = None def create_instance_directories(): @@ -41,11 +43,19 @@ def create_instance_directories(): directory_base = pathlib.Path(AIIDA_CONFIG_FOLDER).expanduser() directory_daemon = directory_base / DAEMON_DIR directory_daemon_log = directory_base / DAEMON_LOG_DIR + directory_access = directory_base / ACCESS_CONTROL_DIR + + list_of_paths = [ + directory_base, + directory_daemon, + directory_daemon_log, + directory_access, + ] umask = os.umask(DEFAULT_UMASK) try: - for path in [directory_base, directory_daemon, directory_daemon_log]: + for path in list_of_paths: if path is directory_base and not path.exists(): warnings.warn(f'Creating AiiDA configuration folder `{path}`.') @@ -75,6 +85,7 @@ def set_configuration_directory(): global AIIDA_CONFIG_FOLDER global DAEMON_DIR global DAEMON_LOG_DIR + global ACCESS_CONTROL_DIR environment_variable = os.environ.get(DEFAULT_AIIDA_PATH_VARIABLE, None) @@ -100,6 +111,7 @@ def set_configuration_directory(): DAEMON_DIR = AIIDA_CONFIG_FOLDER / DEFAULT_DAEMON_DIR_NAME DAEMON_LOG_DIR = DAEMON_DIR / DEFAULT_DAEMON_LOG_DIR_NAME + ACCESS_CONTROL_DIR = AIIDA_CONFIG_FOLDER / DEFAULT_ACCESS_CONTROL_DIR_NAME create_instance_directories() diff --git a/aiida/manage/manager.py b/aiida/manage/manager.py index cc79e13729..b99ddaa8ea 100644 --- a/aiida/manage/manager.py +++ b/aiida/manage/manager.py @@ -114,6 +114,7 @@ def _load_backend(self, schema_check: bool = True, repository_check: bool = True from aiida.common import ConfigurationError, InvalidOperation from aiida.common.log import configure_logging from aiida.manage import configuration + from aiida.manage.profile_access import ProfileAccessManager profile = self.get_profile() @@ -129,6 +130,8 @@ def _load_backend(self, schema_check: bool = True, repository_check: bool = True # Do NOT reload the backend environment if already loaded, simply reload the backend instance after if configuration.BACKEND_UUID is None: + access_manager = ProfileAccessManager(profile) + access_manager.request_access() backend_manager.load_backend_environment(profile, validate_schema=schema_check) configuration.BACKEND_UUID = profile.uuid diff --git a/aiida/manage/profile_access.py b/aiida/manage/profile_access.py new file mode 100644 index 0000000000..2404c2b9f8 --- /dev/null +++ b/aiida/manage/profile_access.py @@ -0,0 +1,222 @@ +# -*- coding: utf-8 -*- +########################################################################### +# Copyright (c), The AiiDA team. All rights reserved. # +# This file is part of the AiiDA code. # +# # +# The code is hosted on GitHub at https://github.com/aiidateam/aiida-core # +# For further information on the license, see the LICENSE.txt file # +# For further information please visit http://www.aiida.net # +########################################################################### +"""Module for the ProfileAccessManager that tracks process access to the profile.""" +import contextlib +import os +from pathlib import Path +import typing + +import psutil + +from aiida.common.exceptions import LockedProfileError, LockingProfileError +from aiida.common.lang import type_check +from aiida.manage.configuration import Profile + + +class ProfileAccessManager: + """Class to manage access to a profile. + + Any process that wants to request access for a given profile, should first call: + + ProfileAccessManager(profile).request_access() + + If this returns normally, the profile can be used safely. It will raise if it is locked, in which case the profile + should not be used. If a process wants to request exclusive access to the profile, it should use ``lock``: + + with ProfileAccessManager(profile).lock(): + pass + + If the profile is already locked or is currently in use, an exception is raised. + + Access and locks of the profile will be recorded in a directory with files with a ``.pid`` and ``.lock`` extension, + respectively. In principle then, at any one time, there can either be a number of pid files, or just a single lock + file. If there is a mixture or there are more than one lock files, we are in an inconsistent state. + """ + + def __init__(self, profile: Profile): + """Class that manages access and locks to the given profile. + + :param profile: the profile whose access to manage. + """ + from aiida.manage.configuration.settings import ACCESS_CONTROL_DIR + + type_check(profile, Profile) + self.profile = profile + self.process = psutil.Process(os.getpid()) + self._dirpath_records = ACCESS_CONTROL_DIR / profile.name + self._dirpath_records.mkdir(exist_ok=True) + + def request_access(self) -> None: + """Request access to the profile. + + If the profile is locked, a ``LockedProfileError`` is raised. Otherwise a PID file is created for this process + and the function returns ``None``. The PID file contains the command of the process. + + :raises ~aiida.common.exceptions.LockedProfileError: if the profile is locked. + """ + error_message = ( + f'process {self.process.pid} cannot access profile `{self.profile.name}`' + f'because it is being locked.' + ) + self._raise_if_locked(error_message) + + filepath_pid = self._dirpath_records / f'{self.process.pid}.pid' + filepath_tmp = self._dirpath_records / f'{self.process.pid}.tmp' + + try: + # Write the content to a temporary file and then move it into place with an atomic operation. + # This prevents the situation where another process requests a lock while this file is being + # written: if that was to happen, when the locking process is checking for outdated records + # it will read the incomplete command, won't be able to correctly compare it with its running + # process, and will conclude the record is old and clean it up. + filepath_tmp.write_text(str(self.process.cmdline())) + os.rename(filepath_tmp, filepath_pid) + + # Check again in case a lock was created in the time between the first check and creating the + # access record file. + error_message = ( + f'profile `{self.profile.name}` was locked while process ' + f'{self.process.pid} was requesting access.' + ) + self._raise_if_locked(error_message) + + except Exception as exc: + filepath_tmp.unlink(missing_ok=True) + filepath_pid.unlink(missing_ok=True) + raise exc + + @contextlib.contextmanager + def lock(self): + """Request a lock on the profile for exclusive access. + + This context manager should be used if exclusive access to the profile is required. Access will be granted if + the profile is currently not in use, nor locked by another process. During the context, the profile will be + locked, which will be lifted automatically as soon as the context exits. + + :raises ~aiida.common.exceptions.LockingProfileError: if there are currently active processes using the profile. + :raises ~aiida.common.exceptions.LockedProfileError: if there currently already is a lock on the profile. + """ + error_message = ( + f'process {self.process.pid} cannot lock profile `{self.profile.name}` ' + f'because it is already locked.' + ) + self._raise_if_locked(error_message) + + self._clear_stale_pid_files() + + error_message = ( + f'process {self.process.pid} cannot lock profile `{self.profile.name}` ' + f'because it is being accessed.' + ) + self._raise_if_active(error_message) + + filepath = self._dirpath_records / f'{self.process.pid}.lock' + filepath.touch() + + try: + # Check if no other lock files were created in the meantime, which is possible if another + # process was trying to obtain a lock at almost the same time. + # By re-checking after creating the lock file we can ensure that racing conditions will never + # cause two different processes to both think that they acquired the lock. It is still possible + # that two processes that are trying to lock will think that the other acquired the lock first + # and then both will fail, but this is a much safer case. + error_message = ( + f'while process {self.process.pid} attempted to lock profile `{self.profile.name}`, ' + f'other process blocked it first.' + ) + self._raise_if_locked(error_message) + + error_message = ( + f'while process {self.process.pid} attempted to lock profile `{self.profile.name}`, ' + f'other process started using it.' + ) + self._raise_if_active(error_message) + + yield + + finally: + filepath.unlink(missing_ok=True) + + def is_locked(self) -> bool: + """Return whether the profile is locked.""" + return self._get_tracking_files('.lock', exclude_self=False) != [] + + def is_active(self) -> bool: + """Return whether the profile is currently in use.""" + return self._get_tracking_files('.pid', exclude_self=False) != [] + + def clear_locks(self) -> None: + """Clear all locks on this profile. + + .. warning:: This should only be used if the profile is currently still incorrectly locked because the lock was + not automatically released after the ``lock`` contextmanager exited its scope. + """ + for lock_file in self._get_tracking_files('.lock'): + lock_file.unlink() + + def _clear_stale_pid_files(self) -> None: + """Clear any stale PID files.""" + for path in self._get_tracking_files('.pid'): + try: + process = psutil.Process(int(path.stem)) + except psutil.NoSuchProcess: + # The process no longer exists, so simply remove the PID file. + path.unlink() + else: + # If the process exists but its command is different from what is written in the PID file, + # we assume the latter is stale and remove it. + if path.read_text() != str(process.cmdline()): + path.unlink() + + def _get_tracking_files(self, ext_string: str, exclude_self: bool = False) -> typing.List[Path]: + """Return a list of all files that track the accessing and locking of the profile. + + :param ext_string: + To get the files that track locking use `.lock`, to get the files that track access use `.pid`. + + :param exclude_self: + If true removes from the returned list any tracking to the current process. + """ + path_iterator = self._dirpath_records.glob('*' + ext_string) + + if exclude_self: + filepath_self = self._dirpath_records / (str(self.process.pid) + ext_string) + list_of_files = [filepath for filepath in path_iterator if filepath != filepath_self] + + else: + list_of_files = list(path_iterator) + + return list_of_files + + def _raise_if_locked(self, message_start): + """Raise a ``LockedProfileError`` if the profile is locked. + + :param message_start: Text to use as the start of the exception message. + :raises ~aiida.common.exceptions.LockedProfileError: if the profile is locked. + """ + list_of_files = self._get_tracking_files('.lock', exclude_self=True) + + if len(list_of_files) > 0: + error_msg = message_start + '\nThe following processes are blocking the profile:\n' + error_msg += '\n'.join(f' - pid {path.stem}' for path in list_of_files) + raise LockedProfileError(error_msg) + + def _raise_if_active(self, message_start): + """Raise a ``LockingProfileError`` if the profile is being accessed. + + :param message_start: Text to use as the start of the exception message. + :raises ~aiida.common.exceptions.LockingProfileError: if the profile is active. + """ + list_of_files = self._get_tracking_files('.pid', exclude_self=True) + + if len(list_of_files) > 0: + error_msg = message_start + '\nThe following processes are accessing the profile:\n' + error_msg += '\n'.join(f' - pid {path.stem} (`{path.read_text()}`)' for path in list_of_files) + raise LockingProfileError(error_msg) diff --git a/tests/backends/managers/__init__.py b/tests/backends/managers/__init__.py new file mode 100644 index 0000000000..2776a55f97 --- /dev/null +++ b/tests/backends/managers/__init__.py @@ -0,0 +1,9 @@ +# -*- coding: utf-8 -*- +########################################################################### +# Copyright (c), The AiiDA team. All rights reserved. # +# This file is part of the AiiDA code. # +# # +# The code is hosted on GitHub at https://github.com/aiidateam/aiida-core # +# For further information on the license, see the LICENSE.txt file # +# For further information please visit http://www.aiida.net # +########################################################################### diff --git a/tests/manage/test_profile_access.py b/tests/manage/test_profile_access.py new file mode 100644 index 0000000000..e0606c2531 --- /dev/null +++ b/tests/manage/test_profile_access.py @@ -0,0 +1,363 @@ +# -*- coding: utf-8 -*- +########################################################################### +# Copyright (c), The AiiDA team. All rights reserved. # +# This file is part of the AiiDA code. # +# # +# The code is hosted on GitHub at https://github.com/aiidateam/aiida-core # +# For further information on the license, see the LICENSE.txt file # +# For further information please visit http://www.aiida.net # +########################################################################### +"""Tests the `request_ProfileAccessManageraccess` class.""" +# pylint: disable=protected-access + +from os import listdir +from pathlib import Path +from subprocess import PIPE, Popen + +import psutil +import pytest + +from aiida.common.exceptions import LockedProfileError, LockingProfileError +from aiida.manage.profile_access import ProfileAccessManager + +########################################################################### +# SIMPLE UNIT TESTS +# +# This first section contains simple unit tests to verify the inner +# workings of the different methods. They are separated from a more +# global set of tests for the whole system. +# +########################################################################### + + +@pytest.fixture(name='profile_access_manager') +def fixture_profile_access_manager(): + """Create special SQLAlchemy engine for use with QueryBuilder - backend-agnostic""" + from aiida.manage.manager import get_manager + aiida_profile = get_manager().get_profile() + return ProfileAccessManager(aiida_profile) + + +def test_get_tracking_files(profile_access_manager): + """Test the `_get_tracking_files` method for listing `.pid` and `.lock` files.""" + records_dir = profile_access_manager._dirpath_records + + # POPULATE WITH RECORDS + filepath_list = [ + records_dir / 'file_1.pid', + records_dir / 'file_2.pid', + records_dir / 'file_3.pid', + records_dir / 'file_4.lock', + records_dir / 'file_5.lock', + records_dir / 'file_6.etc', + ] + + for filepath in filepath_list: + filepath.touch() + + reference_set = {filename for filename in listdir(records_dir) if filename.endswith('.pid')} + resulting_set = {filepath.name for filepath in profile_access_manager._get_tracking_files('.pid')} + assert reference_set == resulting_set + + reference_set = {filename for filename in listdir(records_dir) if filename.endswith('.lock')} + resulting_set = {filepath.name for filepath in profile_access_manager._get_tracking_files('.lock')} + assert reference_set == resulting_set + + for filepath in filepath_list: + filepath.unlink() + + +def test_check_methods(profile_access_manager, monkeypatch): + """Test the `is_active` and `is_locked` methods. + + The underlying `_get_tracking_files` is tested elsewhere, so here it is mocked. For the + tested methods, we just need to check that they evaluate to the right value when files + are returned by `_get_tracking_files`, and when they are not. + """ + + def mockfun_return_path(*args, **kwargs): + """Mock of _raise_if_locked.""" + return [Path('file.txt')] + + monkeypatch.setattr(profile_access_manager, '_get_tracking_files', mockfun_return_path) + assert profile_access_manager.is_active() + assert profile_access_manager.is_locked() + + def mockfun_return_empty(*args, **kwargs): + """Mock of _raise_if_locked.""" + return [] + + monkeypatch.setattr(profile_access_manager, '_get_tracking_files', mockfun_return_empty) + assert not profile_access_manager.is_active() + assert not profile_access_manager.is_locked() + + +def test_raise_methods(profile_access_manager, monkeypatch): + """Test the `_raise_if_active` and `_raise_if_locked` methods. + + The underlying `_get_tracking_files` is tested elsewhere, so here it is mocked so that it + always provides a single result of a temporary file with arbitrary content. For the tested + methods, we just need to check that all the relevant information is passed and shown on the + error message. + """ + pass_message = 'This message should be on the error value' + file_content = 'This is the file content' + file_stem = '123456' + + tempfile = Path(file_stem + '.txt') + tempfile.write_text(file_content, encoding='utf-8') + + def mock_get_tracking_files(*args, **kwargs): + """Mock of _raise_if_locked.""" + return [tempfile] + + monkeypatch.setattr(profile_access_manager, '_get_tracking_files', mock_get_tracking_files) + + with pytest.raises(LockingProfileError) as exc: + profile_access_manager._raise_if_active(pass_message) + error_message = str(exc.value) + assert pass_message in error_message + assert tempfile.stem in error_message + assert file_content in error_message + + with pytest.raises(LockedProfileError) as exc: + profile_access_manager._raise_if_locked(pass_message) + error_message = str(exc.value) + assert pass_message in error_message + assert tempfile.stem in error_message + + tempfile.unlink() + + +def test_clear_locks(profile_access_manager): + """Tests the `test_clear_locks` method. + + For this it creates an artificial lock file and uses the `is_locked` method + (already tested elsewhere) to check if the profile is recognized as being + locked in the different steps. + """ + records_dir = profile_access_manager._dirpath_records + lockfile = records_dir / '1234567890.lock' + + assert not profile_access_manager.is_locked() + lockfile.touch() + assert profile_access_manager.is_locked() + profile_access_manager.clear_locks() + assert not profile_access_manager.is_locked() + + +def test_clear_stale_pid_files(profile_access_manager): + """Tests the `_clear_stale_pid_files` method.""" + records_dir = profile_access_manager._dirpath_records + fake_filename = '1234567890.pid' + fake_file = records_dir / fake_filename + fake_file.touch() + + assert fake_filename in listdir(records_dir) + profile_access_manager._clear_stale_pid_files() + assert fake_filename not in listdir(records_dir) + + +########################################################################### +# SEMI INTEGRATION TESTS +# +# This second section contains more complex tests that go through more +# than one method to test the cohesion of the whole class when working +# with actual processes. It is therefore also dependant on the call to +# the class in: +# +# > aiida.manage.manager::Manager._load_backend +# +# Moreover, they also require the use of a separate construct to keep +# track of processes accessing aiida profiles with ease (MockProcess). +# +# This is separated from the more simple unit tests for each method. +# +########################################################################### + + +class MockProcess(): + """Object that can start/stop an aiida process. + + After starting an aiida process, we need to check that the profile was loaded + successfully. We do so by having a line in the script that creates a check file. + We can then wait for this file to be created before returning control to the caller. + """ + + def __init__(self, profile): + self._process = None + self._profile = profile + + def _write_codefile(self, temppath_codefile, temppath_checkfile, runtime_secs=60): # pylint: disable=no-self-use + """Auxiliary function to write the code for the process.""" + # pylint: disable=f-string-without-interpolation + with temppath_codefile.open('w', encoding='utf-8') as fileobj: + fileobj.write(f'import time\n') + fileobj.write(f'from pathlib import Path\n') + fileobj.write(f'logger_file = Path("{temppath_checkfile.resolve()}")\n') + fileobj.write(f'logger_file.touch()\n') + fileobj.write(f'time.sleep({runtime_secs})\n') + + def _wait_for_checkfile(self, temppath_checkfile): # pylint: disable=no-self-use + """Auxiliary function that waits for the checkfile to be written.""" + import time + check_count = 0 + + while check_count < 100 and not temppath_checkfile.exists(): + time.sleep(0.1) + check_count += 1 + + if check_count == 100: + raise RuntimeError('Process loading was too slow!') + + def start(self, should_raise=False, runtime_secs=60): + """Starts a new process.""" + if self._process is not None: + self.stop() + + temppath_codefile = Path('temp_file.py') + temppath_checkfile = Path('temp_file.log') + + try: + self._write_codefile(temppath_codefile, temppath_checkfile, runtime_secs) + + self._process = Popen(['verdi', '-p', self._profile.name, 'run', 'temp_file.py'], stderr=PIPE) # pylint: disable=consider-using-with + + if should_raise: + error = self._process.communicate() + return_vals = (self._process.pid, error) + + else: + self._wait_for_checkfile(temppath_checkfile) + return_vals = (self._process.pid) + + finally: + temppath_codefile.unlink(missing_ok=True) + temppath_checkfile.unlink(missing_ok=True) + + return return_vals + + def stop(self): + """Kills the current process.""" + if self._process is not None: + self._process.stderr.close() + self._process.kill() + self._process.wait() + self._process = None + + +def test_access_control(profile_access_manager): + """Tests the request_access method indirectly. + + This test is performed in an integral way because the underlying methods used + have all been tested elsewhere, and it is more relevant to indirectly verify + that this method works in real life scenarios, rather than checking the specifics + of its internal logical structure. + """ + accessing_process = MockProcess(profile_access_manager.profile) + accessing_pid = accessing_process.start() + assert profile_access_manager.is_active() + accessing_process.stop() + + process_file = str(accessing_pid) + '.pid' + tracking_files = [filepath.name for filepath in profile_access_manager._get_tracking_files('.pid')] + assert process_file in tracking_files + + +@pytest.mark.parametrize('text_pattern', ['is being locked', 'was locked while']) +def test_request_access_errors(profile_access_manager, monkeypatch, text_pattern): + """Tests the `request_access` method errors when blocked. + + This test requires the use of the MockProcess class because part of what we are + checking is that the error shows the process ID that is trying to access the + profile (without relying on the test instance). The feature being tested is + intrinsically related to the live process instance that is stored as a property + of the `ProfileAccessManager` class. + """ + accessing_process = MockProcess(profile_access_manager.profile) + accessing_pid = accessing_process.start() + + def mock_raise_if_locked(error_message): + """Mock of _raise_if_locked.""" + if text_pattern in error_message: + raise LockedProfileError(error_message) + + monkeypatch.setattr(profile_access_manager, '_raise_if_locked', mock_raise_if_locked) + monkeypatch.setattr(profile_access_manager, 'process', psutil.Process(accessing_pid)) + + with pytest.raises(LockedProfileError) as exc: + profile_access_manager.request_access() + accessing_process.stop() + + assert profile_access_manager.profile.name in str(exc.value) + assert str(accessing_pid) in str(exc.value) + + +def test_lock(profile_access_manager, monkeypatch): + """Tests the locking mechanism. + + This test is performed in an integral way because the underlying methods used + have all been tested elsewhere, and it is more relevant to indirectly verify + that this method works in real life scenarios, rather than checking the specifics + of its internal logical structure. + """ + locking_proc = MockProcess(profile_access_manager.profile) + locking_pid = locking_proc.start() + monkeypatch.setattr(profile_access_manager, 'process', psutil.Process(locking_pid)) + + # It will not lock if there is a process accessing. + access_proc = MockProcess(profile_access_manager.profile) + access_pid = access_proc.start() + with pytest.raises(LockingProfileError) as exc: + with profile_access_manager.lock(): + pass + assert str(locking_pid) in str(exc.value) + assert str(access_pid) in str(exc.value) + access_proc.stop() + + # It will not let other process access if it locks. + # We need to mock the check on other accessing processes because otherwise + # it will detect the pytest process. + def mock_raise_if_1(error_message): # pylint: disable=unused-argument + """Mock function for the `raise_if` methods.""" + + monkeypatch.setattr(profile_access_manager, '_raise_if_active', mock_raise_if_1) + + with profile_access_manager.lock(): + assert profile_access_manager.is_locked() + access_proc = MockProcess(profile_access_manager.profile) + access_pid, error = access_proc.start(should_raise=True) + access_proc.stop() + assert str(locking_pid) in str(error[1]) + assert str(access_pid) in str(error[1]) + assert not profile_access_manager.is_locked() + + # This will raise after the lock file is created, allowing to test for + # racing conditions + records_dir = profile_access_manager._dirpath_records + + def mock_raise_if_2(message_start): + """Mock function for the `raise_if` methods.""" + for filename in listdir(records_dir): + if 'lock' in filename: + raise RuntimeError(message_start) + + monkeypatch.setattr(profile_access_manager, '_raise_if_locked', mock_raise_if_2) + + with pytest.raises(RuntimeError) as exc: + with profile_access_manager.lock(): + pass + assert str(locking_pid) in str(exc.value) + + # To test the second call to `_raise_if_active` we need to unset `_raise_if_locked` + # in case that is called first, but then we need to set again the internal PID + monkeypatch.undo() + monkeypatch.setattr(profile_access_manager, 'process', psutil.Process(locking_pid)) + monkeypatch.setattr(profile_access_manager, '_raise_if_active', mock_raise_if_2) + + with pytest.raises(RuntimeError) as exc: + with profile_access_manager.lock(): + pass + assert str(locking_pid) in str(exc.value) + + locking_proc.stop() diff --git a/tests/restapi/conftest.py b/tests/restapi/conftest.py index 2ee8628764..7bd6504072 100644 --- a/tests/restapi/conftest.py +++ b/tests/restapi/conftest.py @@ -54,7 +54,8 @@ def restrict_sqlalchemy_queuepool(aiida_profile): backend_manager = get_manager().get_backend_manager() backend_manager.reset_backend_environment() - backend_manager.load_backend_environment(aiida_profile, pool_timeout=1, max_overflow=0) + actual_profile = aiida_profile._manager._profile # pylint: disable=protected-access + backend_manager.load_backend_environment(actual_profile, pool_timeout=1, max_overflow=0) @pytest.fixture