Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Process management: cleanup

  • Loading branch information...
commit e0397006650887ddb82c5b657b6a9ea2ec433315 1 parent b34bce7
@chocolateboy chocolateboy authored
View
2  src/main/java/net/pms/PMS.java
@@ -194,7 +194,7 @@ public SystemUtils getRegistry() {
}
/**Executes a new Process and creates a fork that waits for its results.
- * TODO:Extend explanation on where this is being used.
+ * TODO Extend explanation on where this is being used.
* @param name Symbolic name for the process to be launched, only used in the trace log
* @param error (boolean) Set to true if you want PMS to add error messages to the trace pane
* @param workDir (File) optional working directory to run the process in
View
11 src/main/java/net/pms/dlna/DLNAResource.java
@@ -1780,13 +1780,16 @@ public InputStream getInputStream(Range range, RendererConfiguration mediarender
if (externalProcess == null || externalProcess.isDestroyed()) {
LOGGER.info("Starting transcode/remux of " + getName());
+
externalProcess = getPlayer().launchTranscode(
getSystemName(),
this,
getMedia(),
- params);
+ params
+ );
+
if (params.waitbeforestart > 0) {
- LOGGER.trace("Sleeping for " + params.waitbeforestart + " milliseconds");
+ LOGGER.trace("Sleeping for {} milliseconds", params.waitbeforestart);
try {
Thread.sleep(params.waitbeforestart);
@@ -1843,8 +1846,7 @@ public void run() {
LOGGER.trace("External input stream instance is null... sounds not good, waiting 500ms");
try {
Thread.sleep(500);
- } catch (InterruptedException e) {
- }
+ } catch (InterruptedException e) { }
}
}
@@ -1860,6 +1862,7 @@ public void run() {
externalProcess.stopProcess();
}
};
+
new Thread(r, "Hanging External Process Stopper").start();
}
View
6 src/main/java/net/pms/encoders/AviDemuxerInputStream.java
@@ -162,7 +162,7 @@ public void run() {
}
private void parseHeader() throws IOException {
- LOGGER.trace("Parsing AVI Stream");
+ LOGGER.trace("Parsing AVI stream");
String id = getString(stream, 4);
getBytes(stream, 4);
String type = getString(stream, 4);
@@ -288,7 +288,7 @@ private void parseHeader() throws IOException {
try {
command = getString(stream, 4);
} catch (Exception e) {
- LOGGER.trace("Error attendue: " + e.getMessage());
+ LOGGER.trace("Error reading stream: " + e.getMessage());
break;
}
@@ -386,7 +386,7 @@ private final int readBytes(InputStream input, int number) throws IOException {
if (read < number) {
if (read < 0) {
- throw new IOException("End of Stream");
+ throw new IOException("End of stream");
}
for (int i = read; i < number; i++) {
View
16 src/main/java/net/pms/encoders/FFMpegWebVideo.java
@@ -91,6 +91,10 @@ public ProcessWrapper launchTranscode(
// This process wraps the command that creates the named pipe
PipeProcess pipe = new PipeProcess(fifoName);
+ pipe.deleteLater(); // delete the named pipe later; harmless if it isn't created
+ ProcessWrapper mkfifo_process = pipe.getPipeProcess();
+ // start the process as early as possible
+ mkfifo_process.runInNewThread();
params.input_pipes[0] = pipe;
int nThreads = configuration.getNumberOfCpuCores();
@@ -147,23 +151,19 @@ public ProcessWrapper launchTranscode(
// now launch ffmpeg
ProcessWrapperImpl pw = new ProcessWrapperImpl(cmdArray, params);
- ProcessWrapper mkfifo_process = pipe.getPipeProcess();
- pw.attachProcess(mkfifo_process);
-
- // create the named pipe and wait briefly to allow it to be created
- mkfifo_process.runInNewThread();
+ pw.attachProcess(mkfifo_process); // clean up the mkfifo process when the transcode ends
+ // give the mkfifo process a little time
try {
Thread.sleep(200);
} catch (InterruptedException e) {
LOGGER.error("Thread interrupted while waiting for named pipe to be created", e);
}
- pipe.deleteLater();
-
- // launch transcode command and wait briefly to allow it to start
+ // launch the transcode command...
pw.runInNewThread();
+ // ... and wait briefly to allow it to start
try {
Thread.sleep(200);
} catch (InterruptedException e) {
View
2  src/main/java/net/pms/io/BlockerFileInputStream.java
@@ -25,6 +25,8 @@
import java.io.FileInputStream;
import java.io.IOException;
+@Deprecated
+// no longer used
public class BlockerFileInputStream extends UnusedInputStream {
private static final Logger logger = LoggerFactory.getLogger(BlockerFileInputStream.class);
private static final int CHECK_INTERVAL = 1000;
View
49 src/main/java/net/pms/io/MacSystemUtils.java
@@ -1,8 +1,11 @@
package net.pms.io;
+import org.apache.commons.io.IOUtils;
+
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import java.io.InputStream;
import java.io.IOException;
import java.net.NetworkInterface;
import java.net.SocketException;
@@ -11,23 +14,21 @@
import java.util.regex.Pattern;
public class MacSystemUtils extends BasicSystemUtils {
- private final static Logger logger = LoggerFactory.getLogger(MacSystemUtils.class);
+ private final static Logger LOGGER = LoggerFactory.getLogger(MacSystemUtils.class);
- public MacSystemUtils() {
- }
+ public MacSystemUtils() { }
@Override
public void browseURI(String uri) {
try {
- // On OSX, open the given URI with the "open" command.
+ // On OS X, open the given URI with the "open" command.
// This will open HTTP URLs in the default browser.
Runtime.getRuntime().exec(new String[] { "open", uri });
-
} catch (IOException e) {
- logger.trace("Unable to open the given URI: " + uri + ".");
+ LOGGER.trace("Unable to open the given URI: {}", uri);
}
}
-
+
@Override
public boolean isNetworkInterfaceLoopback(NetworkInterface ni) throws SocketException {
return false;
@@ -35,29 +36,28 @@ public boolean isNetworkInterfaceLoopback(NetworkInterface ni) throws SocketExce
/**
* Fetch the hardware address for a network interface.
- *
- * @param ni Interface to fetch the mac address for
- * @return the mac address as bytes, or null if it couldn't be fetched.
+ *
+ * @param ni Interface to fetch the MAC address for
+ * @return the MAC address as bytes, or null if it couldn't be fetched.
* @throws SocketException
- * This won't happen on Mac OS, since the NetworkInterface is
+ * This won't happen on OS X, since the NetworkInterface is
* only used to get a name.
*/
@Override
public byte[] getHardwareAddress(NetworkInterface ni) throws SocketException {
- // On Mac OS, fetch the hardware address from the command line tool "ifconfig".
+ // On Mac OS X, fetch the hardware address from the command line tool "ifconfig".
byte[] aHardwareAddress = null;
+ InputStream inputStream = null;
try {
- Process aProc = Runtime.getRuntime().exec(new String[] { "ifconfig", ni.getName(), "ether" });
- aProc.waitFor();
- OutputTextConsumer aConsumer = new OutputTextConsumer(aProc.getInputStream(), false);
- aConsumer.run();
- List<String> aLines = aConsumer.getResults();
+ Process process = Runtime.getRuntime().exec(new String[] { "ifconfig", ni.getName(), "ether" });
+ inputStream = process.getInputStream();
+ List<String> lines = IOUtils.readLines(inputStream);
String aMacStr = null;
Pattern aMacPattern = Pattern.compile("\\s*ether\\s*([a-d0-9]{2}:[a-d0-9]{2}:[a-d0-9]{2}:[a-d0-9]{2}:[a-d0-9]{2}:[a-d0-9]{2})");
- for (String aLine : aLines) {
- Matcher aMacMatcher = aMacPattern.matcher(aLine);
+ for (String line : lines) {
+ Matcher aMacMatcher = aMacPattern.matcher(line);
if (aMacMatcher.find()) {
aMacStr = aMacMatcher.group(1);
@@ -75,12 +75,11 @@ public boolean isNetworkInterfaceLoopback(NetworkInterface ni) throws SocketExce
}
}
} catch (IOException e) {
- logger.debug("Failed to execute ifconfig", e);
- } catch (InterruptedException e) {
- logger.debug("Interrupted while waiting for ifconfig", e);
- Thread.interrupted(); // XXX work around a Java bug - see ProcessUtil.waitFor()
+ LOGGER.warn("Failed to execute ifconfig", e);
+ } finally {
+ IOUtils.closeQuietly(inputStream);
}
- return aHardwareAddress;
- }
+ return aHardwareAddress;
+ }
}
View
14 src/main/java/net/pms/io/OutputBufferConsumer.java
@@ -26,7 +26,7 @@
import java.util.List;
public class OutputBufferConsumer extends OutputConsumer {
- private static final Logger logger = LoggerFactory.getLogger(OutputBufferConsumer.class);
+ private static final Logger LOGGER = LoggerFactory.getLogger(OutputBufferConsumer.class);
private BufferedOutputFile outputBuffer;
/**
@@ -49,23 +49,23 @@ public OutputBufferConsumer(InputStream inputStream, OutputParams params) {
public void run() {
try {
- //logger.trace("Starting read from pipe");
+ // LOGGER.trace("Starting read from pipe");
byte buf[] = new byte[PIPE_BUFFER_SIZE];
int n = 0;
while ((n = inputStream.read(buf)) > 0) {
- //logger.trace("Fetched " + n + " from pipe");
+ // LOGGER.trace("Fetched " + n + " from pipe");
outputBuffer.write(buf, 0, n);
}
- //logger.debug("Finished to read");
+ // LOGGER.debug("Finished to read");
} catch (IOException ioe) {
- logger.debug("Error consuming stream of spawned process: " + ioe.getMessage());
+ LOGGER.debug("Error consuming stream of spawned process: " + ioe.getMessage());
} finally {
- //logger.trace("Closing read from pipe");
+ // LOGGER.trace("Closing read from pipe");
if (inputStream != null) {
try {
inputStream.close();
} catch (IOException e) {
- logger.debug("Caught exception", e);
+ LOGGER.debug("Caught exception", e);
}
}
}
View
8 src/main/java/net/pms/io/OutputConsumer.java
@@ -18,6 +18,8 @@
*/
package net.pms.io;
+import org.apache.commons.io.IOUtils;
+
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -35,11 +37,7 @@ public OutputConsumer(InputStream inputStream) {
@Deprecated
public void destroy() {
- try {
- inputStream.close();
- } catch (IOException e) {
- LOGGER.debug("Failed to close stream", e);
- }
+ IOUtils.closeQuietly(inputStream);
}
public abstract BufferedOutputFile getBuffer();
View
3  src/main/java/net/pms/io/OutputParams.java
@@ -83,7 +83,6 @@ public OutputParams(PmsConfiguration configuration) {
}
timeseek = 0;
- outputFile = null;
env = null;
}
@@ -105,7 +104,7 @@ public String toString() {
+ losslessaudio + ", lossyaudio=" + lossyaudio + ", maxBufferSize=" + maxBufferSize
+ ", mediaRenderer=" + mediaRenderer + ", minBufferSize=" + minBufferSize + ", minFileSize="
+ minFileSize + ", no_videoencode=" + no_videoencode + ", noexitcheck=" + noexitcheck
- + ", outputFile=" + outputFile + ", output_pipes=" + Arrays.toString(output_pipes)
+ + ", output_pipes=" + Arrays.toString(output_pipes)
+ ", secondread_minsize=" + secondread_minsize + ", shift_scr=" + shift_scr + ", sid=" + sid
+ ", stdin=" + stdin + ", timeend=" + timeend + ", timeseek=" + timeseek + ", toFrame=" + toFrame
+ ", waitbeforestart=" + waitbeforestart + ", workDir=" + workDir + ", env=" + env + "]";
View
47 src/main/java/net/pms/io/OutputTextConsumer.java
@@ -18,6 +18,9 @@
*/
package net.pms.io;
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.io.LineIterator;
+
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -28,8 +31,11 @@
import java.util.ArrayList;
import java.util.List;
+/**
+ * An input stream consumer that stores the consumed lines in a list and optionally logs each line.
+ */
public class OutputTextConsumer extends OutputConsumer {
- private static final Logger logger = LoggerFactory.getLogger(OutputTextConsumer.class);
+ private static final Logger LOGGER = LoggerFactory.getLogger(OutputTextConsumer.class);
private List<String> lines = new ArrayList<String>();
private Object linesLock = new Object();
private boolean log;
@@ -41,35 +47,26 @@ public OutputTextConsumer(InputStream inputStream, boolean log) {
}
public void run() {
- BufferedReader br = null;
+ LineIterator it = null;
+
try {
- br = new BufferedReader(new InputStreamReader(inputStream, "UTF-8"));
- String line = null;
- int authorized = 10;
- while ((line = br.readLine()) != null) {
- if (line.length() > 0 && line.startsWith("[") && authorized > 0) {
- addLine(line);
- if (log) {
- logger.trace(line);
- }
- authorized--;
- } else if (line.length() > 0 && !line.startsWith("[") && !line.startsWith("100") && !line.startsWith("size") && !line.startsWith("frame") && !line.startsWith("Pos") && !line.startsWith("ERROR:") && !line.startsWith("BUFFER") && !line.startsWith("INITV")) {
+ it = IOUtils.lineIterator(inputStream, "UTF-8");
+
+ while (it.hasNext()) {
+ String line = it.nextLine();
+
+ if (line.length() > 0) {
addLine(line);
- if (log) {
- logger.trace(line);
- }
+ }
+
+ if (log) {
+ LOGGER.debug(line);
}
}
} catch (IOException ioe) {
- logger.debug("Error consuming stream of spawned process: " + ioe.getMessage());
+ LOGGER.debug("Error consuming input stream: " + ioe.getMessage());
} finally {
- if (br != null) {
- try {
- br.close();
- } catch (IOException e) {
- logger.debug("Caught exception", e);
- }
- }
+ LineIterator.closeQuietly(it); // clean up all associated resources
}
}
@@ -85,9 +82,11 @@ public BufferedOutputFile getBuffer() {
public List<String> getResults() {
List<String> clonedResults = new ArrayList<String>();
+
synchronized (linesLock) {
clonedResults.addAll(lines);
}
+
return clonedResults;
}
}
View
25 src/main/java/net/pms/io/OutputTextLogger.java
@@ -18,6 +18,9 @@
*/
package net.pms.io;
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.io.LineIterator;
+
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -31,32 +34,26 @@
* A version of OutputTextConsumer that a) logs all output to the debug.log and b) doesn't store the output
*/
public class OutputTextLogger extends OutputConsumer {
- private static final Logger logger = LoggerFactory.getLogger(OutputTextLogger.class);
+ private static final Logger LOGGER = LoggerFactory.getLogger(OutputTextLogger.class);
public OutputTextLogger(InputStream inputStream) {
super(inputStream);
}
public void run() {
- BufferedReader br = null;
+ LineIterator it = null;
try {
- br = new BufferedReader(new InputStreamReader(inputStream, "UTF-8"));
- String line = null;
+ it = IOUtils.lineIterator(inputStream, "UTF-8");
- while ((line = br.readLine()) != null) {
- logger.debug(line);
+ while (it.hasNext()) {
+ String line = it.nextLine();
+ LOGGER.debug(line);
}
} catch (IOException ioe) {
- logger.debug("Error consuming stream of spawned process: " + ioe.getMessage());
+ LOGGER.debug("Error consuming input stream: " + ioe.getMessage());
} finally {
- if (br != null) {
- try {
- br.close();
- } catch (IOException e) {
- logger.debug("Caught exception", e);
- }
- }
+ LineIterator.closeQuietly(it); // clean up all associated resources
}
}
View
105 src/main/java/net/pms/io/ProcessWrapperImpl.java
@@ -32,10 +32,10 @@
import java.util.Map;
public class ProcessWrapperImpl extends Thread implements ProcessWrapper {
- private static final Logger logger = LoggerFactory.getLogger(ProcessWrapperImpl.class);
+ private static final Logger LOGGER = LoggerFactory.getLogger(ProcessWrapperImpl.class);
private String cmdLine;
private Process process;
- private OutputConsumer outConsumer;
+ private OutputConsumer stdoutConsumer;
private OutputConsumer stderrConsumer;
private OutputParams params;
private boolean destroyed;
@@ -122,11 +122,7 @@ public void run() {
ProcessBuilder pb = new ProcessBuilder(cmdArray);
try {
- logger.debug("Starting " + cmdLine);
-
- if (params.outputFile != null && params.outputFile.getParentFile().isDirectory()) {
- pb.directory(params.outputFile.getParentFile());
- }
+ LOGGER.debug("Starting " + cmdLine);
if (params.workDir != null && params.workDir.isDirectory()) {
pb.directory(params.workDir);
@@ -152,79 +148,98 @@ public void run() {
}
}
+ // TODO fix the callers of this code to use simpler mechanisms to
+ // execute short-running commands (e.g. vlc -version) so
+ // that this class is only used to run long-running tasks i.e.
+ // transcodes. in that case, we won't need separate stdout and stderr
+ // and can merge them by calling:
+ // pb.redirectErrorStream(true);
process = pb.start();
PMS.get().currentProcesses.add(process);
+
stderrConsumer = keepStderr
? new OutputTextConsumer(process.getErrorStream(), true)
: new OutputTextLogger(process.getErrorStream());
stderrConsumer.start();
- outConsumer = null;
+ stdoutConsumer = null;
- if (params.outputFile != null) {
- logger.debug("Writing to " + params.outputFile.getAbsolutePath());
- outConsumer = keepStdout
- ? new OutputTextConsumer(process.getInputStream(), false)
- : new OutputTextLogger(process.getInputStream());
- } else if (params.input_pipes[0] != null) {
- logger.debug("Reading pipe: " + params.input_pipes[0].getInputPipe());
+ if (params.input_pipes[0] != null) {
+ LOGGER.debug("Reading pipe: " + params.input_pipes[0].getInputPipe());
bo = params.input_pipes[0].getDirectBuffer();
if (bo == null || params.losslessaudio || params.lossyaudio || params.no_videoencode) {
InputStream is = params.input_pipes[0].getInputStream();
- outConsumer = new OutputBufferConsumer((params.avidemux) ? new AviDemuxerInputStream(is, params, attachedProcesses) : is, params);
- bo = outConsumer.getBuffer();
+
+ if (params.avidemux) {
+ is = new AviDemuxerInputStream(is, params, attachedProcesses);
+ }
+
+ stdoutConsumer = new OutputBufferConsumer(is, params);
+ bo = stdoutConsumer.getBuffer();
}
bo.attachThread(this);
new OutputTextLogger(process.getInputStream()).start();
} else if (params.log) {
- outConsumer = keepStdout
+ stdoutConsumer = keepStdout
? new OutputTextConsumer(process.getInputStream(), true)
: new OutputTextLogger(process.getInputStream());
} else {
- outConsumer = new OutputBufferConsumer(process.getInputStream(), params);
- bo = outConsumer.getBuffer();
+ stdoutConsumer = new OutputBufferConsumer(process.getInputStream(), params);
+ bo = stdoutConsumer.getBuffer();
bo.attachThread(this);
}
- if (params.stdin != null) {
- params.stdin.push(process.getOutputStream());
+ if (stdoutConsumer != null) {
+ stdoutConsumer.start();
}
- if (outConsumer != null) {
- outConsumer.start();
+ if (params.stdin != null) {
+ params.stdin.push(process.getOutputStream());
}
Integer pid = ProcessUtil.getProcessID(process);
if (pid != null) {
- logger.debug("Unix process ID (" + cmdArray[0] + "): " + pid);
+ LOGGER.debug("Unix process ID ({}): {}", cmdArray[0], pid);
}
ProcessUtil.waitFor(process);
+ // wait up to a second for the stderr consumer thread to finish
try {
- if (outConsumer != null) {
- outConsumer.join(1000);
+ if (stderrConsumer != null) {
+ stderrConsumer.join(1000);
}
} catch (InterruptedException e) { }
- if (bo != null) {
- bo.close();
- }
+ // wait up to a second for the stdout consumer thread to finish
+ try {
+ if (stdoutConsumer != null) {
+ stdoutConsumer.join(1000);
+ }
+ } catch (InterruptedException e) { }
} catch (Exception e) {
- logger.error("Fatal error in process initialization: ", e);
+ LOGGER.error("Error initializing process: ", e);
stopProcess();
} finally {
+ try {
+ if (bo != null) {
+ bo.close();
+ }
+ } catch (IOException ioe) {
+ LOGGER.debug("Error closing buffered output file", ioe.getMessage());
+ }
+
if (!destroyed && !params.noexitcheck) {
try {
success = true;
if (process != null && process.exitValue() != 0) {
- logger.info("Process " + cmdArray[0] + " has a return code of " + process.exitValue() + "! Maybe an error occurred... check the log file");
+ LOGGER.info("Process {} has a return code of {}! Maybe an error occurred... check the log file", cmdArray[0], process.exitValue());
success = false;
}
} catch (IllegalThreadStateException itse) {
- logger.error("An error occurred", itse);
+ LOGGER.error("Error reading process exit value", itse);
}
}
@@ -261,27 +276,23 @@ public void runInSameThread() {
public InputStream getInputStream(long seek) throws IOException {
if (bo != null) {
return bo.getInputStream(seek);
- } else if (outConsumer != null && outConsumer.getBuffer() != null) {
- return outConsumer.getBuffer().getInputStream(seek);
- } else if (params.outputFile != null) {
- BlockerFileInputStream fIn = new BlockerFileInputStream(this, params.outputFile, params.minFileSize);
- fIn.skip(seek);
- return fIn;
+ } else if (stdoutConsumer != null && stdoutConsumer.getBuffer() != null) {
+ return stdoutConsumer.getBuffer().getInputStream(seek);
}
return null;
}
public List<String> getOtherResults() {
- if (outConsumer == null) {
+ if (stdoutConsumer == null) {
return null;
}
try {
- outConsumer.join(1000);
+ stdoutConsumer.join(1000);
} catch (InterruptedException e) { }
- return outConsumer.getResults();
+ return stdoutConsumer.getResults();
}
public List<String> getResults() {
@@ -299,9 +310,9 @@ public synchronized void stopProcess() {
Integer pid = ProcessUtil.getProcessID(process);
if (pid != null) {
- logger.debug("Stopping Unix process " + pid + ": " + this);
+ LOGGER.debug("Stopping Unix process " + pid + ": " + this);
} else {
- logger.debug("Stopping process: " + this);
+ LOGGER.debug("Stopping process: " + this);
}
ProcessUtil.destroy(process);
@@ -315,8 +326,8 @@ public synchronized void stopProcess() {
}
}
- if (outConsumer != null && outConsumer.getBuffer() != null) {
- outConsumer.getBuffer().reset();
+ if (stdoutConsumer != null && stdoutConsumer.getBuffer() != null) {
+ stdoutConsumer.getBuffer().reset();
}
}
}
@@ -331,7 +342,7 @@ public boolean isReadyToStop() {
public void setReadyToStop(boolean nullable) {
if (nullable != this.nullable) {
- logger.trace("Ready to Stop: " + nullable);
+ LOGGER.trace("Ready to Stop: " + nullable);
}
this.nullable = nullable;
Please sign in to comment.
Something went wrong with that request. Please try again.