diff --git a/plugins/FileMonitor/StashPluginHelper.py b/plugins/FileMonitor/StashPluginHelper.py index 90c8c1b9..3516b4fb 100644 --- a/plugins/FileMonitor/StashPluginHelper.py +++ b/plugins/FileMonitor/StashPluginHelper.py @@ -481,9 +481,12 @@ def executeProcess(self, args, ExecDetach=False): if self.IS_LINUX: args = ["nohup"] + args self.Trace(f"Executing detached process using Popen({args})") + # start_new_session puts the child in its own session, so it survives the + # parent task process exiting (needed on macOS; harmless on Linux too). + pid = subprocess.Popen(args, start_new_session=True).pid else: self.Trace(f"Executing process using normal Popen({args})") - pid = subprocess.Popen(args).pid # On detach, may need the following for MAC OS subprocess.Popen(args, shell=True, start_new_session=True) + pid = subprocess.Popen(args).pid self.Trace(f"pid={pid}") return pid diff --git a/plugins/FileMonitor/filemonitor.py b/plugins/FileMonitor/filemonitor.py index bbf8c790..030413e7 100644 --- a/plugins/FileMonitor/filemonitor.py +++ b/plugins/FileMonitor/filemonitor.py @@ -14,7 +14,7 @@ from StashPluginHelper import StashPluginHelper from StashPluginHelper import taskQueue from threading import Lock, Condition -from multiprocessing import shared_memory +from multiprocessing import shared_memory, resource_tracker from filemonitor_config import config from filemonitor_task_examples import task_examples from filemonitor_self_unit_test import self_unit_test @@ -84,7 +84,12 @@ signal = Condition(mutex) shouldUpdate = False -SHAREDMEMORY_NAME = "DavidMaisonaveAxter_FileMonitor" # Unique name for shared memory +SHAREDMEMORY_NAME = "DavidMaisonave_FileMonitor" # Unique name for shared memory +# Note: must stay <=30 chars - macOS's POSIX shm_open() rejects names (incl. leading "/") +# longer than PSHMNAMLEN (31), raising OSError "File name too long" on SharedMemory(create=True). +# "DavidMaisonaveAxter_FileMonitor" (31 chars) silently hit this on every macOS start attempt, +# and was reported as the generic "must already be running" message below instead of the +# real OSError, since the except clause doesn't log the caught exception. SHAREDMEMORY_SIZE = 4 RECURSIVE = stash.pluginSettings["recursiveDisabled"] == False SCAN_MODIFIED = stash.pluginConfig["scanModified"] @@ -602,8 +607,8 @@ def start_library_monitor(): try: # Create shared memory buffer which can be used as singleton logic or to get a signal to quit task from external script shm_a = shared_memory.SharedMemory(name=SHAREDMEMORY_NAME, create=True, size=SHAREDMEMORY_SIZE) - except: - stash.Error(f"Could not open shared memory map ({SHAREDMEMORY_NAME}). Change File Monitor must be running. Can not run multiple instance of Change File Monitor. Stop FileMonitor before trying to start it again.") + except Exception as e: + stash.Error(f"Could not open shared memory map ({SHAREDMEMORY_NAME}). Change File Monitor must be running. Can not run multiple instance of Change File Monitor. Stop FileMonitor before trying to start it again. Error: {e}") return type(shm_a.buf) shm_buffer = shm_a.buf @@ -897,9 +902,17 @@ def stop_library_monitor(): def start_library_monitor_service(): # First check if FileMonitor is already running try: + # Just attach-and-close to confirm the segment exists; do NOT unlink it here. + # unlink() would delete the already-running instance's lock as a side effect of + # merely checking for it, breaking that instance's own status checks and stop + # command even though we're refusing to start a duplicate, not actually stopping it. shm_a = shared_memory.SharedMemory(name=SHAREDMEMORY_NAME, create=False, size=SHAREDMEMORY_SIZE) + # Also unregister from *this* process's resource_tracker: just attaching + # (even read-only) registers the segment there, and when this short-lived + # process exits, its tracker treats an unregistered-but-tracked segment as + # "leaked" and unlinks it - destroying the real running instance's lock. + resource_tracker.unregister(shm_a._name, "shared_memory") shm_a.close() - shm_a.unlink() stash.Error("FileMonitor is already running. Need to stop FileMonitor before trying to start it again.") return except: @@ -991,9 +1004,18 @@ def manageTagggedScenes(clearTag=True): def getFileMonitorRunningStatus(): FileMonitorStatus = "NOT running" try: + # Just attach-and-close to confirm the segment exists; do NOT unlink it here. + # unlink() deletes the named shared memory object immediately, so a mere status + # check would permanently break the name for the actually-running service (and + # for subsequent status checks and stop_library_monitor), even though the running + # process keeps its own already-open mapping and is otherwise unaffected. shm_a = shared_memory.SharedMemory(name=SHAREDMEMORY_NAME, create=False, size=SHAREDMEMORY_SIZE) + # Also unregister from *this* process's resource_tracker: just attaching + # (even read-only) registers the segment there, and when this short-lived + # process exits, its tracker treats an unregistered-but-tracked segment as + # "leaked" and unlinks it - destroying the real running instance's lock. + resource_tracker.unregister(shm_a._name, "shared_memory") shm_a.close() - shm_a.unlink() FileMonitorStatus = "RUNNING" stash.Log("FileMonitor is running...") except: diff --git a/plugins/FileMonitor/filemonitor.yml b/plugins/FileMonitor/filemonitor.yml index ccdd1b51..43bffd21 100644 --- a/plugins/FileMonitor/filemonitor.yml +++ b/plugins/FileMonitor/filemonitor.yml @@ -1,6 +1,6 @@ name: FileMonitor description: Monitors the Stash library folders, and updates Stash if any changes occurs in the Stash library paths. -version: 1.0.7 +version: 1.0.8 url: https://discourse.stashapp.cc/t/filemonitor/1333 ui: javascript: diff --git a/plugins/FileMonitor/version_history/README.md b/plugins/FileMonitor/version_history/README.md index 2b619718..4c3c5237 100644 --- a/plugins/FileMonitor/version_history/README.md +++ b/plugins/FileMonitor/version_history/README.md @@ -26,3 +26,8 @@ ### 1.0.5 - Fixed bug associated with library cleaning + +### 1.0.8 + +- Fixed bug on macOS where "Start Library Monitor Service" always failed: the shared-memory name used as the service's running/singleton lock exceeded macOS's POSIX shm_open name length limit (OSError "File name too long"), which was being swallowed and misreported as "FileMonitor must already be running". Shortened the name and surfaced the real exception in the error message. +- Hardened detached process launches (used by the service start, and shared with DupFileManager/RenameFile) to put the child in its own session via `start_new_session=True`, so it isn't torn down when the launching task process exits.