Merge pull request #882 from EDCD/enhancement/859-journal-lock-on-change

Update Journal lock if user changes directory

EDMarketConnector.py

@@ -10,7 +10,6 @@ import sys
 import webbrowser
 from builtins import object, str
 from os import chdir, environ
-from os import getpid as os_getpid
 from os.path import dirname, isdir, join
 from sys import platform
 from time import localtime, strftime, time
@@ -34,6 +33,7 @@ if __name__ == '__main__':
 # After the redirect in case config does logging setup
 from config import appversion, appversion_nobuild, config, copyright
 from EDMCLogging import edmclogger, logger, logging
+from monitor import JournalLock
 
 if __name__ == '__main__':  # noqa: C901
     # Command-line arguments
@@ -71,32 +71,17 @@ if __name__ == '__main__':  # noqa: C901
     if args.force_localserver_for_auth:
         config.set_auth_force_localserver()
 
-    def no_other_instance_running() -> bool:  # noqa: CCR001
+    def handle_edmc_callback_or_foregrounding():  # noqa: CCR001
         """
-        Ensure only one copy of the app is running for the configured journal directory.
+        Handle any edmc:// auth callback, else foreground existing window.
-
-        :returns: True if we are the single instance, else False.
         """
         logger.trace('Begin...')
 
         if platform == 'win32':
-            logger.trace('win32, using msvcrt')
-            # win32 doesn't have fcntl, so we have to use msvcrt
-            import msvcrt
 
-            logger.trace(f'journal_dir_lockfile = {journal_dir_lockfile!r}')
+            # If *this* instance hasn't locked, then another already has and we
-
+            # now need to do the edmc:// checks for auth callback
-            locked = False
+            if not locked:
-            try:
-                msvcrt.locking(journal_dir_lockfile.fileno(), msvcrt.LK_NBLCK, 4096)
-
-            except Exception as e:
-                logger.info(f"Exception: Couldn't lock journal directory \"{journal_dir}\""
-                            f", assuming another process running: {e!r}")
-                locked = True
-
-            if locked:
-                # Need to do the check for this being an edmc:// auth callback
                 import ctypes
                 from ctypes.wintypes import BOOL, HWND, INT, LPARAM, LPCWSTR, LPWSTR
 
@@ -175,31 +160,7 @@ if __name__ == '__main__':  # noqa: C901
                 # Ref: <https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-enumwindows>
                 EnumWindows(enumwindowsproc, 0)
 
-                return False  # Another instance is running
+        return
-
-        else:
-            logger.trace('NOT win32, using fcntl')
-            try:
-                import fcntl
-
-            except ImportError:
-                logger.warning("Not on win32 and we have no fcntl, can't use a file lock!"
-                               "Allowing multiple instances!")
-                return True  # Lie about there being no other instances
-
-            try:
-                fcntl.flock(journal_dir_lockfile, fcntl.LOCK_EX | fcntl.LOCK_NB)
-
-            except Exception as e:
-                logger.info(f"Exception: Couldn't lock journal directory \"{journal_dir}\","
-                            f"assuming another process running: {e!r}")
-                return False
-
-        journal_dir_lockfile.write(f"Path: {journal_dir}\nPID: {os_getpid()}\n")
-        journal_dir_lockfile.flush()
-
-        logger.trace('Done')
-        return True
 
     def already_running_popup():
         """Create the "already running" popup."""
@@ -224,30 +185,21 @@ if __name__ == '__main__':  # noqa: C901
 
         root.mainloop()
 
-    journal_dir: str = config.get('journaldir') or config.default_journal_dir
+    journal_lock = JournalLock()
-    # This must be at top level to guarantee the file handle doesn't go out
+    locked = journal_lock.obtain_lock()
-    # of scope and get cleaned up, removing the lock with it.
-    journal_dir_lockfile_name = join(journal_dir, 'edmc-journal-lock.txt')
-    try:
-        journal_dir_lockfile = open(journal_dir_lockfile_name, mode='w+', encoding='utf-8')
 
