From a1c67b653e54a8ada77ec83dc79a048efc75cf26 Mon Sep 17 00:00:00 2001 From: Uwe Stieber Date: Wed, 12 Jun 2013 09:41:35 +0200 Subject: Target Explorer: Fix value-add may hang up as stdout and stderr of the value add process are not read anymore once launched --- .../processes/ProcessOutputReaderThread.java | 30 +++++++++++++++++-- .../te/tcf/core/va/AbstractExternalValueAdd.java | 35 ++++++++++++++-------- .../tcf/te/tcf/core/va/ValueAddLauncher.java | 30 +++++++++++-------- 3 files changed, 68 insertions(+), 27 deletions(-) (limited to 'target_explorer') diff --git a/target_explorer/plugins/org.eclipse.tcf.te.runtime/src/org/eclipse/tcf/te/runtime/processes/ProcessOutputReaderThread.java b/target_explorer/plugins/org.eclipse.tcf.te.runtime/src/org/eclipse/tcf/te/runtime/processes/ProcessOutputReaderThread.java index 21d3767d4..09ff5f98d 100644 --- a/target_explorer/plugins/org.eclipse.tcf.te.runtime/src/org/eclipse/tcf/te/runtime/processes/ProcessOutputReaderThread.java +++ b/target_explorer/plugins/org.eclipse.tcf.te.runtime/src/org/eclipse/tcf/te/runtime/processes/ProcessOutputReaderThread.java @@ -35,6 +35,7 @@ public class ProcessOutputReaderThread extends Thread { // String builder to collect the read lines private StringBuilder lines; private String lastLine; + private boolean buffering; // finished reading all the output private boolean finished; @@ -54,6 +55,7 @@ public class ProcessOutputReaderThread extends Thread { assert streams != null; lastLine = ""; //$NON-NLS-1$ + buffering = true; finished = false; waiting = false; waiterSemaphore = new Object(); @@ -77,6 +79,28 @@ public class ProcessOutputReaderThread extends Thread { lines = new StringBuilder(); } + /** + * Returns if or if not the process output reader is buffering the output read. + * + * @return True if the output reader is buffering the output read, false otherwise. + */ + public final boolean isBuffering() { + return buffering; + } + + /** + * Toggle if or if not the process output reader is buffering the output read. + *

+ * Note: Switching off the buffering will reset the buffer too. Read the + * output buffer before switching off the buffering if needed. + * + * @param buffering True if the output reader shall buffer the output read, false otherwise. + */ + public final void setBuffering(boolean buffering) { + this.buffering = buffering; + if (!buffering) lines = new StringBuilder(); + } + /** * Returns if or if not the process output reader thread has finished. * @@ -142,8 +166,10 @@ public class ProcessOutputReaderThread extends Thread { } line = buffer.toString(); lastLine = line; - lines.append(line); - lines.append('\n'); + if (buffering) { + lines.append(line); + lines.append('\n'); + } CoreBundleActivator.getTraceHandler().trace(getPrefix() + " processLine: " + line, 3, this); //$NON-NLS-1$ } } diff --git a/target_explorer/plugins/org.eclipse.tcf.te.tcf.core/src/org/eclipse/tcf/te/tcf/core/va/AbstractExternalValueAdd.java b/target_explorer/plugins/org.eclipse.tcf.te.tcf.core/src/org/eclipse/tcf/te/tcf/core/va/AbstractExternalValueAdd.java index 0924d9f86..3a565a078 100644 --- a/target_explorer/plugins/org.eclipse.tcf.te.tcf.core/src/org/eclipse/tcf/te/tcf/core/va/AbstractExternalValueAdd.java +++ b/target_explorer/plugins/org.eclipse.tcf.te.tcf.core/src/org/eclipse/tcf/te/tcf/core/va/AbstractExternalValueAdd.java @@ -25,7 +25,9 @@ import org.eclipse.tcf.protocol.IPeer; import org.eclipse.tcf.protocol.JSON; import org.eclipse.tcf.protocol.Protocol; import org.eclipse.tcf.te.runtime.callback.Callback; +import org.eclipse.tcf.te.runtime.interfaces.IDisposable; import org.eclipse.tcf.te.runtime.interfaces.callback.ICallback; +import org.eclipse.tcf.te.runtime.processes.ProcessOutputReaderThread; import org.eclipse.tcf.te.runtime.utils.net.IPAddressUtil; import org.eclipse.tcf.te.tcf.core.activator.CoreBundleActivator; import org.eclipse.tcf.te.tcf.core.interfaces.tracing.ITraceIds; @@ -42,9 +44,21 @@ public abstract class AbstractExternalValueAdd extends AbstractValueAdd { /** * Class representing a value add entry */ - protected static class ValueAddEntry { + protected static class ValueAddEntry implements IDisposable { public Process process; public IPeer peer; + public ProcessOutputReaderThread outputReader; + public ProcessOutputReaderThread errorReader; + + /* (non-Javadoc) + * @see org.eclipse.tcf.te.runtime.interfaces.IDisposable#dispose() + */ + @Override + public void dispose() { + if (process != null) process.destroy(); + if (outputReader != null) outputReader.interrupt(); + if (errorReader != null) errorReader.interrupt(); + } } /* (non-Javadoc) @@ -140,9 +154,7 @@ public abstract class AbstractExternalValueAdd extends AbstractValueAdd { channel.removeChannelListener(this); // External value-add is not longer alive, clean up entries.remove(id); - if (finEntry.process != null) { - finEntry.process.destroy(); - } + finEntry.dispose(); // Invoke the callback done.done(AbstractExternalValueAdd.this, Status.OK_STATUS); } @@ -199,6 +211,9 @@ public abstract class AbstractExternalValueAdd extends AbstractValueAdd { } catch (IllegalThreadStateException e) { // Still running -> Associate the process with the entry entry.process = process; + // Associate the reader threads with the entry + entry.outputReader = launcher.getOutputReader(); + entry.errorReader = launcher.getErrorReader(); } } @@ -286,9 +301,9 @@ public abstract class AbstractExternalValueAdd extends AbstractValueAdd { entries.put(id, entry); } - // Stop the output reader thread + // Stop the buffering of the output reader if (launcher.getOutputReader() != null) { - launcher.getOutputReader().interrupt(); + launcher.getOutputReader().setBuffering(false); } } else { error = new FileNotFoundException(NLS.bind(Messages.AbstractExternalValueAdd_error_invalidLocation, this.getId())); @@ -343,9 +358,7 @@ public abstract class AbstractExternalValueAdd extends AbstractValueAdd { boolean alive = ((Boolean)getResult()).booleanValue(); if (alive) { entries.remove(id); - if (entry.process != null) { - entry.process.destroy(); - } + entry.dispose(); } done.done(AbstractExternalValueAdd.this, Status.OK_STATUS); } @@ -367,9 +380,7 @@ public abstract class AbstractExternalValueAdd extends AbstractValueAdd { // We force the value-add to shutdown if not yet gone by destroying the process. for (Entry entry : entries.entrySet()) { ValueAddEntry value = entry.getValue(); - if (value.process != null) { - value.process.destroy(); - } + value.dispose(); } // Clear all entries from the list entries.clear(); diff --git a/target_explorer/plugins/org.eclipse.tcf.te.tcf.core/src/org/eclipse/tcf/te/tcf/core/va/ValueAddLauncher.java b/target_explorer/plugins/org.eclipse.tcf.te.tcf.core/src/org/eclipse/tcf/te/tcf/core/va/ValueAddLauncher.java index e986fc212..c886dfc55 100644 --- a/target_explorer/plugins/org.eclipse.tcf.te.tcf.core/src/org/eclipse/tcf/te/tcf/core/va/ValueAddLauncher.java +++ b/target_explorer/plugins/org.eclipse.tcf.te.tcf.core/src/org/eclipse/tcf/te/tcf/core/va/ValueAddLauncher.java @@ -20,7 +20,6 @@ import org.eclipse.core.runtime.IPath; import org.eclipse.core.runtime.IStatus; import org.eclipse.core.runtime.Platform; import org.eclipse.osgi.util.NLS; -import org.eclipse.tcf.te.runtime.interfaces.IDisposable; import org.eclipse.tcf.te.runtime.processes.ProcessLauncher; import org.eclipse.tcf.te.runtime.processes.ProcessOutputReaderThread; import org.eclipse.tcf.te.runtime.utils.Host; @@ -32,7 +31,7 @@ import org.osgi.framework.Bundle; /** * Value-add launcher implementation. */ -public class ValueAddLauncher extends ProcessLauncher implements IDisposable { +public class ValueAddLauncher extends ProcessLauncher { // The target peer id private final String id; // The path of the value-add to launch @@ -43,6 +42,8 @@ public class ValueAddLauncher extends ProcessLauncher implements IDisposable { private Process process; // The process output reader private ProcessOutputReaderThread outputReader; + // The process error reader + private ProcessOutputReaderThread errorReader; /** * Constructor. @@ -62,17 +63,6 @@ public class ValueAddLauncher extends ProcessLauncher implements IDisposable { this.valueAddId = valueAddId; } - /* (non-Javadoc) - * @see org.eclipse.tcf.te.runtime.interfaces.IDisposable#dispose() - */ - @Override - public void dispose() { - if (process != null) { - process.destroy(); - process = null; - } - } - /** * Returns the process handle. * @@ -91,6 +81,15 @@ public class ValueAddLauncher extends ProcessLauncher implements IDisposable { return outputReader; } + /** + * Returns the process error reader. + * + * @return The process error reader or null. + */ + public ProcessOutputReaderThread getErrorReader() { + return errorReader; + } + /* (non-Javadoc) * @see org.eclipse.tcf.te.runtime.processes.ProcessLauncher#launch() */ @@ -143,6 +142,11 @@ public class ValueAddLauncher extends ProcessLauncher implements IDisposable { // Launch the process output reader outputReader = new ProcessOutputReaderThread(path.lastSegment(), new InputStream[] { process.getInputStream() }); outputReader.start(); + + // Launch the process error reader (not buffering) + errorReader = new ProcessOutputReaderThread(path.lastSegment(), new InputStream[] { process.getErrorStream() }); + errorReader.setBuffering(false); + errorReader.start(); } /** -- cgit v1.2.3