diff --git a/CHANGES.md b/CHANGES.md index 7d16300cce..6b2092441d 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -13,6 +13,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( ### Changed * Use palantir-java-format 2.57.0 on Java 21. ([#2447](https://github.com/diffplug/spotless/pull/2447)) * Re-try `npm install` with `--prefer-online` after `ERESOLVE` error. ([#2448](https://github.com/diffplug/spotless/pull/2448)) +* Allow multiple npm-based formatters having the same module dependencies, to share a `node_modules` dir without race conditions. [#2462](https://github.com/diffplug/spotless/pull/2462)) ## [3.1.0] - 2025-02-20 ### Added diff --git a/lib/src/main/java/com/diffplug/spotless/npm/ExclusiveFolderAccess.java b/lib/src/main/java/com/diffplug/spotless/npm/ExclusiveFolderAccess.java new file mode 100644 index 0000000000..f36fcbb8b4 --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/npm/ExclusiveFolderAccess.java @@ -0,0 +1,67 @@ +/* + * Copyright 2025 DiffPlug + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.diffplug.spotless.npm; + +import java.io.File; +import java.util.Objects; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; + +import javax.annotation.Nonnull; + +import com.diffplug.spotless.ThrowingEx; + +interface ExclusiveFolderAccess { + + static ExclusiveFolderAccess forFolder(@Nonnull File folder) { + return forFolder(folder.getAbsolutePath()); + } + + static ExclusiveFolderAccess forFolder(@Nonnull String path) { + return new ExclusiveFolderAccessSharedMutex(Objects.requireNonNull(path)); + } + + void runExclusively(ThrowingEx.Runnable runnable); + + class ExclusiveFolderAccessSharedMutex implements ExclusiveFolderAccess { + + private static final ConcurrentHashMap mutexes = new ConcurrentHashMap<>(); + + private final String path; + + private ExclusiveFolderAccessSharedMutex(@Nonnull String path) { + this.path = Objects.requireNonNull(path); + } + + private Lock getMutex() { + return mutexes.computeIfAbsent(path, k -> new ReentrantLock()); + } + + @Override + public void runExclusively(ThrowingEx.Runnable runnable) { + final Lock lock = getMutex(); + try { + lock.lock(); + runnable.run(); + } catch (Exception e) { + throw ThrowingEx.asRuntime(e); + } finally { + lock.unlock(); + } + } + } +} diff --git a/lib/src/main/java/com/diffplug/spotless/npm/NodeModulesCachingNpmProcessFactory.java b/lib/src/main/java/com/diffplug/spotless/npm/NodeModulesCachingNpmProcessFactory.java index 2f1addf2af..a821dd6b8e 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/NodeModulesCachingNpmProcessFactory.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/NodeModulesCachingNpmProcessFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2023-2024 DiffPlug + * Copyright 2023-2025 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,6 +18,7 @@ import java.io.File; import java.util.List; import java.util.Objects; +import java.util.UUID; import javax.annotation.Nonnull; @@ -65,8 +66,8 @@ public NpmProcess createNpmInstallProcess(NodeServerLayout nodeServerLayout, Npm } @Override - public NpmLongRunningProcess createNpmServeProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations) { - return StandardNpmProcessFactory.INSTANCE.createNpmServeProcess(nodeServerLayout, formatterStepLocations); + public NpmLongRunningProcess createNpmServeProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations, UUID nodeServerInstanceId) { + return StandardNpmProcessFactory.INSTANCE.createNpmServeProcess(nodeServerLayout, formatterStepLocations, nodeServerInstanceId); } private class CachingNmpInstall implements NpmProcess { diff --git a/lib/src/main/java/com/diffplug/spotless/npm/NodeServeApp.java b/lib/src/main/java/com/diffplug/spotless/npm/NodeServeApp.java index c9311a5589..3e2efd43e8 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/NodeServeApp.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/NodeServeApp.java @@ -1,5 +1,5 @@ /* - * Copyright 2023 DiffPlug + * Copyright 2023-2025 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,6 +15,8 @@ */ package com.diffplug.spotless.npm; +import java.util.UUID; + import javax.annotation.Nonnull; import org.slf4j.Logger; @@ -32,9 +34,9 @@ public NodeServeApp(@Nonnull NodeServerLayout nodeServerLayout, @Nonnull NpmConf super(nodeServerLayout, npmConfig, formatterStepLocations); } - ProcessRunner.LongRunningProcess startNpmServeProcess() { + ProcessRunner.LongRunningProcess startNpmServeProcess(UUID nodeServerInstanceId) { return timedLogger.withInfo("Starting npm based server in {} with {}.", this.nodeServerLayout.nodeModulesDir(), this.npmProcessFactory.describe()) - .call(() -> npmProcessFactory.createNpmServeProcess(nodeServerLayout, formatterStepLocations).start()); + .call(() -> npmProcessFactory.createNpmServeProcess(nodeServerLayout, formatterStepLocations, nodeServerInstanceId).start()); } } diff --git a/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java b/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java index 3ec06e0434..9b37533880 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2024 DiffPlug + * Copyright 2016-2025 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -25,6 +25,7 @@ import java.util.Iterator; import java.util.Map; import java.util.Map.Entry; +import java.util.UUID; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -32,6 +33,7 @@ import org.slf4j.LoggerFactory; import com.diffplug.spotless.FormatterFunc; +import com.diffplug.spotless.ProcessRunner; import com.diffplug.spotless.ProcessRunner.LongRunningProcess; import com.diffplug.spotless.ThrowingEx; @@ -87,14 +89,17 @@ protected void prepareNodeServer() throws IOException { } protected void assertNodeServerDirReady() throws IOException { - if (needsPrepareNodeServerLayout()) { - // reinstall if missing - prepareNodeServerLayout(); - } - if (needsPrepareNodeServer()) { - // run npm install if node_modules is missing - prepareNodeServer(); - } + ExclusiveFolderAccess.forFolder(nodeServerLayout.nodeModulesDir()) + .runExclusively(() -> { + if (needsPrepareNodeServerLayout()) { + // reinstall if missing + prepareNodeServerLayout(); + } + if (needsPrepareNodeServer()) { + // run npm install if node_modules is missing + prepareNodeServer(); + } + }); } protected boolean needsPrepareNodeServer() { @@ -109,12 +114,13 @@ protected ServerProcessInfo npmRunServer() throws ServerStartException, IOExcept assertNodeServerDirReady(); LongRunningProcess server = null; try { - // The npm process will output the randomly selected port of the http server process to 'server.port' file + final UUID nodeServerInstanceId = UUID.randomUUID(); + // The npm process will output the randomly selected port of the http server process to 'server-.port' file // so in order to be safe, remove such a file if it exists before starting. - final File serverPortFile = new File(this.nodeServerLayout.nodeModulesDir(), "server.port"); + final File serverPortFile = new File(this.nodeServerLayout.nodeModulesDir(), String.format("server-%s.port", nodeServerInstanceId)); NpmResourceHelper.deleteFileIfExists(serverPortFile); // start the http server in node - server = nodeServeApp.startNpmServeProcess(); + server = nodeServeApp.startNpmServeProcess(nodeServerInstanceId); // await the readiness of the http server - wait for at most 60 seconds try { @@ -124,10 +130,12 @@ protected ServerProcessInfo npmRunServer() throws ServerStartException, IOExcept try { if (server.isAlive()) { server.destroyForcibly(); - server.waitFor(); + ProcessRunner.Result result = server.result(); + logger.info("Launching npm server process failed. Process result:\n{}", result); } } catch (Throwable t) { - // ignore + ProcessRunner.Result result = ThrowingEx.get(server::result); + logger.debug("Unable to forcibly end the server process. Process result:\n{}", result, t); } throw timeoutException; } diff --git a/lib/src/main/java/com/diffplug/spotless/npm/NpmProcessFactory.java b/lib/src/main/java/com/diffplug/spotless/npm/NpmProcessFactory.java index dbc9a807d5..4b3edee462 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/NpmProcessFactory.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/NpmProcessFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2023 DiffPlug + * Copyright 2023-2025 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,6 +15,8 @@ */ package com.diffplug.spotless.npm; +import java.util.UUID; + public interface NpmProcessFactory { enum OnlinePreferrence { @@ -33,7 +35,7 @@ public String option() { NpmProcess createNpmInstallProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations, OnlinePreferrence onlinePreferrence); - NpmLongRunningProcess createNpmServeProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations); + NpmLongRunningProcess createNpmServeProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations, UUID nodeServerInstanceId); default String describe() { return getClass().getSimpleName(); diff --git a/lib/src/main/java/com/diffplug/spotless/npm/StandardNpmProcessFactory.java b/lib/src/main/java/com/diffplug/spotless/npm/StandardNpmProcessFactory.java index 91cf80ad69..b9abd03029 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/StandardNpmProcessFactory.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/StandardNpmProcessFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2023 DiffPlug + * Copyright 2023-2025 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,6 +19,7 @@ import java.io.IOException; import java.util.List; import java.util.Map; +import java.util.UUID; import java.util.concurrent.ExecutionException; import com.diffplug.spotless.ProcessRunner; @@ -37,8 +38,8 @@ public NpmProcess createNpmInstallProcess(NodeServerLayout nodeServerLayout, Npm } @Override - public NpmLongRunningProcess createNpmServeProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations) { - return new NpmServe(nodeServerLayout.nodeModulesDir(), formatterStepLocations); + public NpmLongRunningProcess createNpmServeProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations, UUID nodeServerInstanceId) { + return new NpmServe(nodeServerLayout.nodeModulesDir(), formatterStepLocations, nodeServerInstanceId); } private static abstract class AbstractStandardNpmProcess { @@ -119,8 +120,11 @@ public ProcessRunner.Result waitFor() { private static class NpmServe extends AbstractStandardNpmProcess implements NpmLongRunningProcess { - public NpmServe(File workingDir, NpmFormatterStepLocations formatterStepLocations) { + private final UUID nodeServerInstanceId; + + public NpmServe(File workingDir, NpmFormatterStepLocations formatterStepLocations, UUID nodeServerInstanceId) { super(workingDir, formatterStepLocations); + this.nodeServerInstanceId = nodeServerInstanceId; } @Override @@ -128,7 +132,9 @@ protected List commandLine() { return List.of( npmExecutable(), "start", - "--scripts-prepend-node-path=true"); + "--scripts-prepend-node-path=true", + "--", + "--node-server-instance-id=" + nodeServerInstanceId); } @Override diff --git a/lib/src/main/resources/com/diffplug/spotless/npm/common-serve.js b/lib/src/main/resources/com/diffplug/spotless/npm/common-serve.js index c1c9d62757..5bfcb35612 100644 --- a/lib/src/main/resources/com/diffplug/spotless/npm/common-serve.js +++ b/lib/src/main/resources/com/diffplug/spotless/npm/common-serve.js @@ -4,7 +4,7 @@ const GracefulShutdownManager = require("@moebius/http-graceful-shutdown").Grace const express = require("express"); const app = express(); -app.use(express.json({ limit: "50mb" })); +app.use(express.json({limit: "50mb"})); const fs = require("fs"); @@ -14,13 +14,33 @@ function debugLog() { } } +function getInstanceId() { + const args = process.argv.slice(2); + + // Look for the --node-server-instance-id option + let instanceId; + + args.forEach(arg => { + if (arg.startsWith('--node-server-instance-id=')) { + instanceId = arg.split('=')[1]; + } + }); + + // throw if instanceId is not set + if (!instanceId) { + throw new Error("Missing --node-server-instance-id argument"); + } + return instanceId; +} + var listener = app.listen(0, "127.0.0.1", () => { - debugLog("Server running on port " + listener.address().port); - fs.writeFile("server.port.tmp", "" + listener.address().port, function(err) { + const instanceId = getInstanceId(); + debugLog("Server running on port " + listener.address().port + " for instance " + instanceId); + fs.writeFile("server.port.tmp", "" + listener.address().port, function (err) { if (err) { return console.log(err); } else { - fs.rename("server.port.tmp", "server.port", function(err) { + fs.rename("server.port.tmp", `server-${instanceId}.port`, function (err) { if (err) { return console.log(err); } @@ -32,7 +52,7 @@ const shutdownManager = new GracefulShutdownManager(listener); app.post("/shutdown", (req, res) => { res.status(200).send("Shutting down"); - setTimeout(function() { + setTimeout(function () { shutdownManager.terminate(() => debugLog("graceful shutdown finished.")); }, 200); }); diff --git a/plugin-gradle/CHANGES.md b/plugin-gradle/CHANGES.md index 5d2cb19742..25bfbdef92 100644 --- a/plugin-gradle/CHANGES.md +++ b/plugin-gradle/CHANGES.md @@ -7,6 +7,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( * Use palantir-java-format 2.57.0 on Java 21. ([#2447](https://github.com/diffplug/spotless/pull/2447)) * Re-try `npm install` with `--prefer-online` after `ERESOLVE` error. ([#2448](https://github.com/diffplug/spotless/pull/2448)) * Apply Gradle's strict plugin types validation to the Spotless plugin. ([#2454](https://github.com/diffplug/spotless/pull/2454)) +* Allow multiple npm-based formatters having the same module dependencies, to share a `node_modules` dir without race conditions. [#2462](https://github.com/diffplug/spotless/pull/2462)) ## [7.0.2] - 2025-01-14 ### Fixed diff --git a/plugin-maven/CHANGES.md b/plugin-maven/CHANGES.md index 483258c36a..38402fc5ad 100644 --- a/plugin-maven/CHANGES.md +++ b/plugin-maven/CHANGES.md @@ -6,6 +6,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( ### Changed * Use palantir-java-format 2.57.0 on Java 21. ([#2447](https://github.com/diffplug/spotless/pull/2447)) * Re-try `npm install` with `--prefer-online` after `ERESOLVE` error. ([#2448](https://github.com/diffplug/spotless/pull/2448)) +* Allow multiple npm-based formatters having the same module dependencies, to share a `node_modules` dir without race conditions. [#2462](https://github.com/diffplug/spotless/pull/2462)) ## [2.44.3] - 2025-02-20 * Support for `clang-format` ([#2406](https://github.com/diffplug/spotless/pull/2406))