Skip to content

Commit

Permalink
refactor: do not use file-based locking for installConfigUpdate
Browse files Browse the repository at this point in the history
  • Loading branch information
AlCalzone committed Dec 10, 2024
1 parent ee3c587 commit 7a7b868
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 36 deletions.
19 changes: 19 additions & 0 deletions packages/zwave-js/src/lib/driver/Driver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1461,7 +1461,9 @@ export class Driver extends TypedEventTarget<DriverEventCallbacks>

// Try to create the cache directory. This can fail, in which case we should expose a good error message
try {
// eslint-disable-next-line @typescript-eslint/no-deprecated
if (this._options.storage.driver) {
// eslint-disable-next-line @typescript-eslint/no-deprecated
await this._options.storage.driver.ensureDir(this.cacheDir);
} else {
await this.bindings.fs.ensureDir(this.cacheDir);
Expand Down Expand Up @@ -1657,8 +1659,10 @@ export class Driver extends TypedEventTarget<DriverEventCallbacks>
this.controller.homeId,
this._networkCache,
this._valueDB,
// eslint-disable-next-line @typescript-eslint/no-deprecated
this._options.storage.driver
? wrapLegacyFSDriverForCacheMigrationOnly(
// eslint-disable-next-line @typescript-eslint/no-deprecated
this._options.storage.driver,
)
: this.bindings.fs,
Expand Down Expand Up @@ -7417,6 +7421,8 @@ ${handlers.length} left`,
}
}

private _installConfigUpdatePromise: Promise<boolean> | undefined;

/**
* Installs an update for the embedded configuration DB if there is a compatible one.
* Returns `true` when an update was installed, `false` otherwise.
Expand All @@ -7426,6 +7432,19 @@ ${handlers.length} left`,
public async installConfigUpdate(): Promise<boolean> {
this.ensureReady();

if (this._installConfigUpdatePromise) {
return this._installConfigUpdatePromise;
}
this._installConfigUpdatePromise = this.installConfigUpdateInternal();

try {
return await this._installConfigUpdatePromise;
} finally {
this._installConfigUpdatePromise = undefined;
}
}

private async installConfigUpdateInternal(): Promise<boolean> {
const newVersion = await this.checkForConfigUpdates(true);
if (!newVersion) return false;

Expand Down
36 changes: 0 additions & 36 deletions packages/zwave-js/src/lib/driver/UpdateConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import execa from "execa";
import fsp from "node:fs/promises";
import os from "node:os";
import path from "pathe";
import * as lockfile from "proper-lockfile";
import semverInc from "semver/functions/inc.js";
import semverValid from "semver/functions/valid.js";
import semverMaxSatisfying from "semver/ranges/max-satisfying.js";
Expand Down Expand Up @@ -100,43 +99,13 @@ export async function installConfigUpdate(
);
}

const lockfilePath = external.cacheDir;
const lockfileOptions: lockfile.LockOptions = {
lockfilePath: path.join(external.cacheDir, "config-update.lock"),
};

try {
await lockfile.lock(lockfilePath, {
...lockfileOptions,
onCompromised: () => {
// do nothing
},
});
} catch {
throw new ZWaveError(
`Config update failed: Another installation is already in progress!`,
ZWaveErrorCodes.Config_Update_InstallFailed,
);
}

const freeLock = async () => {
try {
if (await lockfile.check(lockfilePath, lockfileOptions)) {
await lockfile.unlock(lockfilePath, lockfileOptions);
}
} catch {
// whatever - just don't crash
}
};

// Download tarball to a temporary directory
let tmpDir: string;
try {
tmpDir = await fsp.mkdtemp(
path.join(os.tmpdir(), "zjs-config-update-"),
);
} catch (e) {
await freeLock();
throw new ZWaveError(
`Config update failed: Could not create temporary directory. Reason: ${
getErrorMessage(
Expand All @@ -161,7 +130,6 @@ export async function installConfigUpdate(
const response = await ky.get(url);
await response.body?.pipeTo(writable);
} catch (e) {
await freeLock();
throw new ZWaveError(
`Config update failed: Could not download tarball. Reason: ${
getErrorMessage(
Expand Down Expand Up @@ -211,7 +179,6 @@ export async function installConfigUpdate(
);
await writeTextFile(fs, externalVersionFilename, newVersion);
} catch {
await freeLock();
throw new ZWaveError(
`Config update failed: Could not extract tarball`,
ZWaveErrorCodes.Config_Update_InstallFailed,
Expand All @@ -220,7 +187,4 @@ export async function installConfigUpdate(

// Clean up the temp dir and ignore errors
await fs.deleteDir(tmpDir).catch(noop);

// Free the lock
await freeLock();
}

0 comments on commit 7a7b868

Please sign in to comment.