From 500a1e6ff4ec1e0aa82cecb3d18f89ca33ca49e6 Mon Sep 17 00:00:00 2001
From: Marcel Hellkamp <marc@gsites.de>
Date: Tue, 27 Jul 2021 19:39:34 +0200
Subject: [PATCH] fix(common-web): Sub-processes hang if output buffers fill up
ExternalProcessExecutor spawned sub-processes with stderr and
stdout buffers, but did not read from those buffers. Sub-processes
producing output sometimes would fill up these buffers and then hang
until the timeout occurs and the process is killed.
This fixes #12839 (PDF conversion hangs for 10 seconds per page)
by properly discarding command output. The patch also deprecates an
outdated and dangerous API and offers a safer alternative. Splitting
command arguments based on whitespace may result in unwanted behavior
if an argument (e.g. a filename) contains whitespace.
---
.../imp/ExternalProcessExecutor.java | 100 ++++++++++--------
1 file changed, 55 insertions(+), 45 deletions(-)
diff --git a/bbb-common-web/src/main/java/org/bigbluebutton/presentation/imp/ExternalProcessExecutor.java b/bbb-common-web/src/main/java/org/bigbluebutton/presentation/imp/ExternalProcessExecutor.java
index c92943eb4d..d583f1eaad 100755
--- a/bbb-common-web/src/main/java/org/bigbluebutton/presentation/imp/ExternalProcessExecutor.java
+++ b/bbb-common-web/src/main/java/org/bigbluebutton/presentation/imp/ExternalProcessExecutor.java
@@ -19,66 +19,76 @@
package org.bigbluebutton.presentation.imp;
-import java.util.Timer;
-import java.util.TimerTask;
+import java.io.File;
+import java.io.IOException;
+import java.time.Duration;
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
- * A wrapper class the executes an external command.
- *
- * See http://kylecartmell.com/?p=9
+ * A wrapper class the executes an external command.
*
* @author Richard Alam
- *
+ * @author Marcel Hellkamp
*/
public class ExternalProcessExecutor {
private static Logger log = LoggerFactory.getLogger(ExternalProcessExecutor.class);
-
- public boolean exec(String COMMAND, long timeoutMillis) {
- Timer timer = new Timer(false);
- Process p = null;
- try {
- InterruptTimerTask interrupter = new InterruptTimerTask(Thread.currentThread());
- timer.schedule(interrupter, timeoutMillis);
- p = Runtime.getRuntime().exec(COMMAND);
- int result = p.waitFor();
- if (result == 0) {
- return true;
- } else {
- return false;
- }
+ // Replace with ProcessBuilder.Redirect.DISCARD in java 9+
+ private static File DISCARD = new File(
+ System.getProperty("os.name").startsWith("Windows") ? "NUL" : "/dev/null");
- } catch(Exception e) {
- log.info("TIMEDOUT excuting : {}", COMMAND);
- if (p != null) {
- p.destroy();
- }
- } finally {
- timer.cancel(); // If the process returns within the timeout period, we have to stop the interrupter
- // so that it does not unexpectedly interrupt some other code later.
-
- Thread.interrupted(); // We need to clear the interrupt flag on the current thread just in case
- // interrupter executed after waitFor had already returned but before timer.cancel
- // took effect.
- //
- // Oh, and there's also Sun bug 6420270 to worry about here.
- }
- return false;
+ /**
+ * Run COMMAND for at most timeoutMillis while ignoring any output.
+ *
+ * @deprecated The COMMAND string is split on whitespace to create an argument
+ * list. This won't work for arguments that contain whitespace. Use
+ * {@link #exec(List, Duration)} instead.
+ *
+ * @param COMMAND A single command or whitespace separated list of
+ * arguments.
+ * @param timeoutMillis Timeout in milliseconds.
+ * @return true if the command terminated in time with an exit value of 0.
+ */
+ @Deprecated
+ public boolean exec(String COMMAND, long timeoutMillis) {
+ return exec(Arrays.asList(COMMAND.split("[ \\t\\n\\r\\f]+")), Duration.ofMillis(timeoutMillis));
}
-
- class InterruptTimerTask extends TimerTask {
- private Thread thread;
+ /**
+ * Run a command for a limited amount of time while ignoring any output.
+ *
+ * @param cmd List containing the program and its arguments.
+ * @param timeout Maximum execution time.
+ * @return true if the command terminated in time with an exit value of 0.
+ */
+ public boolean exec(List<String> cmd, Duration timeout) {
- public InterruptTimerTask(Thread t) {
- this.thread = t;
- }
+ ProcessBuilder pb = new ProcessBuilder(cmd);
+ pb.redirectError(DISCARD);
+ pb.redirectOutput(DISCARD);
- public void run() {
- thread.interrupt();
- }
+ Process proc;
+ try {
+ proc = pb.start();
+ } catch (IOException e) {
+ log.error("Failed to execute: {}", String.join(" ", cmd), e);
+ return false;
+ }
+ try {
+ if (!proc.waitFor(timeout.toMillis(), TimeUnit.MILLISECONDS)) {
+ log.warn("TIMEDOUT excuting: {}", String.join(" ", cmd));
+ proc.destroy();
+ }
+ return !proc.isAlive() && proc.exitValue() == 0;
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ proc.destroy();
+ return false;
+ }
}
}
--
GitLab