Fix UnsatisfiedLinkError we've been seeing in nodejs formatters

CHANGES.md

@@ -10,6 +10,10 @@ This document is intended for Spotless developers.
 We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `1.27.0`).
 
 ## [Unreleased]
+### Added
+* `NodeJsGlobal.setSharedLibFolder` allows to set the location of nodejs shared libs. ([#586](https://github.com/diffplug/spotless/pull/586))
+### Fixed
+* Previously, the nodejs-based steps would throw `UnsatisfiedLinkError` if they were ever used from more than one classloader.  Now they can be used from any number of classloaders (important for gradle build daemon). ([#586](https://github.com/diffplug/spotless/pull/586))
 
 ## [1.31.0] - 2020-05-21
 ### Added

lib/src/main/java/com/diffplug/spotless/npm/NodeJSWrapper.java

@@ -16,11 +16,12 @@
 package com.diffplug.spotless.npm;
 
 import java.io.File;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Objects;
-import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.Set;
 
-import com.diffplug.spotless.LineEnding;
+import com.diffplug.spotless.ThrowingEx;
 
 class NodeJSWrapper extends ReflectiveObjectWrapper {
 
@@ -29,14 +30,37 @@ class NodeJSWrapper extends ReflectiveObjectWrapper {
 
 	public static final String WRAPPED_CLASS = "com.eclipsesource.v8.NodeJS";
 
-	private static final AtomicBoolean flagsSet = new AtomicBoolean(false);
+	private static final Set<ClassLoader> alreadySetup = new HashSet<>();
 
 	public NodeJSWrapper(ClassLoader classLoader) {
 		super(Reflective.withClassLoader(classLoader),
 				reflective -> {
-					final boolean firstRun = flagsSet.compareAndSet(false, true);
-					if (firstRun && LineEnding.PLATFORM_NATIVE.str().equals("\r\n")) {
-						reflective.invokeStaticMethod(V8_RUNTIME_CLASS, "setFlags", "-color=false"); // required to run prettier on windows
+					if (alreadySetup.add(classLoader)) {
+						// the bridge to node.js needs a .dll/.so/.dylib which gets loaded through System.load
+						// the problem is that when the JVM loads that DLL, it is bound to the specific classloader that called System.load
+						// no other classloaders have access to it, and if any other classloader tries to load it, you get an error
+						//
+						// ...but, you can copy that DLL as many times as you want, and each classloader can load its own copy of the DLL, and
+						// that is fine
+						//
+						// ...but the in order to do that, we have to manually load the DLL per classloader ourselves, which involves some
+						// especially hacky reflection into J2V8 *and* JVM internal
+						//
+						// so this is bad code, but it fixes our problem, and so far we don't have a better way...
+
+						// here we get the name of the DLL within the jar, and we get our own copy of it on disk
+						String resource = (String) reflective.invokeStaticMethodPrivate("com.eclipsesource.v8.LibraryLoader", "computeLibraryFullName");
+						File file = NodeJsGlobal.sharedLibs.nextDynamicLib(classLoader, resource);
+
+						// ideally, we would call System.load, but the JVM does some tricky stuff to
+						// figure out who actually called this, and it realizes it was Reflective, which lives
+						// outside the J2V8 classloader, so System.load doesn't work.  Soooo, we have to dig
+						// into JVM internals and manually tell it "this class from J2V8 called you"
+						Class<?> libraryLoaderClass = ThrowingEx.get(() -> classLoader.loadClass("com.eclipsesource.v8.LibraryLoader"));
+						reflective.invokeStaticMethodPrivate("java.lang.ClassLoader", "loadLibrary0", libraryLoaderClass, file);
+
+						// and now we set the flag in J2V8 which says "the DLL is loaded, don't load it again"
+						reflective.staticFieldPrivate("com.eclipsesource.v8.V8", "nativeLibraryLoaded", true);
 					}
 					return reflective.invokeStaticMethod(WRAPPED_CLASS, "createNodeJS");
 				});

lib/src/main/java/com/diffplug/spotless/npm/NodeJsGlobal.java

@@ -0,0 +1,93 @@
+/*
+ * Copyright 2016 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.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Optional;
+import java.util.stream.IntStream;
+
+import com.diffplug.spotless.LineEnding;
+import com.diffplug.spotless.ThrowingEx;
+
+/** Shared config acress the NodeJS steps. */
+public class NodeJsGlobal {
+	static SharedLibFolder sharedLibs;
+
+	static {
+		sharedLibs = new SharedLibFolder(
+				ThrowingEx.get(() -> Files.createTempDirectory("spotless-nodejs")));
+		sharedLibs.root.deleteOnExit();
+	}
+
+	/**
+	 * All of the NodeJS steps need to extract a bridge DLL for node.  By default this is
+	 * a random location, but you can set it to be anywhere.
+	 */
+	public static void setSharedLibFolder(File sharedLibFolder) {
+		sharedLibs = new SharedLibFolder(sharedLibFolder.toPath());
+	}
+
+	static class SharedLibFolder {
+		private final File root;
+
+		private SharedLibFolder(Path root) {
+			this.root = ThrowingEx.get(() -> root.toFile().getCanonicalFile());
+		}
+
+		static final int MAX_CLASSLOADERS_PER_CLEAN = 1_000;
+
+		synchronized File nextDynamicLib(ClassLoader loader, String resource) {
+			// find a new unique file
+			Optional<File> nextLibOpt = IntStream.range(0, MAX_CLASSLOADERS_PER_CLEAN)
+					.mapToObj(i -> new File(root, i + "_" + resource))
+					.filter(file -> !file.exists())
+					.findFirst();
+			if (!nextLibOpt.isPresent()) {
+				throw new IllegalArgumentException("Overflow, delete the spotless nodeJs cache: " + root);
+			}
+			File nextLib = nextLibOpt.get();
+			// copy the dll to it
+			try {
+				Files.createDirectories(nextLib.getParentFile().toPath());
+				try (FileOutputStream fileOut = new FileOutputStream(nextLib);
+						InputStream resourceIn = loader.loadClass("com.eclipsesource.v8.LibraryLoader").getResourceAsStream("/" + resource)) {
+					byte[] buf = new byte[0x1000];
+					while (true) {
+						int r = resourceIn.read(buf);
+						if (r == -1) {
+							break;
+						}
+						fileOut.write(buf, 0, r);
+					}
+				}
+			} catch (IOException | ClassNotFoundException e) {
+				throw ThrowingEx.asRuntime(e);
+			}
+			// make sure it is executable (on unix)
+			if (LineEnding.PLATFORM_NATIVE.str().equals("\n")) {
+				ThrowingEx.run(() -> {
+					Runtime.getRuntime().exec(new String[]{"chmod", "755", nextLib.getAbsolutePath()}).waitFor(); //$NON-NLS-1$
+				});
+			}
+			return nextLib;
+		}
+	}
+}

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

@@ -96,14 +96,14 @@ private File resolveNpm(@Nullable File npm) {
 	}
 
 	protected NodeJSWrapper nodeJSWrapper() {
-		return new NodeJSWrapper(this.jarState.getClassLoader()); // TODO (simschla, 02.08.18): cache this instance
+		return new NodeJSWrapper(this.jarState.getClassLoader());
 	}
 
 	protected File nodeModulePath() {
 		return new File(new File(this.nodeModulesDir, "node_modules"), this.npmConfig.getNpmModule());
 	}
 
-	private String j2v8MavenCoordinate() {
+	static String j2v8MavenCoordinate() {
 		return "com.eclipsesource.j2v8:j2v8_" + PlatformInfo.normalizedOSName() + "_" + PlatformInfo.normalizedArchName() + ":4.6.0";
 	}
 

lib/src/main/java/com/diffplug/spotless/npm/Reflective.java

@@ -22,6 +22,10 @@
 import java.util.Objects;
 import java.util.StringJoiner;
 
+import com.diffplug.spotless.ThrowingEx;
+
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+
 class Reflective {
 	private final ClassLoader classLoader;
 
@@ -59,6 +63,17 @@ Object invokeStaticMethod(String className, String methodName, Object... paramet
 		}
 	}
 
+	@SuppressFBWarnings("DP_DO_INSIDE_DO_PRIVILEGED")
+	Object invokeStaticMethodPrivate(String className, String methodName, Object... parameters) {
+		try {
+			Method m = staticMethod(className, methodName, parameters);
+			m.setAccessible(true);
+			return m.invoke(m.getDeclaringClass(), parameters);
+		} catch (IllegalAccessException | InvocationTargetException e) {
+			throw new ReflectiveException(e);
+		}
+	}
+
 	private Class<?>[] types(TypedValue[] typedValues) {
 		return Arrays.stream(typedValues)
 				.map(TypedValue::getClazz)
@@ -215,6 +230,18 @@ private Object staticField(Class<?> clazz, String fieldName) {
 		}
 	}
 
+	@SuppressFBWarnings("DP_DO_INSIDE_DO_PRIVILEGED")
+	void staticFieldPrivate(String className, String fieldName, boolean newValue) {
+		Class<?> clazz = clazz(className);
+		try {
+			Field field = clazz.getDeclaredField(fieldName);
+			field.setAccessible(true);
+			field.setBoolean(null, newValue);
+		} catch (NoSuchFieldException | SecurityException | IllegalArgumentException | IllegalAccessException e) {
+			throw ThrowingEx.asRuntime(e);
+		}
+	}
+
 	TypedValue typed(String className, Object obj) {
 		return new TypedValue(clazz(className), obj);
 	}

plugin-gradle/CHANGES.md

@@ -7,6 +7,8 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
 * Support for ktfmt in KotlinGradleExtension. ([#583](https://github.com/diffplug/spotless/pull/583))
 ### Fixed
 * Users can now run `spotlessCheck` and `spotlessApply` in the same build. ([#584](https://github.com/diffplug/spotless/pull/584))
+* Fixed intermittent `UnsatisfiedLinkError` in nodejs-based steps. ([#586](https://github.com/diffplug/spotless/pull/586))
+  * Also, a shared library used by the nodejs steps used to be extracted into the user home directory, but now it is extracted into `{rootProject}/build/spotless-nodejs-cache`.
 
 ## [4.0.1] - 2020-05-21
 ### Fixed

plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java

@@ -17,6 +17,7 @@
 
 import static java.util.Objects.requireNonNull;
 
+import java.io.File;
 import java.lang.reflect.Constructor;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
@@ -31,6 +32,7 @@
 
 import com.diffplug.common.base.Errors;
 import com.diffplug.spotless.LineEnding;
+import com.diffplug.spotless.npm.NodeJsGlobal;
 
 public class SpotlessExtension {
 	final Project project;
@@ -63,6 +65,8 @@ public SpotlessExtension(Project project) {
 		if (registerDependenciesTask == null) {
 			registerDependenciesTask = project.getRootProject().getTasks().create(RegisterDependenciesTask.TASK_NAME, RegisterDependenciesTask.class);
 			registerDependenciesTask.setup();
+			// set where the nodejs runtime will put its temp dlls
+			NodeJsGlobal.setSharedLibFolder(new File(project.getBuildDir(), "spotless-nodejs-cache"));
 		}
 		this.registerDependenciesTask = registerDependenciesTask;
 	}

plugin-maven/CHANGES.md

@@ -3,6 +3,8 @@
 We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `1.27.0`).
 
 ## [Unreleased]
+### Fixed
+* Shared library used by the nodejs-based steps used to be extracted into the user home directory, but now it is extracted into a temporary directory and deleted on VM shutdown. ([#586](https://github.com/diffplug/spotless/pull/586))
 
 ## [1.31.1] - 2020-05-21
 ### Fixed

testlib/src/test/java/com/diffplug/spotless/npm/NodeJsNativeDoubleLoadTest.java

@@ -0,0 +1,53 @@
+/*
+ * Copyright 2016 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.util.Optional;
+
+import org.assertj.core.api.Assertions;
+import org.junit.Test;
+
+import com.diffplug.common.collect.ImmutableMap;
+import com.diffplug.spotless.JarState;
+import com.diffplug.spotless.TestProvisioner;
+
+public class NodeJsNativeDoubleLoadTest {
+	@Test
+	public void inMultipleClassLoaders() throws Exception {
+		JarState state = JarState.from(NpmFormatterStepStateBase.j2v8MavenCoordinate(), TestProvisioner.mavenCentral());
+		ClassLoader loader1 = state.getClassLoader(1);
+		ClassLoader loader2 = state.getClassLoader(2);
+		createAndTestWrapper(loader1);
+		createAndTestWrapper(loader2);
+	}
+
+	@Test
+	public void multipleTimesInOneClassLoader() throws Exception {
+		JarState state = JarState.from(NpmFormatterStepStateBase.j2v8MavenCoordinate(), TestProvisioner.mavenCentral());
+		ClassLoader loader3 = state.getClassLoader(3);
+		createAndTestWrapper(loader3);
+		createAndTestWrapper(loader3);
+	}
+
+	private void createAndTestWrapper(ClassLoader loader) throws Exception {
+		try (NodeJSWrapper node = new NodeJSWrapper(loader)) {
+			V8ObjectWrapper object = node.createNewObject(ImmutableMap.of("a", 1));
+			Optional<Integer> value = object.getOptionalInteger("a");
+			Assertions.assertThat(value).hasValue(1);
+			object.release();
+		}
+	}
+}