diff --git a/.codespellrc b/.codespellrc index c4d28d1af..f70564eb6 100644 --- a/.codespellrc +++ b/.codespellrc @@ -1,6 +1,6 @@ [codespell] # Folders and files to skip -skip = .codespellrc,*.po,Makefile,*.desktop,.git,__pycache__,*.pyc,languages.py +skip = .codespellrc,*.po,Makefile,*.desktop,.git,__pycache__,*.pyc,languages.py,*.html,_build # Print N lines of surrounding context context = 1 # Check hidden files also (empty means True) diff --git a/CHANGES b/CHANGES index 58ff57ed3..0fa00838b 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,7 @@ Back In Time Version 1.4.4-dev (development of upcoming release) +* Fix: Global flock for multiple users (#1122, #1676) * Fix bug: "Backup folders" list does reflect the selected snapshot (#1585) (@rafaelhdr Rafael Hurpia da Rocha) * Breaking Change: GUI started with --debug does no longer add --debug to the crontab for scheduled profiles. Use the new "enable logging for debug messages" in the 'Schedule' section of the 'Manage profiles' GUI instead. diff --git a/common/applicationinstance.py b/common/applicationinstance.py index 268fda713..65aecec9c 100644 --- a/common/applicationinstance.py +++ b/common/applicationinstance.py @@ -1,23 +1,24 @@ -# Back In Time -# Copyright (C) 2008-2022 Oprea Dan, Bart de Koning, Richard Bailey, Germar Reitze +# Back In Time +# Copyright (C) 2008-2022 Oprea Dan, Bart de Koning, Richard Bailey, +# Germar Reitze # -# This program is free software; you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 2 of the License, or -# (at your option) any later version. +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. # -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. # -# You should have received a copy of the GNU General Public License along -# with this program; if not, write to the Free Software Foundation, Inc., -# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +# You should have received a copy of the GNU General Public License along +# with this program; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. """ This module holds the ApplicationInstance class, used to handle -the one application instance mechanism +the one application instance mechanism. """ import os @@ -30,6 +31,8 @@ # one app instance eg. if a restore is running and another # backup starts). # Rename it to eg. LockFileManager +# TODO2 When refactoring have a look at "common/flock.py" still implementing +# a contxt manager for that problem. class ApplicationInstance: """ Class used to handle one application instance mechanism. @@ -154,15 +157,20 @@ def flockExclusiv(self): (so that only the last creator wins). Dev notes: - --------- + ---------- buhtz (2023-09): Not sure but just log an ERROR without doing anything else is IMHO not enough. + aryoda (2023-12): It seems the purpose of this additional lock file using an exclusive lock is to block the other process to continue until this exclusive lock is released (= serialize execution). Therefore advisory locks are used via fcntl.flock (see: man 2 fcntl) + + buhtz (2024-05): + Have a look at the new :mod:`flock` module providing an flock context + manager. """ flock_file_URI = self.pidFile + '.flock' diff --git a/common/doc-dev/flock.rst b/common/doc-dev/flock.rst new file mode 100644 index 000000000..ae0f42354 --- /dev/null +++ b/common/doc-dev/flock.rst @@ -0,0 +1,7 @@ +flock module +============ + +.. automodule:: flock + :members: + :undoc-members: + :show-inheritance: diff --git a/common/doc-dev/modules.rst b/common/doc-dev/modules.rst index f83de3871..83f71f325 100644 --- a/common/doc-dev/modules.rst +++ b/common/doc-dev/modules.rst @@ -14,6 +14,7 @@ common diagnostics encfstools exceptions + flock guiapplicationinstance logger mount diff --git a/common/flock.py b/common/flock.py new file mode 100644 index 000000000..baa8f6de3 --- /dev/null +++ b/common/flock.py @@ -0,0 +1,74 @@ +# SPDX-FileCopyrightText: © 2024 Christian BUHTZ +# +# SPDX-License-Identifier: GPL-2.0 +# +# This file is part of the program "Back In time" which is released under GNU +# General Public License v2 (GPLv2). +# See file LICENSE or go to . +import os +import fcntl +from pathlib import Path +import logger + + +class _FlockContext: + """Context manager to manage file locks (flock). + + The flock file is stored in the folder `/run/lock` or if not present in + `/var/lock`. + + Usage example :: + + class MyFlock(_FlockContext): + def __init__(self): + super().__init__('my.lock') + + with MyFlock(): + do_fancy_things() + + """ + def __init__(self, filename: str, folder: Path = None): + if folder is None: + folder = Path(Path.cwd().root) / 'run' / 'lock' + + # out-dated default + if not folder.exists(): + folder = Path(Path.cwd().root) / 'var' / 'lock' + + self._file_path = folder / filename + """Path to used for flock""" + + def __enter__(self): + self._log('Set') + + # Open (and create if needed) the file + mode = 'r' if self._file_path.exists() else 'w' + self._flock_handle = self._file_path.open(mode) + + # blocks (waits) until an existing flock is released + fcntl.flock(self._flock_handle, fcntl.LOCK_EX) + + # If new created file set itspermissions to "rw-rw-rw". + # otherwise a foreign user is not able to use it. + if mode == 'w': + self._file_path.chmod(int('0o666', 8)) + + return self + + def __exit__(self, exc_type, exc_value, exc_tb): + self._log('Release') + fcntl.fcntl(self._flock_handle, fcntl.LOCK_UN) + self._flock_handle.close() + + def _log(self, prefix: str): + """Generate a log message including the current lock files path and the + process ID. + """ + logger.debug(f'{prefix} flock {self._file_path} by PID {os.getpid()}', + self) + + +class GlobalFlock(_FlockContext): + """Flock context manager used for global flock in Back In Time.""" + def __init__(self): + super().__init__('backintime.lock') diff --git a/common/snapshots.py b/common/snapshots.py index 704d86c95..a13c2d35a 100644 --- a/common/snapshots.py +++ b/common/snapshots.py @@ -29,7 +29,6 @@ import shutil import time import re -import fcntl from tempfile import TemporaryDirectory import config import configfile @@ -39,6 +38,7 @@ import mount import progress import snapshotlog +import flock from applicationinstance import ApplicationInstance from exceptions import MountException, LastSnapshotSymlink @@ -59,7 +59,6 @@ class Snapshots: cfg (config.Config): current config """ SNAPSHOT_VERSION = 3 - GLOBAL_FLOCK = '/tmp/backintime.lock' def __init__(self, cfg = None): self.config = cfg @@ -83,7 +82,6 @@ def __init__(self, cfg = None): r'(.*$)') #trash at the end self.lastBusyCheck = datetime.datetime(1, 1, 1) - self.flock = None self.restorePermissionFailed = False # TODO: make own class for takeSnapshotMessage @@ -250,7 +248,7 @@ def uid(self, name, callback = None, backup = None): callback(msg) else: self.restorePermissionFailed = True - msg = 'Failed to get UID for %s: %s' % (name, str(e)) + msg = 'Failed to get UID for %s: %s' %(name, str(e)) logger.error(msg, self) if callback: callback(msg) @@ -704,7 +702,7 @@ def remove(self, sid): # - Nested "if"s # - Fuzzy names of classes, attributes and methods # - unclear variable names (at least for the return values) - def backup(self, force = False): + def backup(self, force=False): """Wrapper for :py:func:`takeSnapshot` which will prepare and clean up things for the main :py:func:`takeSnapshot` method. @@ -783,183 +781,191 @@ def backup(self, force = False): # global flock to block backups from other profiles or users # (and run them serialized) - self.flockExclusive() - logger.info('Lock', self) + # self.flockExclusive() + with flock.GlobalFlock(): + logger.info('Lock', self) - now = datetime.datetime.today() + now = datetime.datetime.today() - # inhibit suspend/hibernate during snapshot is running - self.config.inhibitCookie \ - = tools.inhibitSuspend(toplevel_xid=self.config.xWindowId) + # inhibit suspend/hibernate during snapshot is running + self.config.inhibitCookie = tools.inhibitSuspend( + toplevel_xid=self.config.xWindowId) - # mount - try: - hash_id = mount.Mount(cfg=self.config).mount() - - except MountException as ex: - logger.error(str(ex), self) - instance.exitApplication() - logger.info('Unlock', self) - time.sleep(2) - - return True - - else: - self.config.setCurrentHashId(hash_id) - - include_folders = self.config.include() + # mount + try: + hash_id = mount.Mount(cfg=self.config).mount() - if not include_folders: - logger.info('Nothing to do', self) + except MountException as ex: + logger.error(str(ex), self) + instance.exitApplication() + logger.info('Unlock', self) + time.sleep(2) - elif not self.config.PLUGIN_MANAGER.processBegin(): - logger.info('A plugin prevented the backup', self) + return True - else: - # take snapshot process begin - self.setTakeSnapshotMessage(0, '...') - self.snapshotLog.new(now) - - profile_id = self.config.currentProfile() - profile_name = self.config.profileName() - - logger.info(f"Take a new snapshot. Profile: {profile_id} " - f"{profile_name}", self) - - if not self.config.canBackup(profile_id): - - if (self.config.PLUGIN_MANAGER.hasGuiPlugins - and self.config.notify()): - - message = ( - _('Can\'t find snapshots folder.\nIf it is ' - 'on a removable drive please plug it in.') - + '\n' - + gettext.ngettext('Waiting %s second.', - 'Waiting %s seconds.', - 30) % 30 - ) - - self.setTakeSnapshotMessage( - type_id=1, - message=message, - timeout=30) + else: + self.config.setCurrentHashId(hash_id) - counter = 0 - for counter in range(0, 30): - logger.debug( - "Cannot start snapshot yet: target directory " - "not accessible. Waiting 1s.") + include_folders = self.config.include() - time.sleep(1) + if not include_folders: + logger.info('Nothing to do', self) - if self.config.canBackup(): - break + elif not self.config.PLUGIN_MANAGER.processBegin(): + logger.info('A plugin prevented the backup', self) - if counter != 0: - logger.info( - f"Waited {counter} seconds for target " - "directory to be available", self) + else: + # take snapshot process begin + self.setTakeSnapshotMessage(0, '…') + self.snapshotLog.new(now) + + profile_id = self.config.currentProfile() + profile_name = self.config.profileName() + + logger.info(f"Take a new snapshot. Profile: {profile_id} " + f"{profile_name}", self) + + if not self.config.canBackup(profile_id): + + if (self.config.PLUGIN_MANAGER.hasGuiPlugins + and self.config.notify()): + + message = ( + _("Can't find snapshots folder.\n" + "If it is on a removable drive please " + "plug it in.") + + '\n' + + gettext.ngettext('Waiting %s second.', + 'Waiting %s seconds.', + 30) % 30 + ) + + self.setTakeSnapshotMessage( + type_id=1, + message=message, + timeout=30) + + counter = 0 + for counter in range(0, 30): + logger.debug( + 'Cannot start snapshot yet: target ' + 'directory not accessible. Waiting 1s.') + + time.sleep(1) + + if self.config.canBackup(): + break + + if counter != 0: + logger.info( + f'Waited {counter} seconds for target ' + 'directory to be available', self) + + if not self.config.canBackup(profile_id): + logger.warning( + "Can't find snapshots folder!", self) + # Can't find snapshots directory (is it on a + # removable drive ?) + self.config.PLUGIN_MANAGER.error(3) - if not self.config.canBackup(profile_id): - logger.warning("Can't find snapshots folder!", self) - # Can't find snapshots directory (is it on a removable - # drive ?) - self.config.PLUGIN_MANAGER.error(3) + else: + ret_error = False + sid = SID(now, self.config) - else: - ret_error = False - sid = SID(now, self.config) + if sid.exists(): + logger.warning(f'Snapshot path "{sid.path()}" ' + 'already exists', self) + # This snapshot already exists + self.config.PLUGIN_MANAGER.error(4, sid) - if sid.exists(): - logger.warning(f'Snapshot path "{sid.path()}" ' - 'already exists', self) - # This snapshot already exists - self.config.PLUGIN_MANAGER.error(4, sid) + else: - else: + try: + # TODO + # rename ret_val to new_snapshot_created + # and ret_error to has_error for clearer + # code + ret_val, ret_error = self.takeSnapshot( + sid, now, include_folders) - try: - # TODO - # rename ret_val to new_snapshot_created and - # ret_error to has_error for clearer code - ret_val, ret_error = self.takeSnapshot( - sid, now, include_folders) + except: # TODO too broad exception + new = NewSnapshot(self.config) - except: - new = NewSnapshot(self.config) + if new.exists(): + new.saveToContinue = False + new.failed = True - if new.exists(): - new.saveToContinue = False - new.failed = True + raise - raise + if not ret_val: + self.remove(sid) - if not ret_val: - self.remove(sid) + if ret_error: + logger.error( + 'Failed to take snapshot.', self) + msg = _('Failed to take snapshot ' + '{snapshot_id}.').format( + snapshot_id=sid.displayID) + self.setTakeSnapshotMessage(1, msg) + # Fixes #1491 + self.config.PLUGIN_MANAGER.error(5, msg) - if ret_error: - logger.error('Failed to take snapshot.', self) - msg = _('Failed to take snapshot ' - '{snapshot_id}.').format( - snapshot_id=sid.displayID) - self.setTakeSnapshotMessage(1, msg) - # Fixes #1491 - self.config.PLUGIN_MANAGER.error(5, msg) + time.sleep(2) - time.sleep(2) + else: + logger.warning("No new snapshot", self) - else: - logger.warning("No new snapshot", self) + else: # new snapshot taken... - else: # new snapshot taken... + if ret_error: + logger.error('New snapshot taken but ' + 'errors detected', + self) + # Fixes #1491 + self.config.PLUGIN_MANAGER.error( + 6, sid.displayID) - if ret_error: - logger.error( - 'New snapshot taken but errors detected', - self) - # Fixes #1491 - self.config.PLUGIN_MANAGER.error( - 6, sid.displayID) + # Why ignore errors now? + ret_error = False + # Probably because a new snapshot has been + # created (= changes transferred) and + # "continue on errors" is enabled - # Why ignore errors now? - ret_error = False - # Probably because a new snapshot has been created - # (= changes transferred) and "continue on errors" - # is enabled + if not ret_error: + self.freeSpace(now) + self.setTakeSnapshotMessage(0, _('Finalizing')) - if not ret_error: - self.freeSpace(now) - self.setTakeSnapshotMessage(0, _('Finalizing')) + time.sleep(2) + sleep = False - time.sleep(2) - sleep = False + if ret_val: + # new snapshot + self.config.PLUGIN_MANAGER.newSnapshot( + sid, sid.path()) - if ret_val: - # new snapshot - self.config.PLUGIN_MANAGER.newSnapshot( - sid, sid.path()) + # Take snapshot process end + self.config.PLUGIN_MANAGER.processEnd() - # Take snapshot process end - self.config.PLUGIN_MANAGER.processEnd() + if sleep: + time.sleep(2) + sleep = False - if sleep: - time.sleep(2) - sleep = False + if not ret_error: + self.clearTakeSnapshotMessage() - if not ret_error: - self.clearTakeSnapshotMessage() + # unmount + try: + mount.Mount(cfg=self.config) \ + .umount(self.config.current_hash_id) - # unmount - try: - mount.Mount(cfg = self.config).umount(self.config.current_hash_id) + except MountException as ex: + logger.error(str(ex), self) - except MountException as ex: - logger.error(str(ex), self) + instance.exitApplication() + # self.flockRelease() - instance.exitApplication() - self.flockRelease() - logger.info('Unlock', self) + logger.info('Unlock', self) + # --- END GlobalFlock context --- if sleep: # max 1 backup / second @@ -2202,34 +2208,6 @@ def createLastSnapshotSymlink(self, sid): logger.error('Failed to create symlink %s: %s' %(symlink, str(e)), self) return False - # TODO Rename to make the purpose obvious, eg: Serialize_[backup|execution]_via_exclusive_global_flock() - def flockExclusive(self): - """ - Block :py:func:`backup` from other profiles or users - and run them serialized - """ - if self.config.globalFlock(): - logger.debug('Set flock %s' %self.GLOBAL_FLOCK, self) - self.flock = open(self.GLOBAL_FLOCK, 'w') - fcntl.flock(self.flock, fcntl.LOCK_EX) # blocks (waits) until an existing flock is released - # make it rw by all if that's not already done. - perms = stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | \ - stat.S_IWGRP | stat.S_IROTH | stat.S_IWOTH - s = os.fstat(self.flock.fileno()) - if not s.st_mode & perms == perms: - logger.debug('Set flock permissions %s' %self.GLOBAL_FLOCK, self) - os.fchmod(self.flock.fileno(), perms) - - def flockRelease(self): - """ - Release lock so other snapshots can continue - """ - if self.flock: - logger.debug('Release flock %s' %self.GLOBAL_FLOCK, self) - fcntl.fcntl(self.flock, fcntl.LOCK_UN) - self.flock.close() - self.flock = None - def rsyncSuffix(self, includeFolders = None, excludeFolders = None): """ Create suffixes for rsync. @@ -3068,7 +3046,7 @@ def iterSnapshots(cfg, includeNewSnapshot = False): yield sid # REFACTOR! - # LastSnapshotSymlink is an exception instance and could be catched + # LastSnapshotSymlink is an exception instance and could be caught # explicit. But not sure about its purpose. except Exception as e: if not isinstance(e, LastSnapshotSymlink): diff --git a/common/test/test_backintime.py b/common/test/test_backintime.py index 4ec1bae84..7bc937e9b 100644 --- a/common/test/test_backintime.py +++ b/common/test/test_backintime.py @@ -53,7 +53,7 @@ def test_local_snapshot_is_successful(self): and observe the intended behavior. Heavy refactoring is needed. But because of the "level" of that tests it won't happen in the near future. Also maintenance costs of this tests are damn high because - every tiny modifcation of BIT gives a false fail of this test. + every tiny modification of BIT gives a false fail of this test. Development notes (by Buhtz, 2024-05): It is just dumb stdout parsing. I tend to remove this test because of diff --git a/common/test/test_snapshots.py b/common/test/test_snapshots.py index ff6f4bcf1..542f83429 100644 --- a/common/test/test_snapshots.py +++ b/common/test/test_snapshots.py @@ -204,33 +204,33 @@ def test_createLastSnapshotSymlink(self): self.assertIsLink(symlink) self.assertEqual(os.path.realpath(symlink), sid2.path()) - def flockSecondInstance(self): - cfgFile = os.path.abspath(os.path.join(__file__, os.pardir, 'config')) - cfg = config.Config(cfgFile) - sn = snapshots.Snapshots(cfg) - sn.GLOBAL_FLOCK = self.sn.GLOBAL_FLOCK - - cfg.setGlobalFlock(True) - sn.flockExclusive() - sn.flockRelease() - - def test_flockExclusive(self): - RWUGO = 33206 #-rw-rw-rw - self.cfg.setGlobalFlock(True) - thread = Thread(target = self.flockSecondInstance, args = ()) - self.sn.flockExclusive() - - self.assertExists(self.sn.GLOBAL_FLOCK) - mode = os.stat(self.sn.GLOBAL_FLOCK).st_mode - self.assertEqual(mode, RWUGO) - - thread.start() - thread.join(0.01) - self.assertTrue(thread.is_alive()) - - self.sn.flockRelease() - thread.join() - self.assertFalse(thread.is_alive()) + # def flockSecondInstance(self): + # cfgFile = os.path.abspath(os.path.join(__file__, os.pardir, 'config')) + # cfg = config.Config(cfgFile) + # sn = snapshots.Snapshots(cfg) + # sn.GLOBAL_FLOCK = self.sn.GLOBAL_FLOCK + + # cfg.setGlobalFlock(True) + # sn.flockExclusive() + # sn.flockRelease() + + # def test_flockExclusive(self): + # RWUGO = 33206 #-rw-rw-rw + # self.cfg.setGlobalFlock(True) + # thread = Thread(target = self.flockSecondInstance, args = ()) + # self.sn.flockExclusive() + + # self.assertExists(self.sn.GLOBAL_FLOCK) + # mode = os.stat(self.sn.GLOBAL_FLOCK).st_mode + # self.assertEqual(mode, RWUGO) + + # thread.start() + # thread.join(0.01) + # self.assertTrue(thread.is_alive()) + + # self.sn.flockRelease() + # thread.join() + # self.assertFalse(thread.is_alive()) def test_statFreeSpaceLocal(self): self.assertIsInstance(self.sn.statFreeSpaceLocal('/'), int) diff --git a/qt/settingsdialog.py b/qt/settingsdialog.py index cf43a1aa6..7205f5d40 100644 --- a/qt/settingsdialog.py +++ b/qt/settingsdialog.py @@ -2264,7 +2264,7 @@ def _formatExcludeItem(self, item): item.setIcon(0, self.icon.DEFAULT_EXCLUDE) else: - # Icon: user definied + # Icon: user defined item.setIcon(0, self.icon.EXCLUDE)