From 409e851840d8311f67913f149da239942c4cb8fb Mon Sep 17 00:00:00 2001 From: Athanasius Date: Mon, 22 Mar 2021 14:21:29 +0000 Subject: [PATCH] Port in the journals_dir locking and other changes * Use a lock file in the journals_dir location to prevent more than one instance running against the same journals. We no longer check just for a Windows handle. So this is more correct on win32 *and* is now a thing on all other platforms. * Adds `--suppress-dupe-process-popup` CL arg to suppress "we're a dupe!" popup to aid those using batch files to launch EDMC alongside the game. * Two minor fixups of typos in PLUGINS.md. * Misc noqa comments and other flake8 fixups. We're now only missing type annotations in EDMarketConnector.py. # Conflicts: # EDMarketConnector.py --- EDMarketConnector.py | 275 ++++++++++++++----------------------------- 1 file changed, 91 insertions(+), 184 deletions(-) diff --git a/EDMarketConnector.py b/EDMarketConnector.py index 55ff13a5..29fe30d5 100755 --- a/EDMarketConnector.py +++ b/EDMarketConnector.py @@ -11,126 +11,17 @@ 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 -from typing import TYPE_CHECKING, Any, Mapping, Optional, Tuple, cast +from typing import TYPE_CHECKING, Any, Mapping, Optional, Tuple from constants import applongname, appname, protocolhandler_redirect # config will now cause an appname logger to be set up, so we need the # console redirect before this if __name__ == '__main__': - def no_other_instance_running() -> bool: # noqa: CCR001 - """ - Ensure only one copy of the app is running under this user account. - - OSX does this automatically. - - :returns: True if we are the single instance, else False. - """ - # TODO: Linux implementation - if platform == 'win32': - import ctypes - from ctypes.wintypes import BOOL, HWND, INT, LPARAM, LPCWSTR, LPWSTR - - EnumWindows = ctypes.windll.user32.EnumWindows # noqa: N806 - GetClassName = ctypes.windll.user32.GetClassNameW # noqa: N806 - GetClassName.argtypes = [HWND, LPWSTR, ctypes.c_int] - GetWindowText = ctypes.windll.user32.GetWindowTextW # noqa: N806 - GetWindowText.argtypes = [HWND, LPWSTR, ctypes.c_int] - GetWindowTextLength = ctypes.windll.user32.GetWindowTextLengthW # noqa: N806 - GetProcessHandleFromHwnd = ctypes.windll.oleacc.GetProcessHandleFromHwnd # noqa: N806 - - SW_RESTORE = 9 # noqa: N806 - SetForegroundWindow = ctypes.windll.user32.SetForegroundWindow # noqa: N806 - ShowWindow = ctypes.windll.user32.ShowWindow # noqa: N806 - ShowWindowAsync = ctypes.windll.user32.ShowWindowAsync # noqa: N806 - - COINIT_MULTITHREADED = 0 # noqa: N806,F841 - COINIT_APARTMENTTHREADED = 0x2 # noqa: N806 - COINIT_DISABLE_OLE1DDE = 0x4 # noqa: N806 - CoInitializeEx = ctypes.windll.ole32.CoInitializeEx # noqa: N806 - - ShellExecute = ctypes.windll.shell32.ShellExecuteW # noqa: N806 - ShellExecute.argtypes = [HWND, LPCWSTR, LPCWSTR, LPCWSTR, LPCWSTR, INT] - - def window_title(h): - if h: - text_length = GetWindowTextLength(h) + 1 - buf = ctypes.create_unicode_buffer(text_length) - if GetWindowText(h, buf, text_length): - return buf.value - - return None - - @ctypes.WINFUNCTYPE(BOOL, HWND, LPARAM) - def enumwindowsproc(window_handle, l_param): - # class name limited to 256 - https://msdn.microsoft.com/en-us/library/windows/desktop/ms633576 - cls = ctypes.create_unicode_buffer(257) - if GetClassName(window_handle, cls, 257) \ - and cls.value == 'TkTopLevel' \ - and window_title(window_handle) == applongname \ - and GetProcessHandleFromHwnd(window_handle): - # If GetProcessHandleFromHwnd succeeds then the app is already running as this user - if len(sys.argv) > 1 and sys.argv[1].startswith(protocolhandler_redirect): - CoInitializeEx(0, COINIT_APARTMENTTHREADED | COINIT_DISABLE_OLE1DDE) - # Wait for it to be responsive to avoid ShellExecute recursing - ShowWindow(window_handle, SW_RESTORE) - ShellExecute(0, None, sys.argv[1], None, None, SW_RESTORE) - - else: - ShowWindowAsync(window_handle, SW_RESTORE) - SetForegroundWindow(window_handle) - - return False - - return True - - return EnumWindows(enumwindowsproc, 0) - - return True - - def already_running_popup(): - """Create the "already running" popup.""" - import tkinter as tk - from tkinter import ttk - - root = tk.Tk(className=appname.lower()) - - frame = tk.Frame(root) - frame.grid(row=1, column=0, sticky=tk.NSEW) - - label = tk.Label(frame) - label['text'] = 'An EDMarketConnector.exe process was already running, exiting.' - label.grid(row=1, column=0, sticky=tk.NSEW) - - button = ttk.Button(frame, text='OK', command=lambda: sys.exit(0)) - button.grid(row=2, column=0, sticky=tk.S) - - root.mainloop() - - if not no_other_instance_running(): - # There's a copy already running. We want to inform the user by - # **appending** to the log file, not truncating it. - if getattr(sys, 'frozen', False): - # By default py2exe tries to write log to dirname(sys.executable) which fails when installed - import tempfile - - # unbuffered not allowed for text in python3, so use `1 for line buffering - sys.stdout = sys.stderr = open(join(tempfile.gettempdir(), f'{appname}.log'), mode='a', buffering=1) - - # Logging isn't set up yet. - # We'll keep this print, but it will be over-written by any subsequent - # write by the already-running process. - print("An EDMarketConnector.exe process was already running, exiting.") - - # To be sure the user knows, we need a popup - already_running_popup() - # If the user closes the popup with the 'X', not the 'OK' button we'll - # reach here. - sys.exit(0) - # Keep this as the very first code run to be as sure as possible of no # output until after this redirect is done, if needed. if getattr(sys, 'frozen', False): @@ -141,16 +32,14 @@ if __name__ == '__main__': sys.stdout = sys.stderr = open(join(tempfile.gettempdir(), f'{appname}.log'), mode='wt', buffering=1) # TODO: Test: Make *sure* this redirect is working, else py2exe is going to cause an exit popup +# These need to be after the stdout/err redirect because they will cause +# logging to be set up. # isort: off -import killswitch # Will cause a logging import/startup so needs to be after the redirect +import killswitch from config import appversion, appversion_nobuild, config, copyright # isort: on - -# After the redirect in case config does logging setup -from config import appversion, appversion_nobuild, config, copyright from EDMCLogging import edmclogger, logger, logging -from journal_lock import JournalLock, JournalLockResult if __name__ == '__main__': # noqa: C901 # Command-line arguments @@ -162,26 +51,23 @@ if __name__ == '__main__': # noqa: C901 "such as EDSM, Inara.cz and EDDB." ) - parser.add_argument('--trace', - help='Set the Debug logging loglevel to TRACE', - action='store_true', - ) + parser.add_argument( + '--trace', + help='Set the Debug logging loglevel to TRACE', + action='store_true', + ) + + parser.add_argument( + '--reset-ui', + help='reset UI theme and transparency to defaults', + action='store_true' + ) parser.add_argument('--suppress-dupe-process-popup', help='Suppress the popup from when the application detects another instance already running', action='store_true' ) - parser.add_argument('--force-localserver-for-auth', - help='Force EDMC to use a localhost webserver for Frontier Auth callback', - action='store_true' - ) - - parser.add_argument('edmc', - help='Callback from Frontier Auth', - nargs='*' - ) - args = parser.parse_args() if args.trace: @@ -190,18 +76,32 @@ if __name__ == '__main__': # noqa: C901 else: edmclogger.set_channels_loglevel(logging.DEBUG) - if args.force_localserver_for_auth: - config.set_auth_force_localserver() + def no_other_instance_running() -> bool: # noqa: CCR001 + """ + Ensure only one copy of the app is running for the configured journal directory. - def handle_edmc_callback_or_foregrounding(): # noqa: CCR001 - """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 - # If *this* instance hasn't locked, then another already has and we - # now need to do the edmc:// checks for auth callback - if locked != JournalLockResult.LOCKED: + logger.trace(f'journal_dir_lockfile = {journal_dir_lockfile!r}') + + locked = False + 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 @@ -236,7 +136,7 @@ if __name__ == '__main__': # noqa: C901 return None @ctypes.WINFUNCTYPE(BOOL, HWND, LPARAM) - def enumwindowsproc(window_handle, l_param): + def enumwindowsproc(window_handle, l_param): # noqa: CCR001 """ Determine if any window for the Application exists. @@ -259,7 +159,6 @@ if __name__ == '__main__': # noqa: C901 if GetProcessHandleFromHwnd(window_handle): # If GetProcessHandleFromHwnd succeeds then the app is already running as this user if len(sys.argv) > 1 and sys.argv[1].startswith(protocolhandler_redirect): - logger.debug('Invoked with edmc:// protocol handler arg') CoInitializeEx(0, COINIT_APARTMENTTHREADED | COINIT_DISABLE_OLE1DDE) # Wait for it to be responsive to avoid ShellExecute recursing ShowWindow(window_handle, SW_RESTORE) @@ -281,7 +180,31 @@ if __name__ == '__main__': # noqa: C901 # Ref: EnumWindows(enumwindowsproc, 0) - return + return False # Another instance is running + + 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 that no other instances are running. + + 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.""" @@ -306,22 +229,30 @@ if __name__ == '__main__': # noqa: C901 root.mainloop() - journal_lock = JournalLock() - locked = journal_lock.obtain_lock() + journal_dir: str = config.get_str('journaldir') or config.default_journal_dir + # This must be at top level to guarantee the file handle doesn't go out + # 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') - handle_edmc_callback_or_foregrounding() + # 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 \"{journal_dir_lockfile_name}\" for \"w+\"" + f" Aborting duplicate process checks: {e!r}") - if locked == JournalLockResult.ALREADY_LOCKED: - # There's a copy already running. + else: + if not no_other_instance_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 - if not args.edmc: + # To be sure the user knows, we need a popup already_running_popup() - # If the user closes the popup with the 'X', not the 'OK' button we'll - # reach here. - sys.exit(0) + # If the user closes the popup with the 'X', not the 'OK' button we'll + # reach here. + sys.exit(0) if getattr(sys, 'frozen', False): # Now that we're sure we're the only instance running we can truncate the logfile @@ -353,7 +284,7 @@ import tkinter as tk import tkinter.filedialog import tkinter.font import tkinter.messagebox -from tkinter import Toplevel, ttk +from tkinter import ttk import commodity import companion @@ -602,7 +533,7 @@ class AppWindow(object): # update geometry if config.get_str('geometry'): - match = re.match(r'\+([\-\d]+)\+([\-\d]+)', config.get_str('geometry')) # noqa: W605 + match = re.match(r'\+([\-\d]+)\+([\-\d]+)', config.get_str('geometry')) if match: if platform == 'darwin': # http://core.tcl.tk/tk/tktview/c84f660833546b1b84e7 @@ -1140,7 +1071,11 @@ class AppWindow(object): def shipyard_url(self, shipname: str) -> str: """Despatch a ship URL to the configured handler.""" if not bool(config.get_int("use_alt_shipyard_open")): - return plug.invoke(config.get_str('shipyard_provider'), 'EDSY', 'shipyard_url', monitor.ship(), monitor.is_beta) + return plug.invoke(config.get_str('shipyard_provider'), + 'EDSY', + 'shipyard_url', + monitor.ship(), + monitor.is_beta) # Avoid file length limits if possible provider = config.get_str('shipyard_provider', default='EDSY') @@ -1489,36 +1424,7 @@ def show_killswitch_poppup(root=None): # Run the app -if __name__ == "__main__": - # Command-line arguments - parser = argparse.ArgumentParser( - prog=appname, - description="Utilises Elite Dangerous Journal files and the Frontier " - "Companion API (CAPI) service to gather data about a " - "player's state and actions to upload to third-party sites " - "such as EDSM, Inara.cz and EDDB." - ) - - parser.add_argument( - '--trace', - help='Set the Debug logging loglevel to TRACE', - action='store_true', - ) - - parser.add_argument( - '--reset-ui', - help='reset UI theme and transparency to defaults', - action='store_true' - ) - - args = parser.parse_args() - - if args.trace: - logger.setLevel(logging.TRACE) - edmclogger.set_channels_loglevel(logging.TRACE) - else: - edmclogger.set_channels_loglevel(logging.DEBUG) - +if __name__ == "__main__": # noqa: C901 logger.info(f'Startup v{appversion} : Running on Python v{sys.version}') logger.debug(f'''Platform: {sys.platform} {sys.platform == "win32" and sys.getwindowsversion()} argv[0]: {sys.argv[0]} @@ -1610,6 +1516,7 @@ sys.path: {sys.path}''' @property def test_prop(self): + """Test property.""" logger.debug("test log from property") return "Test property is testy"