fix: Stale library lock no longer blocking execution

steilerDev · Nov 4, 2023 · 8933563 · 8933563
1 parent 24a3cf9
commit 8933563
Show file tree

Hide file tree

Showing 6 changed files with 119 additions and 16 deletions.
diff --git a/app/src/app/factory.ts b/app/src/app/factory.ts
@@ -179,13 +179,13 @@ export function argParser(callback: (res: iCPSApp) => void): Command {
             .env(`SCHEDULE`)
             .default(`0 2 * * *`)
             .argParser(commanderParseCron))
-        .addOption(new Option(`--enable-crash-reporting`, `Enables automatic collection of errors and crashes, see https://icloud-photos-sync.steilerdev.de/user-guides/error-reporting/ for more information.`)
+        .addOption(new Option(`--enable-crash-reporting`, `Enables automatic collection of errors and crashes, see https://icps.steiler.dev/error-reporting/ for more information.`)
             .env(`ENABLE_CRASH_REPORTING`)
             .default(false))
         .addOption(new Option(`--fail-on-mfa`, `If a MFA is necessary, exit the program.`)
             .env(`FAIL_ON_MFA`)
             .default(false))
-        .addOption(new Option(`--force`, `Force the execution of the operation, independent of an existing lock on the library. USE WITH CAUTION!`)
+        .addOption(new Option(`--force`, `Forcefully remove an existing library lock. USE WITH CAUTION!`)
             .env(`FORCE`)
             .default(false))
         .addOption(new Option(`--refresh-token`, `Invalidate any stored trust token upon startup.`)

diff --git a/app/src/app/icloud-app.ts b/app/src/app/icloud-app.ts
@@ -134,15 +134,23 @@ abstract class iCloudApp extends iCPSApp {
             .catch(() => false);
 
         if (lockFileExists) {
-            if (!Resources.manager().force) {
-                const lockingProcess = (await fs.promises.readFile(lockFilePath, `utf-8`)).toString();
+            const lockingProcess = parseInt(await fs.promises.readFile(lockFilePath, `utf-8`), 10);
+
+            if (process.pid === lockingProcess) {
+                Resources.logger(this).warn(`Lock file exists, but is owned by this process. Continuing.`);
+                return;
+            }
+
+            if (Resources.pidIsRunning(lockingProcess) && !Resources.manager().force) {
                 throw new iCPSError(LIBRARY_ERR.LOCKED)
                     .addMessage(`Locked by PID ${lockingProcess}`);
             }
 
+            // Clear stale lock file
             await fs.promises.rm(lockFilePath, {force: true});
         }
 
+        // Create lock file
         await fs.promises.writeFile(lockFilePath, process.pid.toString(), {encoding: `utf-8`});
     }
 
@@ -157,11 +165,13 @@ abstract class iCloudApp extends iCPSApp {
             .catch(() => false);
 
         if (!lockFileExists) {
+            Resources.logger(this).warn(`Cannot release lock: Lock file does not exist.`);
             return;
         }
 
-        const lockingProcess = (await fs.promises.readFile(lockFilePath, `utf-8`)).toString();
-        if (lockingProcess !== process.pid.toString() && !Resources.manager().force) {
+        const lockingProcess = parseInt(await fs.promises.readFile(lockFilePath, `utf-8`), 10);
+
+        if (process.pid !== lockingProcess && Resources.pidIsRunning(lockingProcess) && !Resources.manager().force) {
             throw new iCPSError(LIBRARY_ERR.LOCKED)
                 .addMessage(`Locked by PID ${lockingProcess}`);
         }

diff --git a/app/src/lib/resources/main.ts b/app/src/lib/resources/main.ts
@@ -174,6 +174,20 @@ export namespace Resources {
         return Resources.event().emit(event, ...args);
     }
 
+    /**
+     * Can be used to check if a process is running
+     * @param pid - The process id to check
+     * @returns True if the process is running or the user does not have the necessary permissions to check this, false otherwise
+     */
+    export function pidIsRunning(pid: number): boolean {
+        try {
+            process.kill(pid, 0);
+            return true;
+        } catch (e) {
+            return e.code === `EPERM`;
+        }
+    }
+
     /**
      * Typing namespace
      */

diff --git a/app/test/unit/app.test.ts b/app/test/unit/app.test.ts
@@ -547,15 +547,17 @@ describe(`Library Lock`, () => {
         const tokenApp = await appFactory(validOptions.token) as TokenApp;
         const thisPID = process.pid.toString();
 
-        await tokenApp.acquireLibraryLock();
+        await expect(tokenApp.acquireLibraryLock()).resolves.toBeUndefined();
 
         const lockFile = (await fs.promises.readFile(path.join(Config.defaultConfig.dataDir, LIBRARY_LOCK_FILE_NAME), {encoding: `utf-8`})).toString();
         expect(lockFile).toEqual(thisPID);
     });
 
-    test(`Acquire lock error - already locked`, async () => {
+    test(`Acquire lock error - already locked by running process`, async () => {
         const tokenApp = await appFactory(validOptions.token) as TokenApp;
         const notThisPID = (process.pid + 1).toString();
+        Resources.pidIsRunning = jest.fn<typeof Resources.pidIsRunning>()
+            .mockReturnValue(true);
 
         mockfs({
             [Config.defaultConfig.dataDir]: {
@@ -564,22 +566,61 @@ describe(`Library Lock`, () => {
         });
 
         await expect(tokenApp.acquireLibraryLock()).rejects.toThrow(/^Library locked. Use --force \(or FORCE env variable\) to forcefully remove the lock$/);
+        expect(Resources.pidIsRunning).toHaveBeenCalled();
         expect(fs.existsSync(path.join(Resources.manager().dataDir, LIBRARY_LOCK_FILE_NAME))).toBeTruthy();
     });
 
-    test(`Acquire lock warning - already locked with --force`, async () => {
+    test(`Acquire lock warning - already locked by this process`, async () => {
+        const tokenApp = await appFactory(validOptions.token) as TokenApp;
+        const thisPID = process.pid.toString();
+
+        mockfs({
+            [Config.defaultConfig.dataDir]: {
+                [LIBRARY_LOCK_FILE_NAME]: thisPID,
+            },
+        });
+
+        await expect(tokenApp.acquireLibraryLock()).resolves.toBeUndefined();
+
+        const lockFile = (await fs.promises.readFile(path.join(Resources.manager().dataDir, LIBRARY_LOCK_FILE_NAME), {encoding: `utf-8`})).toString();
+        expect(lockFile).toEqual(thisPID);
+    });
+
+    test(`Acquire lock warning - already locked by non-running process`, async () => {
+        const tokenApp = await appFactory(validOptions.token) as TokenApp;
+        const thisPID = process.pid.toString();
+        const notThisPID = (process.pid + 1).toString();
+        Resources.pidIsRunning = jest.fn<typeof Resources.pidIsRunning>()
+            .mockReturnValue(false);
+
+        mockfs({
+            [Config.defaultConfig.dataDir]: {
+                [LIBRARY_LOCK_FILE_NAME]: notThisPID,
+            },
+        });
+
+        await expect(tokenApp.acquireLibraryLock()).resolves.toBeUndefined();
+
+        const lockFile = (await fs.promises.readFile(path.join(Resources.manager().dataDir, LIBRARY_LOCK_FILE_NAME), {encoding: `utf-8`})).toString();
+        expect(lockFile).toEqual(thisPID);
+    });
+
+    test(`Acquire lock warning - already locked by running process with --force`, async () => {
         const tokenApp = await appFactory(validOptions.tokenWithForce) as TokenApp;
         const thisPID = process.pid.toString();
         const notThisPID = (process.pid + 1).toString();
+        Resources.pidIsRunning = jest.fn<typeof Resources.pidIsRunning>()
+            .mockReturnValue(true);
 
         mockfs({
             [Resources.manager().dataDir]: {
                 [LIBRARY_LOCK_FILE_NAME]: notThisPID,
             },
         });
 
-        await tokenApp.acquireLibraryLock();
+        await expect(tokenApp.acquireLibraryLock()).resolves.toBeUndefined();
 
+        expect(Resources.pidIsRunning).toHaveBeenCalled();
         const lockFile = (await fs.promises.readFile(path.join(Resources.manager().dataDir, LIBRARY_LOCK_FILE_NAME), {encoding: `utf-8`})).toString();
         expect(lockFile).toEqual(thisPID);
     });
@@ -594,15 +635,18 @@ describe(`Library Lock`, () => {
             },
         });
 
-        await tokenApp.releaseLibraryLock();
+        await expect(tokenApp.releaseLibraryLock()).resolves.toBeUndefined();
 
         expect(fs.existsSync(path.join(Resources.manager().dataDir, LIBRARY_LOCK_FILE_NAME))).toBeFalsy();
     });
 
-    test(`Release lock error - not this process' lock`, async () => {
+    test(`Release lock error - other running process' lock`, async () => {
         const tokenApp = await appFactory(validOptions.token) as TokenApp;
         const notThisPID = (process.pid + 1).toString();
 
+        Resources.pidIsRunning = jest.fn<typeof Resources.pidIsRunning>()
+            .mockReturnValue(true);
+
         mockfs({
             [Resources.manager().dataDir]: {
                 [LIBRARY_LOCK_FILE_NAME]: notThisPID,
@@ -611,25 +655,48 @@ describe(`Library Lock`, () => {
 
         await expect(tokenApp.releaseLibraryLock()).rejects.toThrow(/^Library locked. Use --force \(or FORCE env variable\) to forcefully remove the lock$/);
 
+        expect(Resources.pidIsRunning).toHaveBeenCalled();
         expect(fs.existsSync(path.join(Resources.manager().dataDir, LIBRARY_LOCK_FILE_NAME))).toBeTruthy();
     });
 
-    test(`Release lock error - not this process' lock --force`, async () => {
+    test(`Release lock warning - other non-running process' lock`, async () => {
+        const tokenApp = await appFactory(validOptions.token) as TokenApp;
+        const notThisPID = (process.pid + 1).toString();
+
+        Resources.pidIsRunning = jest.fn<typeof Resources.pidIsRunning>()
+            .mockReturnValue(false);
+
+        mockfs({
+            [Resources.manager().dataDir]: {
+                [LIBRARY_LOCK_FILE_NAME]: notThisPID,
+            },
+        });
+
+        await expect(tokenApp.releaseLibraryLock()).resolves.toBeUndefined();
+
+        expect(Resources.pidIsRunning).toHaveBeenCalled();
+        expect(fs.existsSync(path.join(Resources.manager().dataDir, LIBRARY_LOCK_FILE_NAME))).toBeFalsy();
+    });
+
+    test(`Release lock warning - not this process' lock with --force`, async () => {
         const tokenApp = await appFactory(validOptions.tokenWithForce) as TokenApp;
         const notThisPID = (process.pid + 1).toString();
+        Resources.pidIsRunning = jest.fn<typeof Resources.pidIsRunning>()
+            .mockReturnValue(true);
 
         mockfs({
             [Resources.manager().dataDir]: {
                 [LIBRARY_LOCK_FILE_NAME]: notThisPID,
             },
         });
 
-        await tokenApp.releaseLibraryLock();
+        await expect(tokenApp.releaseLibraryLock()).resolves.toBeUndefined();
 
+        expect(Resources.pidIsRunning).toHaveBeenCalled();
         expect(fs.existsSync(path.join(Resources.manager().dataDir, LIBRARY_LOCK_FILE_NAME))).toBeFalsy();
     });
 
-    test(`Release lock error - no lock`, async () => {
+    test(`Release lock warning - no lock`, async () => {
         const tokenApp = await appFactory(validOptions.token) as TokenApp;
 
         await expect(tokenApp.releaseLibraryLock()).resolves.toBeUndefined();

diff --git a/app/test/unit/resources.test.ts b/app/test/unit/resources.test.ts
@@ -187,4 +187,15 @@ describe(`package.json metadata`, () => {
     test(`should return the application description`, () => {
         expect(Resources.PackageInfo.description).toEqual(`One-way sync engine for the iCloud Photos Library into the native file system with archiving capabilities`);
     });
+});
+
+describe(`PID helper`, () => {
+    test(`should return true if PID is running`, () => {
+        // This process' pid is running
+        expect(Resources.pidIsRunning(process.pid)).toBeTruthy();
+    });
+    test(`should return true if PID is not running`, () => {
+        // Default Linux max PID is 32768 (adjustable), Mac max PID is 99998 (non-adjustable)
+        expect(Resources.pidIsRunning(100000)).toBeFalsy();
+    });
 });
diff --git a/docs/mkdocs.yml b/docs/mkdocs.yml
@@ -67,4 +67,5 @@ plugins:
   - search
   - redirects: 
      redirect_maps: 
-        warnings.md: user-guides/common-warnings.md
+        warnings.md: user-guides/common-warnings.md
+        error-reporting.md: user-guides/error-reporting.md