-    # Linux CIFS read-only mount throws: OSError(30, 'Read-only file system')
+    handle_edmc_callback_or_foregrounding()
-    # Linux no-write-perm directory throws: PermissionError(13, 'Permission denied')
-    except Exception as e:  # For remote FS this could be any of a wide range of exceptions
-        logger.warning(f"Couldn't open \"{journal_dir_lockfile_name}\" for \"w+\""
-                       f" Aborting duplicate process checks: {e!r}")
 
-    else:
+    if not locked:
-        if not no_other_instance_running():
+        # There's a copy already running.
-            # There's a copy already running.
 
-            logger.info("An EDMarketConnector.exe process was already running, exiting.")
+        logger.info("An EDMarketConnector.exe process was already running, exiting.")
 
-            # To be sure the user knows, we need a popup
+        # To be sure the user knows, we need a popup
-            already_running_popup()
+        already_running_popup()
-            # If the user closes the popup with the 'X', not the 'OK' button we'll
+        # If the user closes the popup with the 'X', not the 'OK' button we'll
-            # reach here.
+        # reach here.
-            sys.exit(0)
+        sys.exit(0)
 
     if getattr(sys, 'frozen', False):
         # Now that we're sure we're the only instance running we can truncate the logfile
@@ -596,6 +548,9 @@ class AppWindow(object):
         # (Re-)install hotkey monitoring
         hotkeymgr.register(self.w, config.getint('hotkey_code'), config.getint('hotkey_mods'))
 
+        # Update Journal lock if needs be.
+        journal_lock.update_lock(self.w)
+
         # (Re-)install log monitoring
         if not monitor.start(self.w):
             self.status['text'] = f'Error: Check {_("E:D journal file location")}'

L10n/en.template

@@ -226,9 +226,15 @@
 /* Hotkey/Shortcut settings prompt on Windows. [prefs.py] */
 "Hotkey" = "Hotkey";
 
+/* Changed journal update_lock failed [monitor.py] */
+"Ignore" = "Ignore";
+
 /* Section heading in settings. [inara.py] */
 "Inara credentials" = "Inara credentials";
 
+/* Changed journal update_lock failed [monitor.py] */
+"Journal directory already locked" = "Journal directory already locked";
+
 /* Hotkey/Shortcut settings prompt on OSX. [prefs.py] */
 "Keyboard shortcut" = "Keyboard shortcut";
 
@@ -409,6 +415,9 @@
 /* Help menu item. [EDMarketConnector.py] */
 "Release Notes" = "Release Notes";
 
+/* Changed journal update_lock failed [monitor.py] */
+"Retry" = "Retry";
+
 /* Multicrew role label in main window. [EDMarketConnector.py] */
 "Role" = "Role";
 
@@ -478,6 +487,9 @@
 /* Main window. [EDMarketConnector.py] */
 "System" = "System";
 
+/* Changed journal update_lock failed [monitor.py] */
+"The new Journal Directory location is already locked.{CR}You can either attempt to resolve this and then Retry, or choose to Ignore this." = "The new Journal Directory location is already locked.{CR}You can either attempt to resolve this and then Retry, or choose to Ignore this.";
+
 /* Appearance setting. [prefs.py] */
 "Theme" = "Theme";
 

monitor.py

@@ -2,12 +2,16 @@ from collections import defaultdict, OrderedDict
 import json
 import re
 import threading
+from os import getpid as os_getpid
 from os import listdir, SEEK_SET, SEEK_END
 from os.path import basename, expanduser, isdir, join
+import pathlib
 from sys import platform
+import tkinter as tk
+from tkinter import ttk
 from time import gmtime, localtime, sleep, strftime, strptime, time
 from calendar import timegm
-from typing import Any, List, MutableMapping, Optional, OrderedDict as OrderedDictT, Tuple, TYPE_CHECKING, Union
+from typing import Any, Callable, List, MutableMapping, Optional, OrderedDict as OrderedDictT, Tuple, TYPE_CHECKING, Union
 
 if TYPE_CHECKING:
     import tkinter
