From 91e87b9691630d27249f186ece2ffcae823a2bf8 Mon Sep 17 00:00:00 2001 From: Arnaud Roques Date: Sun, 14 Apr 2024 11:29:32 +0200 Subject: [PATCH] refactor: remove dead code --- src/net/sourceforge/plantuml/OptionFlags.java | 12 +++---- .../plantuml/dot/AbstractGraphviz.java | 23 ++++++------ .../plantuml/dot/ProcessRunner.java | 35 ++++++++++++------- 3 files changed, 39 insertions(+), 31 deletions(-) diff --git a/src/net/sourceforge/plantuml/OptionFlags.java b/src/net/sourceforge/plantuml/OptionFlags.java index bd0e912f4..e8fe6859f 100644 --- a/src/net/sourceforge/plantuml/OptionFlags.java +++ b/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() { - return checkDotError; - } +// public final boolean isCheckDotError() { +// return checkDotError; +// } public final void setCheckDotError(boolean checkDotError) { - this.checkDotError = checkDotError; +// this.checkDotError = checkDotError; } private final AtomicBoolean logDataInitized = new AtomicBoolean(false); diff --git a/src/net/sourceforge/plantuml/dot/AbstractGraphviz.java b/src/net/sourceforge/plantuml/dot/AbstractGraphviz.java index 43fd96541..b602c7c58 100644 --- a/src/net/sourceforge/plantuml/dot/AbstractGraphviz.java +++ b/src/net/sourceforge/plantuml/dot/AbstractGraphviz.java @@ -121,18 +121,17 @@ abstract class AbstractGraphviz implements Graphviz { } finally { Log.info("Ending Graphviz process"); } - if (OptionFlags.getInstance().isCheckDotError() && p != null && p.getError().length() > 0) { - Log.error("GraphViz error stream : " + p.getError()); - if (OptionFlags.getInstance().isCheckDotError()) - throw new IllegalStateException("Dot error " + p.getError()); - - } - if (OptionFlags.getInstance().isCheckDotError() && p != null && p.getOut().length() > 0) { - Log.error("GraphViz out stream : " + p.getOut()); - if (OptionFlags.getInstance().isCheckDotError()) - throw new IllegalStateException("Dot out " + p.getOut()); - - } +// if (OptionFlags.getInstance().isCheckDotError() && p != null && p.getError().length() > 0) { +// Log.error("GraphViz error stream : " + p.getError()); +// if (OptionFlags.getInstance().isCheckDotError()) +// throw new IllegalStateException("Dot error " + p.getError()); +// +// } +// if (OptionFlags.getInstance().isCheckDotError() && p != null && p.getOut().length() > 0) { +// Log.error("GraphViz out stream : " + p.getOut()); +// if (OptionFlags.getInstance().isCheckDotError()) +// throw new IllegalStateException("Dot out " + p.getOut()); +// } return state; } diff --git a/src/net/sourceforge/plantuml/dot/ProcessRunner.java b/src/net/sourceforge/plantuml/dot/ProcessRunner.java index be1eb195d..e64d267c9 100644 --- a/src/net/sourceforge/plantuml/dot/ProcessRunner.java +++ b/src/net/sourceforge/plantuml/dot/ProcessRunner.java @@ -36,7 +36,6 @@ 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; @@ -49,8 +48,8 @@ public class ProcessRunner { // ::remove file when __CORE__ private final String[] cmd; - private String error = ""; - private String out = ""; + private String error; + private String out; public ProcessRunner(String[] cmd) { this.cmd = cmd; @@ -61,14 +60,14 @@ public class ProcessRunner { } public ProcessState run(byte[] in, OutputStream redirection, SFile dir) { + Process process = null; try { final ProcessBuilder builder = new ProcessBuilder(cmd); if (dir != null) builder.directory(dir.conv()); - builder.redirectErrorStream(true); - final Process process = builder.start(); + process = builder.start(); // Handling input to the process if (in != null) @@ -96,20 +95,30 @@ public class ProcessRunner { return ProcessState.TERMINATED_OK(); } - // Process did not finish in time, kill it - process.destroy(); - this.error = "Timeout - kill"; - if (process.waitFor(500, TimeUnit.MILLISECONDS) == false) { - process.destroyForcibly(); - this.error = "Timeout - kill force"; - } - 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()) { + // Process did not finish in time, kill it + process.destroy(); + // Not really sure that we should overwrite "this.error" here + this.error = "Timeout - kill"; + 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(); + } + + } + } }