Skip to content

Commit d97b95d

Browse files
nedtwiggnedtwigg
authored
fix: allow concurrent prettier setups (#2462)
2 parents 395a79a + 279372b commit d97b95d

File tree

10 files changed

+141
-32
lines changed

10 files changed

+141
-32
lines changed

‎CHANGES.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
1818
### Changed
1919
* Use palantir-java-format 2.57.0 on Java 21. ([#2447](https://github.com/diffplug/spotless/pull/2447))
2020
* Re-try `npm install` with `--prefer-online` after `ERESOLVE` error. ([#2448](https://github.com/diffplug/spotless/pull/2448))
21+
* 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))
2122

2223
## [3.1.0] - 2025-02-20
2324
### Added

‎lib/src/main/java/com/diffplug/spotless/npm/ExclusiveFolderAccess.java

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/*
2+
* Copyright 2025 DiffPlug
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.diffplug.spotless.npm;
17+
18+
import java.io.File;
19+
import java.util.Objects;
20+
import java.util.concurrent.ConcurrentHashMap;
21+
import java.util.concurrent.locks.Lock;
22+
import java.util.concurrent.locks.ReentrantLock;
23+
24+
import javax.annotation.Nonnull;
25+
26+
import com.diffplug.spotless.ThrowingEx;
27+
28+
interface ExclusiveFolderAccess {
29+
30+
static ExclusiveFolderAccess forFolder(@Nonnull File folder) {
31+
return forFolder(folder.getAbsolutePath());
32+
}
33+
34+
static ExclusiveFolderAccess forFolder(@Nonnull String path) {
35+
return new ExclusiveFolderAccessSharedMutex(Objects.requireNonNull(path));
36+
}
37+
38+
void runExclusively(ThrowingEx.Runnable runnable);
39+
40+
class ExclusiveFolderAccessSharedMutex implements ExclusiveFolderAccess {
41+
42+
private static final ConcurrentHashMap<String, Lock> mutexes = new ConcurrentHashMap<>();
43+
44+
private final String path;
45+
46+
private ExclusiveFolderAccessSharedMutex(@Nonnull String path) {
47+
this.path = Objects.requireNonNull(path);
48+
}
49+
50+
private Lock getMutex() {
51+
return mutexes.computeIfAbsent(path, k -> new ReentrantLock());
52+
}
53+
54+
@Override
55+
public void runExclusively(ThrowingEx.Runnable runnable) {
56+
final Lock lock = getMutex();
57+
try {
58+
lock.lock();
59+
runnable.run();
60+
} catch (Exception e) {
61+
throw ThrowingEx.asRuntime(e);
62+
} finally {
63+
lock.unlock();
64+
}
65+
}
66+
}
67+
}

‎lib/src/main/java/com/diffplug/spotless/npm/NodeModulesCachingNpmProcessFactory.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2023-2024 DiffPlug
2+
* Copyright 2023-2025 DiffPlug
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -18,6 +18,7 @@
1818
import java.io.File;
1919
import java.util.List;
2020
import java.util.Objects;
21+
import java.util.UUID;
2122

2223
import javax.annotation.Nonnull;
2324

@@ -65,8 +66,8 @@ public NpmProcess createNpmInstallProcess(NodeServerLayout nodeServerLayout, Npm
6566
}
6667

6768
@Override
68-
public NpmLongRunningProcess createNpmServeProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations) {
69-
return StandardNpmProcessFactory.INSTANCE.createNpmServeProcess(nodeServerLayout, formatterStepLocations);
69+
public NpmLongRunningProcess createNpmServeProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations, UUID nodeServerInstanceId) {
70+
return StandardNpmProcessFactory.INSTANCE.createNpmServeProcess(nodeServerLayout, formatterStepLocations, nodeServerInstanceId);
7071
}
7172

7273
private class CachingNmpInstall implements NpmProcess {

‎lib/src/main/java/com/diffplug/spotless/npm/NodeServeApp.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2023 DiffPlug
2+
* Copyright 2023-2025 DiffPlug
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -15,6 +15,8 @@
1515
*/
1616
package com.diffplug.spotless.npm;
1717

18+
import java.util.UUID;
19+
1820
import javax.annotation.Nonnull;
1921

2022
import org.slf4j.Logger;
@@ -32,9 +34,9 @@ public NodeServeApp(@Nonnull NodeServerLayout nodeServerLayout, @Nonnull NpmConf
3234
super(nodeServerLayout, npmConfig, formatterStepLocations);
3335
}
3436

35-
ProcessRunner.LongRunningProcess startNpmServeProcess() {
37+
ProcessRunner.LongRunningProcess startNpmServeProcess(UUID nodeServerInstanceId) {
3638
return timedLogger.withInfo("Starting npm based server in {} with {}.", this.nodeServerLayout.nodeModulesDir(), this.npmProcessFactory.describe())
37-
.call(() -> npmProcessFactory.createNpmServeProcess(nodeServerLayout, formatterStepLocations).start());
39+
.call(() -> npmProcessFactory.createNpmServeProcess(nodeServerLayout, formatterStepLocations, nodeServerInstanceId).start());
3840
}
3941

4042
}

‎lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2016-2024 DiffPlug
2+
* Copyright 2016-2025 DiffPlug
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -25,13 +25,15 @@
2525
import java.util.Iterator;
2626
import java.util.Map;
2727
import java.util.Map.Entry;
28+
import java.util.UUID;
2829
import java.util.concurrent.TimeUnit;
2930
import java.util.concurrent.TimeoutException;
3031

3132
import org.slf4j.Logger;
3233
import org.slf4j.LoggerFactory;
3334

3435
import com.diffplug.spotless.FormatterFunc;
36+
import com.diffplug.spotless.ProcessRunner;
3537
import com.diffplug.spotless.ProcessRunner.LongRunningProcess;
3638
import com.diffplug.spotless.ThrowingEx;
3739

@@ -87,14 +89,17 @@ protected void prepareNodeServer() throws IOException {
8789
}
8890

8991
protected void assertNodeServerDirReady() throws IOException {
90-
if (needsPrepareNodeServerLayout()) {
91-
// reinstall if missing
92-
prepareNodeServerLayout();
93-
}
94-
if (needsPrepareNodeServer()) {
95-
// run npm install if node_modules is missing
96-
prepareNodeServer();
97-
}
92+
ExclusiveFolderAccess.forFolder(nodeServerLayout.nodeModulesDir())
93+
.runExclusively(() -> {
94+
if (needsPrepareNodeServerLayout()) {
95+
// reinstall if missing
96+
prepareNodeServerLayout();
97+
}
98+
if (needsPrepareNodeServer()) {
99+
// run npm install if node_modules is missing
100+
prepareNodeServer();
101+
}
102+
});
98103
}
99104

100105
protected boolean needsPrepareNodeServer() {
@@ -109,12 +114,13 @@ protected ServerProcessInfo npmRunServer() throws ServerStartException, IOExcept
109114
assertNodeServerDirReady();
110115
LongRunningProcess server = null;
111116
try {
112-
// The npm process will output the randomly selected port of the http server process to 'server.port' file
117+
final UUID nodeServerInstanceId = UUID.randomUUID();
118+
// The npm process will output the randomly selected port of the http server process to 'server-<id>.port' file
113119
// so in order to be safe, remove such a file if it exists before starting.
114-
final File serverPortFile = new File(this.nodeServerLayout.nodeModulesDir(), "server.port");
120+
final File serverPortFile = new File(this.nodeServerLayout.nodeModulesDir(), String.format("server-%s.port", nodeServerInstanceId));
115121
NpmResourceHelper.deleteFileIfExists(serverPortFile);
116122
// start the http server in node
117-
server = nodeServeApp.startNpmServeProcess();
123+
server = nodeServeApp.startNpmServeProcess(nodeServerInstanceId);
118124

119125
// await the readiness of the http server - wait for at most 60 seconds
120126
try {
@@ -124,10 +130,12 @@ protected ServerProcessInfo npmRunServer() throws ServerStartException, IOExcept
124130
try {
125131
if (server.isAlive()) {
126132
server.destroyForcibly();
127-
server.waitFor();
133+
ProcessRunner.Result result = server.result();
134+
logger.info("Launching npm server process failed. Process result:\n{}", result);
128135
}
129136
} catch (Throwable t) {
130-
// ignore
137+
ProcessRunner.Result result = ThrowingEx.get(server::result);
138+
logger.debug("Unable to forcibly end the server process. Process result:\n{}", result, t);
131139
}
132140
throw timeoutException;
133141
}

‎lib/src/main/java/com/diffplug/spotless/npm/NpmProcessFactory.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2023 DiffPlug
2+
* Copyright 2023-2025 DiffPlug
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -15,6 +15,8 @@
1515
*/
1616
package com.diffplug.spotless.npm;
1717

18+
import java.util.UUID;
19+
1820
public interface NpmProcessFactory {
1921

2022
enum OnlinePreferrence {
@@ -33,7 +35,7 @@ public String option() {
3335

3436
NpmProcess createNpmInstallProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations, OnlinePreferrence onlinePreferrence);
3537

36-
NpmLongRunningProcess createNpmServeProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations);
38+
NpmLongRunningProcess createNpmServeProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations, UUID nodeServerInstanceId);
3739

3840
default String describe() {
3941
return getClass().getSimpleName();

‎lib/src/main/java/com/diffplug/spotless/npm/StandardNpmProcessFactory.java

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2023 DiffPlug
2+
* Copyright 2023-2025 DiffPlug
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -19,6 +19,7 @@
1919
import java.io.IOException;
2020
import java.util.List;
2121
import java.util.Map;
22+
import java.util.UUID;
2223
import java.util.concurrent.ExecutionException;
2324

2425
import com.diffplug.spotless.ProcessRunner;
@@ -37,8 +38,8 @@ public NpmProcess createNpmInstallProcess(NodeServerLayout nodeServerLayout, Npm
3738
}
3839

3940
@Override
40-
public NpmLongRunningProcess createNpmServeProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations) {
41-
return new NpmServe(nodeServerLayout.nodeModulesDir(), formatterStepLocations);
41+
public NpmLongRunningProcess createNpmServeProcess(NodeServerLayout nodeServerLayout, NpmFormatterStepLocations formatterStepLocations, UUID nodeServerInstanceId) {
42+
return new NpmServe(nodeServerLayout.nodeModulesDir(), formatterStepLocations, nodeServerInstanceId);
4243
}
4344

4445
private static abstract class AbstractStandardNpmProcess {
@@ -119,16 +120,21 @@ public ProcessRunner.Result waitFor() {
119120

120121
private static class NpmServe extends AbstractStandardNpmProcess implements NpmLongRunningProcess {
121122

122-
public NpmServe(File workingDir, NpmFormatterStepLocations formatterStepLocations) {
123+
private final UUID nodeServerInstanceId;
124+
125+
public NpmServe(File workingDir, NpmFormatterStepLocations formatterStepLocations, UUID nodeServerInstanceId) {
123126
super(workingDir, formatterStepLocations);
127+
this.nodeServerInstanceId = nodeServerInstanceId;
124128
}
125129

126130
@Override
127131
protected List<String> commandLine() {
128132
return List.of(
129133
npmExecutable(),
130134
"start",
131-
"--scripts-prepend-node-path=true");
135+
"--scripts-prepend-node-path=true",
136+
"--",
137+
"--node-server-instance-id=" + nodeServerInstanceId);
132138
}
133139

134140
@Override

‎lib/src/main/resources/com/diffplug/spotless/npm/common-serve.js

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ const GracefulShutdownManager = require("@moebius/http-graceful-shutdown").Grace
44
const express = require("express");
55
const app = express();
66

7-
app.use(express.json({ limit: "50mb" }));
7+
app.use(express.json({limit: "50mb"}));
88

99
const fs = require("fs");
1010

@@ -14,13 +14,33 @@ function debugLog() {
1414
}
1515
}
1616

17+
function getInstanceId() {
18+
const args = process.argv.slice(2);
19+
20+
// Look for the --node-server-instance-id option
21+
let instanceId;
22+
23+
args.forEach(arg => {
24+
if (arg.startsWith('--node-server-instance-id=')) {
25+
instanceId = arg.split('=')[1];
26+
}
27+
});
28+
29+
// throw if instanceId is not set
30+
if (!instanceId) {
31+
throw new Error("Missing --node-server-instance-id argument");
32+
}
33+
return instanceId;
34+
}
35+
1736
var listener = app.listen(0, "127.0.0.1", () => {
18-
debugLog("Server running on port " + listener.address().port);
19-
fs.writeFile("server.port.tmp", "" + listener.address().port, function(err) {
37+
const instanceId = getInstanceId();
38+
debugLog("Server running on port " + listener.address().port + " for instance " + instanceId);
39+
fs.writeFile("server.port.tmp", "" + listener.address().port, function (err) {
2040
if (err) {
2141
return console.log(err);
2242
} else {
23-
fs.rename("server.port.tmp", "server.port", function(err) {
43+
fs.rename("server.port.tmp", `server-${instanceId}.port`, function (err) {
2444
if (err) {
2545
return console.log(err);
2646
}
@@ -32,7 +52,7 @@ const shutdownManager = new GracefulShutdownManager(listener);
3252

3353
app.post("/shutdown", (req, res) => {
3454
res.status(200).send("Shutting down");
35-
setTimeout(function() {
55+
setTimeout(function () {
3656
shutdownManager.terminate(() => debugLog("graceful shutdown finished."));
3757
}, 200);
3858
});

‎plugin-gradle/CHANGES.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
1212
* Use palantir-java-format 2.57.0 on Java 21. ([#2447](https://github.com/diffplug/spotless/pull/2447))
1313
* Re-try `npm install` with `--prefer-online` after `ERESOLVE` error. ([#2448](https://github.com/diffplug/spotless/pull/2448))
1414
* Apply Gradle's strict plugin types validation to the Spotless plugin. ([#2454](https://github.com/diffplug/spotless/pull/2454))
15+
* 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))
1516

1617
## [7.0.2] - 2025-01-14
1718
### Fixed

‎plugin-maven/CHANGES.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
1111
### Changed
1212
* Use palantir-java-format 2.57.0 on Java 21. ([#2447](https://github.com/diffplug/spotless/pull/2447))
1313
* Re-try `npm install` with `--prefer-online` after `ERESOLVE` error. ([#2448](https://github.com/diffplug/spotless/pull/2448))
14+
* 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))
1415

1516
## [2.44.3] - 2025-02-20
1617
* Support for `clang-format` ([#2406](https://github.com/diffplug/spotless/pull/2406))

0 commit comments

Comments
 (0)