@@ -18,6 +22,10 @@ from EDMCLogging import get_main_logger
 
 logger = get_main_logger()
 
+if TYPE_CHECKING:
+    def _(x: str) -> str:
+        return x
+
 if platform == 'darwin':
     from AppKit import NSWorkspace
     from watchdog.observers import Observer
@@ -1021,3 +1029,195 @@ class EDLogs(FileSystemEventHandler):  # type: ignore # See below
 
 # singleton
 monitor = EDLogs()
+
+
+class JournalLock:
+    """Handle locking of journal directory."""
+
+    def __init__(self):
+        """Initialise where the journal directory and lock file are."""
+        self.journal_dir: str = config.get('journaldir') or config.default_journal_dir
+        self.journal_dir_path = pathlib.Path(self.journal_dir)
+        self.journal_dir_lockfile_name = None
+        self.journal_dir_lockfile = None
+
+    def obtain_lock(self) -> bool:
+        """
+        Attempt to obtain a lock on the journal directory.
+
+        :return: bool - True if we successfully obtained the lock
+        """
+
+        self.journal_dir_lockfile_name = self.journal_dir_path / 'edmc-journal-lock.txt'
+        logger.trace(f'journal_dir_lockfile_name = {self.journal_dir_lockfile_name!r}')
+        try:
+            self.journal_dir_lockfile = open(self.journal_dir_lockfile_name, mode='w+', encoding='utf-8')
+
+        # Linux CIFS read-only mount throws: OSError(30, 'Read-only file system')
+        # Linux no-write-perm directory throws: PermissionError(13, 'Permission denied')
+        except Exception as e:  # For remote FS this could be any of a wide range of exceptions
+            logger.warning(f"Couldn't open \"{self.journal_dir_lockfile_name}\" for \"w+\""
+                           f" Aborting duplicate process checks: {e!r}")
+
+        if platform == 'win32':
+            logger.trace('win32, using msvcrt')
+            # win32 doesn't have fcntl, so we have to use msvcrt
+            import msvcrt
+
+            try:
+                msvcrt.locking(self.journal_dir_lockfile.fileno(), msvcrt.LK_NBLCK, 4096)
+
+            except Exception as e:
+                logger.info(f"Exception: Couldn't lock journal directory \"{self.journal_dir}\""
+                            f", assuming another process running: {e!r}")
+                return False
+
+        else:
+            logger.trace('NOT win32, using fcntl')
+            try:
+                import fcntl
+
+            except ImportError:
+                logger.warning("Not on win32 and we have no fcntl, can't use a file lock!"
+                               "Allowing multiple instances!")
+                return True  # Lie about being locked
+
+            try:
+                fcntl.flock(self.journal_dir_lockfile, fcntl.LOCK_EX | fcntl.LOCK_NB)
+
+            except Exception as e:
+                logger.info(f"Exception: Couldn't lock journal directory \"{self.journal_dir}\", "
+                            f"assuming another process running: {e!r}")
+                return False
+
+        self.journal_dir_lockfile.write(f"Path: {self.journal_dir}\nPID: {os_getpid()}\n")
+        self.journal_dir_lockfile.flush()
+
+        logger.trace('Done')
+        return True
+
+    def release_lock(self) -> bool:
+        """
+        Release lock on journal directory.
+
+        :return: bool - Success of unlocking operation."""
+        unlocked = False
+        if platform == 'win32':
+            logger.trace('win32, using msvcrt')
+            # win32 doesn't have fcntl, so we have to use msvcrt
+            import msvcrt
+
+            try:
+                # Need to seek to the start first, as lock range is relative to
+                # current position
+                self.journal_dir_lockfile.seek(0)
+                msvcrt.locking(self.journal_dir_lockfile.fileno(), msvcrt.LK_UNLCK, 4096)
+
+            except Exception as e:
+                logger.info(f"Exception: Couldn't unlock journal directory \"{self.journal_dir}\": {e!r}")
+
+            else:
+                unlocked = True
+
+        else:
+            logger.trace('NOT win32, using fcntl')
+            try:
+                import fcntl
+
+            except ImportError:
+                logger.warning("Not on win32 and we have no fcntl, can't use a file lock!")
+                return True  # Lie about being unlocked
+
+            try:
+                fcntl.flock(self.journal_dir_lockfile, fcntl.LOCK_UN)
+
+            except Exception as e:
+                logger.info(f"Exception: Couldn't unlock journal directory \"{self.journal_dir}\": {e!r}")
+
+            else:
+                unlocked = True
+
+        # Close the file whether or not the unlocking succeeded.
+        self.journal_dir_lockfile.close()
+
+        self.journal_dir_lockfile_name = None
+        self.journal_dir_lockfile = None
+
+        return unlocked
+
+    class JournalAlreadyLocked(tk.Toplevel):
+        """Pop-up for when Journal directory already locked."""
+
+        def __init__(self, parent: tk.Tk, callback: Callable):
+            tk.Toplevel.__init__(self, parent)
+
+            self.parent = parent
+            self.callback = callback
+            self.title(_('Journal directory already locked'))
+
+            # remove decoration
+            if platform == 'win32':
+                self.attributes('-toolwindow', tk.TRUE)
+
+            elif platform == 'darwin':
+                # http://wiki.tcl.tk/13428
+                parent.call('tk::unsupported::MacWindowStyle', 'style', self, 'utility')
+
+            self.resizable(tk.FALSE, tk.FALSE)
+
+            frame = ttk.Frame(self)
+            frame.grid(sticky=tk.NSEW)
+
+            self.blurb = tk.Label(frame)
+            self.blurb['text'] = _("The new Journal Directory location is already locked.{CR}"
+                                   "You can either attempt to resolve this and then Retry, or choose to Ignore this.")
+            self.blurb.grid(row=1, column=0, columnspan=2, sticky=tk.NSEW)
+
+            self.retry_button = ttk.Button(frame, text=_('Retry'), command=self.retry)
+            self.retry_button.grid(row=2, column=0, sticky=tk.EW)
+
+            self.ignore_button = ttk.Button(frame, text=_('Ignore'), command=self.ignore)
+            self.ignore_button.grid(row=2, column=1, sticky=tk.EW)
+            self.protocol("WM_DELETE_WINDOW", self._destroy)
+
+        def retry(self):
+            logger.trace('User selected: Retry')
+            self.destroy()
+            self.callback(True, self.parent)
+
+        def ignore(self):
+            logger.trace('User selected: Ignore')
+            self.destroy()
+            self.callback(False, self.parent)
+
+        def _destroy(self):
+            logger.trace('User force-closed popup, treating as Ignore')
+            self.ignore()
+
+    def update_lock(self, parent: tk.Tk):
+        """Update journal directory lock to new location if possible."""
+        current_journaldir = config.get('journaldir') or config.default_journal_dir
+
+        if current_journaldir == self.journal_dir:
+            return  # Still the same
+
+        self.release_lock()
+
+        self.journal_dir = current_journaldir
+        self.journal_dir_path = pathlib.Path(self.journal_dir)
+        if not self.obtain_lock():
+            # Pop-up message asking for Retry or Ignore
+            self.retry_popup = self.JournalAlreadyLocked(parent, self.retry_lock)
+
+    def retry_lock(self, retry: bool, parent: tk.Tk):
+        logger.trace(f'We should retry: {retry}')
+
+        if not retry:
+            return
+
+        current_journaldir = config.get('journaldir') or config.default_journal_dir
+        self.journal_dir = current_journaldir
+        self.journal_dir_path = pathlib.Path(self.journal_dir)
+        if not self.obtain_lock():
+            # Pop-up message asking for Retry or Ignore
+            self.retry_popup = self.JournalAlreadyLocked(parent, self.retry_lock)