Bug 547109 - [GTK] JVM crash when multiple processes were started and native libraries are missing

Change 1
--------
Libraries are now first extracted into a temporary file, then moved to
where they should be. This way, applications no longer see partially
written libraries where good ones are expected.

Change 2
--------
Some refactoring

Change 3
--------
extract() now returns true when file is already there. This makes the
behavior more reasonable in multi-process environment.

Change-Id: I43052a57b240fb3fef7474fcec627ea2b5f8414c
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>

2 files changed, 188 insertions, 32 deletions

diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/common/org/eclipse/swt/internal/Library.java b/bundles/org.eclipse.swt/Eclipse SWT PI/common/org/eclipse/swt/internal/Library.java
index 2588daf932..05b645444f 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/common/org/eclipse/swt/internal/Library.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/common/org/eclipse/swt/internal/Library.java
@@ -16,6 +16,8 @@ package org.eclipse.swt.internal;
 import java.io.*;
 import java.lang.reflect.*;
 import java.net.*;
+import java.nio.file.Files;
+import java.nio.file.StandardCopyOption;
 import java.util.function.*;
 import java.util.jar.*;
 
@@ -136,45 +138,59 @@ public static int SWT_VERSION (int major, int minor) {
 	return major * 1000 + minor;
 }
 
+private static boolean extractResource(String resourceName, File outFile) {
+	try (InputStream inputStream = Library.class.getResourceAsStream (resourceName)) {
+		if (inputStream == null) return false;
+		Files.copy(inputStream, outFile.toPath(), StandardCopyOption.REPLACE_EXISTING);
+	} catch (Throwable e) {
+		return false;
+	}
+
+	return true;
+}
+
 /**
- *	Extract file with 'mappedName' into path 'extractToFilePath'. Cleanup leftovers if extract failed.
+ * Extract file with 'mappedName' into path 'extractToFilePath'.
+ * Does not overwrite existing file.
+ * Does not leave trash on error.
  * @param extractToFilePath full path of where the file is to be extacted to, inc name of file,
  *                          e.g /home/USER/.swt/lib/linux/x86_64/libswt-MYLIB-gtk-4826.so
  * @param mappedName file to be searched in jar.
  * @return	true upon success, failure if something went wrong.
  */
