gradle.properties

@@ -1,4 +1,4 @@
 # Warning, "version" should be the same in gradle.properties and Version.java
 # Any idea anyone how to magically synchronize those :-) ?
-version = 1.2024.5beta1
+version = 1.2024.4
 org.gradle.workers.max = 3

src/net/sourceforge/plantuml/OptionFlags.java

@@ -113,7 +113,7 @@ public class OptionFlags {
 		systemExit = exit;
 		gui = false;
 		quiet = false;
-		// checkDotError = false;
+		checkDotError = false;
 		printFonts = false;
 		// failOnError = false;
 		encodesprite = false;
@@ -130,7 +130,7 @@ public class OptionFlags {
 	private boolean systemExit;
 	private boolean gui;
 	private boolean quiet;
-	// private boolean checkDotError;
+	private boolean checkDotError;
 	private boolean printFonts;
 	private boolean encodesprite;
 	private boolean dumpHtmlStats;
@@ -195,12 +195,12 @@ public class OptionFlags {
 		this.quiet = quiet;
 	}
 
-//	public final boolean isCheckDotError() {
+	public final boolean isCheckDotError() {
-//		return checkDotError;
+		return checkDotError;
-//	}
+	}
 
 	public final void setCheckDotError(boolean checkDotError) {
-//		this.checkDotError = checkDotError;
+		this.checkDotError = checkDotError;
 	}
 
 	private final AtomicBoolean logDataInitized = new AtomicBoolean(false);

src/net/sourceforge/plantuml/dot/AbstractGraphviz.java

@@ -61,13 +61,14 @@ abstract class AbstractGraphviz implements Graphviz {
 
 	private static String findExecutableOnPath(String name) {
 		final String path = System.getenv("PATH");
-		if (path != null) 
+		if (path != null) {
 			for (String dirname : path.split(SFile.pathSeparator)) {
 				final File file = new File(dirname, name);
-				if (file.isFile() && file.canExecute())
+				if (file.isFile() && file.canExecute()) {
 					return file.getAbsolutePath();
+				}
 			}
-		
+		}
 		return null;
 	}
 
@@ -83,12 +84,12 @@ abstract class AbstractGraphviz implements Graphviz {
 
 	final protected File searchDotExe() {
 		String getenv = GraphvizUtils.getenvGraphvizDot();
-		if (findExecutableOnPath() && getenv == null)
+		if (findExecutableOnPath() && getenv == null) {
 			getenv = findExecutableOnPath(getExeName());
-
+		}
-		if (getenv == null)
+		if (getenv == null) {
 			return specificDotExe();
-
+		}
 		return new File(getenv);
 	}
 
@@ -99,9 +100,11 @@ abstract class AbstractGraphviz implements Graphviz {
 	final public ProcessState createFile3(OutputStream os) {
 		Objects.requireNonNull(dotString);
 
-		if (getExeState() != ExeState.OK)
+		if (getExeState() != ExeState.OK) {
+			// createPngNoGraphviz(os, new
+			// FileFormatOption(FileFormat.valueOf(type[0].goUpperCase())));
 			throw new IllegalStateException();
-
+		}
 		final String cmd[] = getCommandLine();
 		ProcessRunner p = null;
 		ProcessState state = null;
@@ -110,6 +113,9 @@ abstract class AbstractGraphviz implements Graphviz {
 			Log.info("DotString size: " + dotString.length());
 			p = new ProcessRunner(cmd);
 			state = p.run(dotString.getBytes(), os);
+			// if (state == ProcessState.TERMINATED_OK) {
+			// result = true;
+			// }
 			Log.info("Ending process ok");
 		} catch (Throwable e) {
 			Logme.error(e);
@@ -121,17 +127,18 @@ abstract class AbstractGraphviz implements Graphviz {
 		} finally {
 			Log.info("Ending Graphviz process");
 		}
-//		if (OptionFlags.getInstance().isCheckDotError() && p != null && p.getError().length() > 0) {
+		if (OptionFlags.getInstance().isCheckDotError() && p != null && p.getError().length() > 0) {
-//			Log.error("GraphViz error stream : " + p.getError());
+			Log.error("GraphViz error stream : " + p.getError());
-//			if (OptionFlags.getInstance().isCheckDotError())
+			if (OptionFlags.getInstance().isCheckDotError()) {
-//				throw new IllegalStateException("Dot error " + p.getError());
+				throw new IllegalStateException("Dot error " + p.getError());
-//
+			}
-//		}
+		}
-//		if (OptionFlags.getInstance().isCheckDotError() && p != null && p.getOut().length() > 0) {
+		if (OptionFlags.getInstance().isCheckDotError() && p != null && p.getOut().length() > 0) {
-//			Log.error("GraphViz out stream : " + p.getOut());
+			Log.error("GraphViz out stream : " + p.getOut());
-//			if (OptionFlags.getInstance().isCheckDotError())
+			if (OptionFlags.getInstance().isCheckDotError()) {
-//				throw new IllegalStateException("Dot out " + p.getOut());
+				throw new IllegalStateException("Dot out " + p.getOut());
-//		}
+			}
+		}
 		return state;
 	}
 
@@ -147,17 +154,17 @@ abstract class AbstractGraphviz implements Graphviz {
 	private String executeCmd(final String cmd[]) {
 		final ProcessRunner p = new ProcessRunner(cmd);
 		final ProcessState state = p.run(null, null);
-		if (state.differs(ProcessState.TERMINATED_OK()))
+		if (state.differs(ProcessState.TERMINATED_OK())) {
 			return "?";
-
+		}
 		final StringBuilder sb = new StringBuilder();
-		if (StringUtils.isNotEmpty(p.getOut()))
+		if (StringUtils.isNotEmpty(p.getOut())) {
 			sb.append(p.getOut());
-
+		}
 		if (StringUtils.isNotEmpty(p.getError())) {
-			if (sb.length() > 0)
+			if (sb.length() > 0) {
 				sb.append(' ');
-
+			}
 			sb.append(p.getError());
 		}
 		return StringUtils.trin(sb.toString().replace('\n', ' '));
@@ -170,16 +177,16 @@ abstract class AbstractGraphviz implements Graphviz {
 			result[1] = "-n";
 			result[2] = "10";
 			result[3] = getDotExe().getAbsolutePath();
-			for (int i = 0; i < type.length; i++)
+			for (int i = 0; i < type.length; i++) {
 				result[i + 4] = "-T" + type[i];
-
+			}
 			return result;
 		}
 		final String[] result = new String[type.length + 1];
 		result[0] = getDotExe().getAbsolutePath();
-		for (int i = 0; i < type.length; i++)
+		for (int i = 0; i < type.length; i++) {
 			result[i + 1] = "-T" + type[i];
-
+		}
 		return result;
 	}
 

src/net/sourceforge/plantuml/dot/ProcessRunner.java

@@ -35,12 +35,15 @@
  */
 package net.sourceforge.plantuml.dot;
 
-import java.io.ByteArrayOutputStream;
+import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
-import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
 
 import net.sourceforge.plantuml.OptionFlags;
+import net.sourceforge.plantuml.api.MyRunnable;
+import net.sourceforge.plantuml.api.TimeoutExecutor;
 import net.sourceforge.plantuml.log.Logme;
 import net.sourceforge.plantuml.security.SFile;
 
@@ -48,78 +51,210 @@ public class ProcessRunner {
 	// ::remove file when __CORE__
 
 	private final String[] cmd;
+
 	private String error;
 	private String out;
 
+	private volatile ProcessState state = ProcessState.INIT();
+	private final Lock changeState = new ReentrantLock();
+
 	public ProcessRunner(String[] cmd) {
 		this.cmd = cmd;
 	}
 
-	public ProcessState run(byte[] in, OutputStream redirection) {
+	public ProcessState run(byte in[], OutputStream redirection) {
 		return run(in, redirection, null);
 	}
 
-	public ProcessState run(byte[] in, OutputStream redirection, SFile dir) {
+	public ProcessState run(byte in[], OutputStream redirection, SFile dir) {
-		Process process = null;
+		if (this.state.differs(ProcessState.INIT())) {
+			throw new IllegalStateException();
+		}
+		this.state = ProcessState.RUNNING();
+		final MainThread mainThread = new MainThread(cmd, dir, redirection, in);
 		try {
-			final ProcessBuilder builder = new ProcessBuilder(cmd);
+			// http://steveliles.github.io/invoking_processes_from_java.html
-			if (dir != null)
-				builder.directory(dir.conv());
-			builder.redirectErrorStream(true);
-
-			process = builder.start();
-
-			// Handling input to the process
-			if (in != null)
-				try (OutputStream os = process.getOutputStream()) {
-					os.write(in);
-				}
-
-			final ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
-			try (InputStream is = process.getInputStream()) {
-				final byte[] buffer = new byte[1024];
-				int length;
-				while ((length = is.read(buffer)) != -1) {
-					outputStream.write(buffer, 0, length);
-					if (redirection != null)
-						redirection.write(buffer, 0, length);
-				}
-			}
-
-			// Wait for process to terminate
 			final long timeoutMs = OptionFlags.getInstance().getTimeoutMs();
-			final boolean finished = process.waitFor(timeoutMs, TimeUnit.MILLISECONDS);
+			final boolean done = new TimeoutExecutor(timeoutMs).executeNow(mainThread);
-			outputStream.close();
-			if (finished) {
-				this.out = new String(outputStream.toByteArray(), "UTF-8");
-				return ProcessState.TERMINATED_OK();
-			}
-
-			return ProcessState.TIMEOUT();
-
-		} catch (Throwable e) {
-			Logme.error(e);
-			this.error = e.toString();
-			return ProcessState.EXCEPTION(e);
 		} finally {
-			if (process != null && out == null && process.isAlive()) {
+			changeState.lock();
-				// Process did not finish in time, kill it
+			try {
-				process.destroy();
+				if (state.equals(ProcessState.RUNNING())) {
-				// Not really sure that we should overwrite "this.error" here
+					state = ProcessState.TIMEOUT();
-				this.error = "Timeout - kill";
+					// mainThread.cancel();
-				try {
-					if (process.waitFor(500, TimeUnit.MILLISECONDS) == false) {
-						process.destroyForcibly();
-						this.error = "Timeout - kill force";
-					}
-				} catch (InterruptedException e) {
-					// Nothing we can really do
-					e.printStackTrace();
 				}
+			} finally {
+				changeState.unlock();
+			}
+		}
+		if (state.equals(ProcessState.TERMINATED_OK())) {
+			assert mainThread != null;
+			this.error = mainThread.getError();
+			this.out = mainThread.getOut();
+		}
+		return state;
+	}
 
+	class MainThread implements MyRunnable {
+
+		private final String[] cmd;
+		private final SFile dir;
+		private final OutputStream redirection;
+		private final byte[] in;
+		private volatile Process process;
+		private volatile ThreadStream errorStream;
+		private volatile ThreadStream outStream;
+
+		public MainThread(String[] cmd, SFile dir, OutputStream redirection, byte[] in) {
+			this.cmd = cmd;
+			this.dir = dir;
+			this.redirection = redirection;
+			this.in = in;
+		}
+
+		public String getOut() {
+			return outStream.getString();
+		}
+
+		public String getError() {
+			return errorStream.getString();
+		}
+
+		public void runJob() throws InterruptedException {
+			try {
+				startThreads();
+				if (state.equals(ProcessState.RUNNING())) {
+					final int result = joinInternal();
+				}
+			} finally {
+				changeState.lock();
+				try {
+					if (state.equals(ProcessState.RUNNING())) {
+						state = ProcessState.TERMINATED_OK();
+					}
+				} finally {
+					changeState.unlock();
+				}
+				if (process != null) {
+					process.destroy();
+					close(process.getErrorStream());
+					close(process.getOutputStream());
+					close(process.getInputStream());
+				}
 			}
 
 		}
+
+		public void cancelJob() {
+			// The changeState lock is ok
+			// assert changeState.tryLock();
+			// assert state == ProcessState.TIMEOUT;
+			if (process != null) {
+				errorStream.cancel();
+				outStream.cancel();
+				process.destroy();
+				// interrupt();
+				close(process.getErrorStream());
+				close(process.getOutputStream());
+				close(process.getInputStream());
+			}
+		}
+
+		private void startThreads() {
+			try {
+				process = Runtime.getRuntime().exec(cmd, null, dir == null ? null : dir.conv());
+			} catch (IOException e) {
+				Logme.error(e);
+				changeState.lock();
+				try {
+					state = ProcessState.IO_EXCEPTION1(e);
+				} finally {
+					changeState.unlock();
+				}
+				Logme.error(e);
+				return;
+			}
+			errorStream = new ThreadStream(process.getErrorStream(), null);
+			outStream = new ThreadStream(process.getInputStream(), redirection);
+			errorStream.start();
+			outStream.start();
+			if (in != null) {
+				final OutputStream os = process.getOutputStream();
+				try {
+					try {
+						os.write(in);
+					} finally {
+						os.close();
+					}
+				} catch (IOException e) {
+					changeState.lock();
+					try {
+						state = ProcessState.IO_EXCEPTION2(e);
+					} finally {
+						changeState.unlock();
+					}
+					Logme.error(e);
+				}
+			}
+		}
+
+		public int joinInternal() throws InterruptedException {
+			errorStream.join();
+			outStream.join();
+			final int result = process.waitFor();
+			return result;
+		}
+
+	}
+
+	class ThreadStream extends Thread {
+
+		private volatile InputStream streamToRead;
+		private volatile OutputStream redirection;
+		private volatile StringBuffer sb = new StringBuffer();
+
+		ThreadStream(InputStream streamToRead, OutputStream redirection) {
+			this.streamToRead = streamToRead;
+			this.redirection = redirection;
+		}
+
+		public String getString() {
+			if (sb == null) {
+				return "";
+			}
+			return sb.toString();
+		}
+
+		public void cancel() {
+			assert state.equals(ProcessState.TIMEOUT()) || state.equals(ProcessState.RUNNING()) : state;
+			this.interrupt();
+			sb = null;
+			streamToRead = null;
+			redirection = null;
+			// Because of this, some NPE may occurs in run() method, but we do not care
+		}
+
+		@Override
+		public void run() {
+			int read = 0;
+			try {
+				while ((read = streamToRead.read()) != -1) {
+					if (state.equals(ProcessState.TIMEOUT())) {
+						return;
+					}
+					if (redirection == null) {
+						sb.append((char) read);
+					} else {
+						redirection.write(read);
+					}
+				}
+			} catch (Throwable e) {
+				System.err.println("ProcessRunnerA " + e);
+				Logme.error(e);
+				sb.append('\n');
+				sb.append(e.toString());
+			}
+		}
 	}
 
 	public final String getError() {
@@ -130,4 +265,24 @@ public class ProcessRunner {
 		return out;
 	}
 
+	private void close(InputStream is) {
+		try {
+			if (is != null) {
+				is.close();
+			}
+		} catch (IOException e) {
+			Logme.error(e);
+		}
+	}
+
+	private void close(OutputStream os) {
+		try {
+			if (os != null) {
+				os.close();
+			}
+		} catch (IOException e) {
+			Logme.error(e);
+		}
+	}
+
 }

src/net/sourceforge/plantuml/dot/ProcessState.java

@@ -35,28 +35,42 @@
  */
 package net.sourceforge.plantuml.dot;
 
+import java.io.IOException;
+
 public class ProcessState {
 	// ::remove file when __CORE__
 
 	private final String name;
-	private final Throwable cause;
+	private final IOException cause;
 
-	private ProcessState(String name, Throwable cause) {
+	private ProcessState(String name, IOException cause) {
 		this.name = name;
 		this.cause = cause;
 	}
 
 	@Override
 	public String toString() {
-		if (cause == null)
+		if (cause == null) {
 			return name;
-
+		}
 		return name + " " + cause.toString();
 	}
 
+	private final static ProcessState INIT = new ProcessState("INIT", null);
+	private final static ProcessState RUNNING = new ProcessState("RUNNING", null);
 	private final static ProcessState TERMINATED_OK = new ProcessState("TERMINATED_OK", null);
 	private final static ProcessState TIMEOUT = new ProcessState("TIMEOUT", null);
 
+	// INIT, RUNNING, TERMINATED_OK, TIMEOUT, IO_EXCEPTION1, IO_EXCEPTION2;
+
+	public static ProcessState INIT() {
+		return INIT;
+	}
+
+	public static ProcessState RUNNING() {
+		return RUNNING;
+	}
+
 	public static ProcessState TERMINATED_OK() {
 		return TERMINATED_OK;
 	}
@@ -65,8 +79,12 @@ public class ProcessState {
 		return TIMEOUT;
 	}
 
-	public static ProcessState EXCEPTION(Throwable e) {
+	public static ProcessState IO_EXCEPTION1(IOException e) {
-		return new ProcessState("EXCEPTION", e);
+		return new ProcessState("IO_EXCEPTION1", e);
+	}
+
+	public static ProcessState IO_EXCEPTION2(IOException e) {
+		return new ProcessState("IO_EXCEPTION2", e);
 	}
 
 	public boolean differs(ProcessState other) {

src/net/sourceforge/plantuml/version/Version.java

@@ -46,7 +46,7 @@ public class Version {
 
 	// Warning, "version" should be the same in gradle.properties and Version.java
 	// Any idea anyone how to magically synchronize those :-) ?
-	private static final String version = "1.2024.5beta1";
+	private static final String version = "1.2024.4";
 
 	public static String versionString() {
 		return version;