Will the code below safely acquires multiple FileLock context manager objects and also release all resources at any error in __enter__
?
When using multiple context manager resources, the resource acquisitions must be atomic (all succeed or none succeed). With any error, all the resources that have been already acquired need to be released. Read Pythonic way to compose context managers for objects owned by a class but it looks complicated, and would like to have simpler one. If there is better way, please suggest.
from contextlib import (
ExitStack
)
from typing import (
Sequence,
ContextManager,
Optional,
)
class MultiFileLocks:
def __init__(self, filepaths: Sequence[str]):
if filepaths is None or len(filepaths) == 0:
raise ValueError(f"invalid filepath:[{filepaths}].")
self._filepaths: Sequence[str] = [
str(pathlib.Path(_path).expanduser().resolve()) for _path in filepaths
]
self._stack: Optional[ExitStack] = None
self._locks: Optional[Sequence[FileLock]] = None
def __enter__(self) -> ContextManager:
with ExitStack() as temp_stack:
_locks = [
temp_stack.enter_context(FileLock(_path)) for _path in self._filepaths
]
self._stack = temp_stack.pop_all()
self._locks = _locks
return self
def __exit__(self, exc_type, exc_value, traceback):
self._stack.close()
self._stack = None
self._locks = None
This looks mostly fine, but you have one major issue to contend with. If the file locks are acquired in a blocking manner, and different MultiFileLock
s are created with the same files to lock, in different orders, you can end up with a variation on the Dining Philosophers problem.
The simplest solution to such an issue is to impose an ordering on your files, e.g. sorting them by canonical paths, so that all locks are acquired in a single partial ordering (there are flaws to this solution involving fairness, you can read more on the Dining Philosophers link for more advanced solutions).
As a more minor issue, unless you need access to the stack itself after __enter__
returns, you could simplify things by just binding the close
method alone, per the example on pop_all
to avoid visibly storing the stack itself (it still stores everything, this just limits complexity from your point of view).
Otherwise you're looking good.