-static boolean extract (String extractToFilePath, String mappedName, StringBuilder message) {
-	FileOutputStream os = null;
-	InputStream is = null;
+static boolean extract (String extractToFilePath, String mappedName) {
 	File file = new File(extractToFilePath);
-	boolean extracted = false;
+	if (file.exists ()) return true;
+
+	// Write to temp file first, so that other processes don't see
+	// partially written library on disk
+	File tempFile;
 	try {
-		if (!file.exists ()) {
-			is = Library.class.getResourceAsStream ("/" + mappedName); //$NON-NLS-1$
-			if (is != null) {
-				extracted = true;
-				int read;
-				byte [] buffer = new byte [4096];
-				os = new FileOutputStream (extractToFilePath);
-				while ((read = is.read (buffer)) != -1) {
-					os.write(buffer, 0, read);
-				}
-				os.close ();
-				is.close ();
-				chmod ("755", extractToFilePath);
-				return true;
-			}
-		}
+		tempFile = File.createTempFile (file.getName(), ".tmp", file.getParentFile()); //$NON-NLS-1$
 	} catch (Throwable e) {
-		try {
-			if (os != null) os.close ();
-		} catch (IOException e1) {}
-		try {
-			if (is != null) is.close ();
-		} catch (IOException e1) {}
-		if (extracted && file.exists ()) file.delete ();
+		return false;
 	}
-	return false;
+
+	// Extract resource
+	String resourceName = "/" + mappedName; //$NON-NLS-1$
+	if (!extractResource (resourceName, tempFile)) {
+		tempFile.delete();
+		return false;
+	}
+
+	// Make it executable
+	chmod ("755", tempFile.getPath()); //$NON-NLS-1$
+
+	// "Publish" file now that it's ready to use.
+	// If there is a file already, then someone published while we were
+	// extracting, just delete our file and consider it a success.
+	try {
+		Files.move (tempFile.toPath(), file.toPath());
+	} catch (Throwable e) {
+		tempFile.delete();
+	}
+
+	return true;
 }
 
 static boolean isLoadable () {
@@ -315,11 +331,11 @@ public static void loadLibrary (String name, boolean mapName) {
 
 	/* Try extracting and loading library from jar. */
 	if (path != null) {
-		if (extract (path + SEPARATOR + fileName1, mappedName1, message)) {
+		if (extract (path + SEPARATOR + fileName1, mappedName1)) {
 			load(path + SEPARATOR + fileName1, message);
 			return;
 		}
-		if (mapName && extract (path + SEPARATOR + fileName2, mappedName2, message)) {
+		if (mapName && extract (path + SEPARATOR + fileName2, mappedName2)) {
 			load(path + SEPARATOR + fileName2, message);
 			return;
 		}
@@ -448,7 +464,7 @@ public static File findResource(String subDir, String resourceName, boolean mapR
 			}
 
 			StringBuilder message = new StringBuilder("");
-			if (extract(file.getPath(), maybeSubDirPath + finalResourceName, message)) {
+			if (extract(file.getPath(), maybeSubDirPath + finalResourceName)) {
 				if (file.exists()) {
 					return file;
 				}
diff --git a/tests/org.eclipse.swt.tests/ManualTests/org/eclipse/swt/tests/manual/Bug547109_JvmCrashNativeLibraryLoad.java b/tests/org.eclipse.swt.tests/ManualTests/org/eclipse/swt/tests/manual/Bug547109_JvmCrashNativeLibraryLoad.java
new file mode 100644
index 0000000000..3147ec0d44
--- /dev/null
+++ b/tests/org.eclipse.swt.tests/ManualTests/org/eclipse/swt/tests/manual/Bug547109_JvmCrashNativeLibraryLoad.java
@@ -0,0 +1,140 @@
+/*******************************************************************************
+ * Copyright (c) 2019 Syntevo and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *     Syntevo - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.swt.tests.manual;
+
+import org.eclipse.swt.*;
+import org.eclipse.swt.internal.*;
+import org.eclipse.swt.layout.*;
+import org.eclipse.swt.widgets.*;
+
+import java.io.*;
+import java.util.*;
+
+public class Bug547109_JvmCrashNativeLibraryLoad {
+	private static final String ARG_NO_UI = "-noUI";
+	private static final int EXIT_CODE_GOOD = 0;
+
+	private static void createEmptyFolder(File folder) {
+		if (!folder.exists()) {
+			folder.mkdir();
+		}
+		else {
+			File[] files = folder.listFiles();
+			for (int i = 0; i < files.length; i++) {
+				files[i].delete();
+			}
+		}
+	}
+
+	private static void test(Shell shell) {
+		// Create a folder to hold the native libraries
+		// Delete any libraries if already present
+		File libsFolder = new File("nativeLibs");
+		createEmptyFolder(libsFolder);
+
+		boolean isMacOS = Platform.PLATFORM.equalsIgnoreCase("cocoa");
+
+		// Run processes
+		ProcessBuilder processBuilder = new ProcessBuilder(
+				System.getProperty("java.home") + "/bin/java",
+				"-Dswt.library.path=" + libsFolder.getAbsolutePath(),
+				"-classpath", System.getProperty("java.class.path"),
+				isMacOS ? "-XstartOnFirstThread" : "",
+				Bug547109_JvmCrashNativeLibraryLoad.class.getName(),
+				ARG_NO_UI
+		);
+		processBuilder.inheritIO();
+
+		int numErrors = 0;
+		try {
+			Process[] processes = new Process[15];
+
+			// Run many processes at once
+			for (int i = 0; i < processes.length; i++) {
+				processes[i] = processBuilder.start();
+			}
+
+			// Test for errors
+			for (int i = 0; i < processes.length; i++) {
+				int exitCode = processes[i].waitFor();
+				if (EXIT_CODE_GOOD != exitCode) numErrors++;
+			}
+		} catch (Throwable ex) {
+			ex.printStackTrace();
+		}
+
+		// Report
+		String message = String.format("%d crashes/errors detected", numErrors);
+		MessageBox dialog = new MessageBox(shell, SWT.ICON_INFORMATION);
+		dialog.setMessage(message);
+		dialog.open();
+	}
+
+	private static boolean isSwtJar() {
+		// Code from Library.loadLibrary
+		String libName = "swt-" + Platform.PLATFORM + "-" + Library.getVersionString();
+		// Code from Library.mapLibraryName
+		libName = System.mapLibraryName (libName);
+		String ext = ".dylib"; //$NON-NLS-1$
+		if (libName.endsWith(ext)) {
+			libName = libName.substring(0, libName.length() - ext.length()) + ".jnilib"; //$NON-NLS-1$
+		}
+
+		// Is it available?
+		try (InputStream inputStream = Display.class.getResourceAsStream("/" + libName))
+		{
+			return (inputStream != null);
+		} catch (Throwable e) {
+			return false;
+		}
+	}
+
+	public static void main(String[] args) throws Exception {
+		final Display display = new Display();
+
+		if (Arrays.asList(args).contains(ARG_NO_UI)) {
+			System.exit(EXIT_CODE_GOOD);
+		}
+
+		final Shell shell = new Shell(display, SWT.SHELL_TRIM);
+		shell.setSize(300, 200);
+
+		FillLayout layout = new FillLayout();
+		layout.marginWidth = 30;
+		layout.marginHeight = 30;
+		shell.setLayout(layout);
+
+		final Button button = new Button(shell, SWT.PUSH | SWT.WRAP);
+
+		if (isSwtJar()) {
+			button.setText("Click me && wait for msgbox\n\nIf you have crashes/errors, bug exists. Check console to see detected problems.");
+			button.addListener(SWT.Selection, event -> {
+				test(shell);
+			});
+		} else {
+			// Only swt.jar has the libraries
+			button.setText("Native library resource not found. Add it to classpath or run with swt.jar");
+		}
+
+		shell.open();
+
+		while (!shell.isDisposed()) {
+			if (!display.readAndDispatch()) {
+				display.sleep();
+			}
+		}
+
+		display.dispose();
+	}
+}