Add developer documentation for usage of lock, flock and other files …

…in BiT (related to #1573) (#1590)

* Document lock and flock file usage for developers (related to #1573)

Co-authored-by: aryoda <11374410+aryoda@users.noreply.github.com>
Co-authored-by: Christian Buhtz <c.buhtz@posteo.jp>

FAQ.md

@@ -320,7 +320,7 @@ schedules.
 
 ## Known Errors and Warnings
 ### WARNING: A backup is already running
-_Back In Time_ uses signal files like `worker.lock` to avoid starting the same backup twice.
+_Back In Time_ uses signal files like `worker<PID>.lock` to avoid starting the same backup twice.
 Normally it is deleted as soon as the backup finishes. In some case something went wrong
 so that _Back In Time_ was forcefully stopped without having the chance to delete
 this signal file.
@@ -336,12 +336,16 @@ ps aux | grep -i backintime
 If the output shows a running instance of _Back In Time_ it must be waited until it finishes
 or killed via `kill <process id>`.
 
+For more details see the developer documentation: [Usage of control files (locks, flocks, logs and others)](common/doc-dev/4_Control_files_usage_(locks_flocks_logs_and_others).md)
+
 ### The application is already running! (pid: 1234567)
 This message occurs when _Back In Time_ did not finish regularly and wasn't able
 to delete its application lock file. Before deleting that file make sure
 no backintime process is running via `ps aux | grep -i backintime`. Otherwise
 kill the process. After that look into the folder
-`~/.local/share/backintime` for the file `app.loc.pid` and delete it.
+`~/.local/share/backintime` for the file `app.lock.pid` and delete it.
+
+For more details see the developer documentation: [Usage of control files (locks, flocks, logs and others)](common/doc-dev/4_Control_files_usage_(locks_flocks_logs_and_others).md)
 
 ## Error Handling
 

README.md

@@ -71,7 +71,7 @@ installation options provided and maintained by third parties.
 
 - A Personal Package Archive ([PPA](https://launchpad.net/ubuntu/+ppas)) offering [`ppa:bit-team/stable`](https://launchpad.net/~bit-team/+archive/ubuntu/stable) as stable and [`ppa:bit-team/testing`](https://launchpad.net/~bit-team/+archive/ubuntu/testing) as testing PPA. Hosted at Launchpad and provided by [@Germar](https://github.com/germar).
 - A PPA distributing [_Back In Time_ for the latest stable Ubuntu release](https://git.sdxlive.com/PPA/about). See [PPA requirements](https://git.sdxlive.com/PPA/about/#requirements) and [install instructions](https://git.sdxlive.com/PPA/about/#installing). The PPA is self-hosted and provided by [@jean-christophe-manciot](https://github.com/jean-christophe-manciot).
-- The Arch User Repository ([AUR](https://aur.archlinux.org/)) do offer [some packages](https://aur.archlinux.org/packages?K=backintime).
+- The Arch User Repository ([AUR](https://aur.archlinux.org/)) does offer [some packages](https://aur.archlinux.org/packages?K=backintime).
 
 ## Known Problems and Workarounds
 

common/applicationinstance.py

@@ -26,6 +26,10 @@
 import tools
 
 
+# TODO This class name is a misnomer (there may be more than
+#      one app instance eg. if a restore is running and another
+#      backup starts).
+#      Rename it to eg. LockFileManager
 class ApplicationInstance:
     """
     Class used to handle one application instance mechanism.
@@ -54,12 +58,13 @@ def __init__(self, pidFile, autoExit=True, flock=False):
     def __del__(self):
         self.flockUnlock()
 
+    # TODO Rename to is_single_instance() to make the purpose more obvious
     def check(self, autoExit=False):
         """
         Check if the current application is already running
 
         Args:
-            autoExit (bool): automatically call sys.exit if there is an other
+            autoExit (bool): automatically call sys.exit if there is another
                              instance running
 
         Returns:
@@ -71,7 +76,7 @@ def check(self, autoExit=False):
 
         self.pid, self.procname = self.readPidFile()
 
-        # check if the process with specified by pid exists
+        # check if the process (PID) that created the pid file still exists
         if 0 == self.pid:
             return True
 
@@ -119,6 +124,9 @@ def startApplication(self):
                 'Failed to write PID file %s: [%s] %s'
                 % (e.filename, e.errno, e.strerror))
 
+        # The flock is removed here because it shall only ensure serialized access to the "pidFile" (lock file):
+        # Without setting flock in __init__ another process could also check for the existences of the "pidFile"
+        # in parallel and also create a new one (overwriting the one created here).
         self.flockUnlock()
 
     def exitApplication(self):
@@ -132,15 +140,52 @@ def exitApplication(self):
 
     def flockExclusiv(self):
         """
-        Create an exclusive lock to block a second instance while
-        the first instance is starting.
-
-        Dev note (buhtz: 2023-09)
+        Create an exclusive advisory file lock named <PID file>.flock
+        to block the creation of a second instance while the first instance
+        is still in the process of starting (but has not yet completely
+        started).
+
+        The purpose is to make
+        1. the check if the PID lock file already exists
+        2. and the subsequent creation of the PID lock file
+        an atomic operation by using a blocking "flock" file lock
+        on a second file to avoid that two or more processes check for
+        an existing PID lock file, find none and create a new one
+        (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).
+        Therefor advisory locks are used via fcntl.flock (see: man 2 fcntl)
         """
+
+        flock_file_URI = self.pidFile + '.flock'
+        logger.debug(f"Trying to put an advisory lock on the flock file {flock_file_URI}")
+
         try:
-            self.flock = open(self.pidFile + '.flock', 'w')
+            self.flock = open(flock_file_URI, 'w')
+            # This opens an advisory lock which which is considered only
+            # if other processes cooperate by explicitly acquiring locks
+            # (which BiT does IMHO).
+            # TODO Does this lock request really block if another processes
+            #      already holds a lock (until the lock is released)?
+            #      = Is the execution serialized?
+            # Provisional answer:
+            #      Yes, fcntl.flock seems to wait until the lock is removed
+            #      from the file.
+            #      Tested by starting one process in the console via
+            #         python3 applicationinstance.py
+            #      and then (while the first process is still running)
+            #      the same command in a 2nd terminal.
+            #      The ApplicationInstance constructor call needs
+            #      to be changed for this by adding "False, True"
+            #      to trigger an exclusive lock.
             fcntl.flock(self.flock, fcntl.LOCK_EX)
 
         except OSError as e:
@@ -153,6 +198,7 @@ def flockUnlock(self):
         but should find it self to be obsolete.
         """
         if self.flock:
+            logger.debug(f"Trying to remove the advisory lock from the flock file {self.flock.name}")
             fcntl.fcntl(self.flock, fcntl.LOCK_UN)
             self.flock.close()