Systemtap: Fix remote script run errors.

-Assign a set of credentials (username, password, hostname) to each
remote script run rather than using a global set of values that get
overwritten with those of the most recent remote run. This makes it
possible to stop a remote script that has been started before another
remote script.

-Make remote runs compatible with the Debug view and the Launch workflow.

-Prevent NPEs caused by error dialogs of failed scripts.

-Make improvements to concurrency of script runs.

Change-Id: I701361cb6735028d84abe1b83bcaa34c40bd90b8
Reviewed-on: https://git.eclipse.org/r/22269
Tested-by: Hudson CI
Reviewed-by: Sami Wagiaalla <swagiaal@redhat.com>
IP-Clean: Sami Wagiaalla <swagiaal@redhat.com>
Tested-by: Sami Wagiaalla <swagiaal@redhat.com>

4 files changed, 97 insertions, 74 deletions

diff --git a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/actions/RunScriptHandler.java b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/actions/RunScriptHandler.java
index 321f360d10..7825ded015 100644
--- a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/actions/RunScriptHandler.java
+++ b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/actions/RunScriptHandler.java
@@ -23,6 +23,7 @@ import java.util.regex.Pattern;
 
 import org.eclipse.core.commands.AbstractHandler;
 import org.eclipse.core.commands.ExecutionEvent;
+import org.eclipse.core.commands.ExecutionException;
 import org.eclipse.core.commands.IHandlerListener;
 import org.eclipse.core.filesystem.URIUtil;
 import org.eclipse.core.resources.IFile;
@@ -47,6 +48,7 @@ import org.eclipse.linuxtools.internal.systemtap.ui.ide.structures.StapErrorPars
 import org.eclipse.linuxtools.internal.systemtap.ui.ide.structures.TapsetLibrary;
 import org.eclipse.linuxtools.systemtap.graphingapi.ui.widgets.ExceptionErrorDialog;
 import org.eclipse.linuxtools.systemtap.ui.consolelog.ScpClient;
+import org.eclipse.linuxtools.systemtap.ui.consolelog.structures.RemoteScriptOptions;
 import org.eclipse.linuxtools.systemtap.ui.consolelog.structures.ScriptConsole;
 import org.eclipse.linuxtools.systemtap.ui.editor.PathEditorInput;
 import org.eclipse.swt.layout.FillLayout;
@@ -78,9 +80,8 @@ public class RunScriptHandler extends AbstractHandler {
 	 * @since 2.0
 	 */
 	protected boolean continueRun = true;
-	private boolean runLocal = true;
+	private RemoteScriptOptions remoteOptions = null;
 	private IEditorPart ed = null;
-	private Shell shell = null;
 	private String fileName = null;
 	private String tmpfileName = null;
 	private String serverfileName = null;
@@ -128,23 +129,21 @@ public class RunScriptHandler extends AbstractHandler {
 	 */
 	private void findTargetEditor() {
 		ed = null;
-		shell = null;
 
 		if (path == null) {
 			IWorkbenchWindow window = PlatformUI.getWorkbench().getActiveWorkbenchWindow();
 			ed = window.getActivePage().getActiveEditor();
-			shell = window.getShell();
 			return;
 		}
 
 		for (IWorkbenchWindow window : PlatformUI.getWorkbench().getWorkbenchWindows()) {
 			IEditorPart edTest = window.getActivePage().getActiveEditor();
-			if (edTest != null && matchesEditor(edTest.getEditorInput(), edTest, window.getShell())) {
+			if (edTest != null && matchesEditor(edTest.getEditorInput(), edTest)) {
 				return;
 			}
 			for (IEditorReference ref : window.getActivePage().getEditorReferences()) {
 				try {
-					if (matchesEditor(ref.getEditorInput(), ref.getEditor(false), window.getShell())) {
+					if (matchesEditor(ref.getEditorInput(), ref.getEditor(false))) {
 						return;
 					}
 				} catch (PartInitException e) {
@@ -154,13 +153,12 @@ public class RunScriptHandler extends AbstractHandler {
 		}
 	}
 
-	private boolean matchesEditor(IEditorInput input, IEditorPart editor, Shell shell) {
+	private boolean matchesEditor(IEditorInput input, IEditorPart editor) {
 		if (input instanceof IPathEditorInput && ((IPathEditorInput) (input)).getPath().equals(this.path)) {
 			// Only save the editor when working with a file without a project (PathEditorInput),
 			// otherwise the editor isn't needed at all (saving is handled elsewhere in that case).
 			if (input instanceof PathEditorInput) {
 				this.ed = editor;
-				this.shell = shell;
 			}
 			return true;
 		}
@@ -173,14 +171,12 @@ public class RunScriptHandler extends AbstractHandler {
 	 * Finally, it gets an instance of <code>ScriptConsole</code> to run the script.
 	 */
 	@Override
-	public Object execute(ExecutionEvent event){
+	public Object execute(ExecutionEvent event) throws ExecutionException {
 		findTargetEditor();
 		final boolean local = getRunLocal();
 		if(isValid()) {
-			if(!local) {
-				if (!prepareNonLocalScript()) {
-					return null;
-				}
+			if(!local && !prepareNonLocalScript()) {
+				return null;
 			}
 			final String[] script = buildStandardScript();
 			final String[] envVars = getEnvironmentVariables();
@@ -210,7 +206,7 @@ public class RunScriptHandler extends AbstractHandler {
 						}
 						synchronized (console) {
 							if (!local) {
-								console.run(script, envVars, new StapErrorParser());
+								console.run(script, envVars, remoteOptions, new StapErrorParser());
 							} else {
 								console.runLocally(script, envVars, new StapErrorParser(), getProject());
 							}
@@ -224,21 +220,39 @@ public class RunScriptHandler extends AbstractHandler {
 		return null;
 	}
 
-	private boolean prepareNonLocalScript() {
+	/**
+	 * Attempts to set up a channel for a script that runs on a non-local host or user.
+	 * @return <code>true</code> on success, <code>false</code> on failure.
+	 * @throws ExecutionException If failure occurs during a (non-simple) launch,
+	 * this will throw an exception instead of returning <code>false</code>.
+	 */
+	private boolean prepareNonLocalScript() throws ExecutionException {
 		try {
-			ScpClient scpclient = new ScpClient();
+			ScpClient scpclient = new ScpClient(remoteOptions);
 			serverfileName = fileName.substring(fileName.lastIndexOf('/')+1);
 			tmpfileName="/tmp/"+ serverfileName; //$NON-NLS-1$
 			scpclient.transfer(fileName,tmpfileName);
-		} catch (JSchException e) {
-			ErrorDialog.openError(PlatformUI.getWorkbench()
-					.getActiveWorkbenchWindow().getShell(),
-					Localization.getString("RunScriptHandler.serverError"), Localization.getString("RunScriptHandler.serverError"), //$NON-NLS-1$ //$NON-NLS-2$
-					new Status(IStatus.ERROR, IDEPlugin.PLUGIN_ID, Localization.getString("RunScriptHandler.checkCredentials"))); //$NON-NLS-1$
-			return false;
-		} catch (IOException e) {
-			ExceptionErrorDialog.openError(Localization.getString("RunScriptHandler.ioError"), e); //$NON-NLS-1$
-			return false;
+		} catch (final JSchException | IOException e) {
+			final String message = e instanceof JSchException ? Localization.getString("RunScriptHandler.checkCredentials") //$NON-NLS-1$
+					: Localization.getString("RunScriptHandler.ioError"); //$NON-NLS-1$
+			if (launch == null) {
+				Display.getDefault().asyncExec(new Runnable() {
+					@Override
+					public void run() {
+						if (e instanceof JSchException) {
+							ErrorDialog.openError(PlatformUI.getWorkbench()
+									.getActiveWorkbenchWindow().getShell(),
+									Localization.getString("RunScriptHandler.serverError"), Localization.getString("RunScriptHandler.serverError"), //$NON-NLS-1$ //$NON-NLS-2$
+									new Status(IStatus.ERROR, IDEPlugin.PLUGIN_ID, message));
+						} else {
+							ExceptionErrorDialog.openError(Localization.getString("RunScriptHandler.ioError"), e); //$NON-NLS-1$
+						}
+					}
+				});
+				return false;
+			} else {
+				throw new ExecutionException(message, e);
+			}
 		}
 		return true;
 	}
@@ -284,10 +298,17 @@ public class RunScriptHandler extends AbstractHandler {
 	private boolean isValid() {
 		// If the path is not set this action will run the script from
 		// the active editor
-		if(!tryEditorSave() && this.path == null){
-			String msg = MessageFormat.format(Localization.getString("RunScriptAction.NoScriptFile"),(Object[]) null); //$NON-NLS-1$
-			MessageDialog.openWarning(PlatformUI.getWorkbench().getActiveWorkbenchWindow().getShell(), Localization.getString("RunScriptAction.Problem"), msg); //$NON-NLS-1$
-			return false;
+		if(!tryEditorSave()){
+			if (this.path == null){
+				Display.getDefault().asyncExec(new Runnable() {
+					@Override
+					public void run() {
+						String msg = MessageFormat.format(Localization.getString("RunScriptAction.NoScriptFile"),(Object[]) null); //$NON-NLS-1$
+						MessageDialog.openWarning(PlatformUI.getWorkbench().getActiveWorkbenchWindow().getShell(), Localization.getString("RunScriptAction.Problem"), msg); //$NON-NLS-1$
+					}
+				});
+				return false;
+			}
 		}
 		String filePath = this.getFilePath();
 		return filePath.endsWith(".stp") //$NON-NLS-1$
@@ -303,7 +324,7 @@ public class RunScriptHandler extends AbstractHandler {
 			Display.getDefault().syncExec(new Runnable() {
 				@Override
 				public void run() {
-					ed.doSave(new ProgressMonitorPart(shell, new FillLayout()));
+					ed.doSave(new ProgressMonitorPart(PlatformUI.getWorkbench().getActiveWorkbenchWindow().getShell(), new FillLayout()));
 				}
 			});
 		}
@@ -327,8 +348,13 @@ public class RunScriptHandler extends AbstractHandler {
 		}
 
 		if(fileName.contains(IDESessionSettings.tapsetLocation)) {
-			String msg = MessageFormat.format(Localization.getString("RunScriptAction.TapsetDirectoryRun"),(Object []) null); //$NON-NLS-1$
-			MessageDialog.openWarning(PlatformUI.getWorkbench().getActiveWorkbenchWindow().getShell(), Localization.getString("RunScriptAction.Error"), msg); //$NON-NLS-1$
+			Display.getDefault().asyncExec(new Runnable() {
+				@Override
+				public void run() {
+					String msg = MessageFormat.format(Localization.getString("RunScriptAction.TapsetDirectoryRun"),(Object []) null); //$NON-NLS-1$
+					MessageDialog.openWarning(PlatformUI.getWorkbench().getActiveWorkbenchWindow().getShell(), Localization.getString("RunScriptAction.Error"), msg); //$NON-NLS-1$
+				}
+			});
 			return false;
 		}
 		return true;
@@ -495,14 +521,18 @@ public class RunScriptHandler extends AbstractHandler {
 	}
 
 	/**
-	 * @since 2.0
+	 * Set the options for running the script remotely. If the script is to be run locally,
+	 * pass <code>null</code> as the only parameter.
+	 * @param remoteOptions The remote options of the script run, or <code>null</code> if the script
+	 * is to be run locally.
+	 * @since 3.0
 	 */
-	public void setLocalScript(boolean enabled) {
-		runLocal = enabled;
+	public void setRemoteScriptOptions(RemoteScriptOptions remoteOptions) {
+		this.remoteOptions = remoteOptions;
 	}
 
 	private boolean getRunLocal() {
-		return runLocal;
+		return remoteOptions == null;
 	}
 
 	@Override
diff --git a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/launcher/SystemTapScriptLaunch.java b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/launcher/SystemTapScriptLaunch.java
index ac72c679a3..7a04169047 100644
--- a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/launcher/SystemTapScriptLaunch.java
+++ b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/launcher/SystemTapScriptLaunch.java
@@ -7,12 +7,12 @@ import org.eclipse.debug.core.ILaunchManager;
 import org.eclipse.debug.core.Launch;
 import org.eclipse.linuxtools.systemtap.ui.consolelog.structures.ScriptConsole;
 import org.eclipse.linuxtools.systemtap.ui.consolelog.structures.ScriptConsole.ScriptConsoleObserver;
-import org.eclipse.swt.widgets.Display;
 
 public class SystemTapScriptLaunch extends Launch
 	implements ScriptConsoleObserver {
 
 	private ScriptConsole console = null;
+	private boolean runStarted = false;
 	private boolean runStopped = false;
 
 	public SystemTapScriptLaunch(ILaunchConfiguration launchConfiguration, String mode) {
@@ -43,23 +43,28 @@ public class SystemTapScriptLaunch extends Launch
 	}
 
 	@Override
-	public void runningStateChanged(boolean running) {
-		DebugPlugin.newProcess(this, console.getCommand().getProcess(), console.getName());
-		console.removeScriptConsoleObserver(this);
+	public void runningStateChanged(boolean started, boolean stopped) {
+		if (!runStarted && started) {
+			runStarted = started;
+			if (console.getCommand().getProcess() != null) {
+				DebugPlugin.newProcess(this, console.getCommand().getProcess(), console.getName());
+			}
+			if (stopped) {
+				runStopped = true;
+			}
+			console.removeScriptConsoleObserver(this);
+		}
 	}
 
 	@Override
 	public boolean canTerminate() {
-		if (runStopped) {
-			return false;
-		}
-		return console != null && !runStopped;
+		return !isTerminated();
 	}
 
 	@Override
 	public boolean isTerminated() {
 		if (!runStopped) {
-			if (super.isTerminated()) {
+			if (super.isTerminated() || (console != null && !console.isRunning())) {
 				runStopped = true;
 			}
 		}
@@ -68,14 +73,8 @@ public class SystemTapScriptLaunch extends Launch
 
 	@Override
 	public void terminate() {
-		if (console != null)
-		{
-			Display.getDefault().asyncExec(new Runnable() {
-				@Override
-				public void run() {
-					console.stop();
-				}
-			});
+		if (console != null) {
+			console.stop();
 		}
 	}
 
@@ -90,13 +89,14 @@ public class SystemTapScriptLaunch extends Launch
 	private void removeConsole() {
 		if (console != null) {
 			console.removeScriptConsoleObserver(this);
+			console.stop();
 			console = null;
 		}
 	}
 
 	public void forceRemove() {
-		runStopped = true;
 		removeConsole();
+		runStopped = true;
 		ILaunchManager manager = DebugPlugin.getDefault().getLaunchManager();
 		manager.removeLaunch(this);
 	}
diff --git a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/launcher/SystemTapScriptLaunchConfigurationDelegate.java b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/launcher/SystemTapScriptLaunchConfigurationDelegate.java
index 36808e36e5..07ca6f2097 100644
--- a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/launcher/SystemTapScriptLaunchConfigurationDelegate.java
+++ b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/launcher/SystemTapScriptLaunchConfigurationDelegate.java
@@ -15,6 +15,7 @@ import java.text.MessageFormat;
 import java.util.LinkedList;
 import java.util.List;
 
+import org.eclipse.core.commands.ExecutionException;
 import org.eclipse.core.resources.IFile;
 import org.eclipse.core.resources.IProject;
 import org.eclipse.core.resources.ResourcesPlugin;
@@ -30,7 +31,6 @@ import org.eclipse.debug.core.ILaunchConfiguration;
 import org.eclipse.debug.core.ILaunchConfigurationWorkingCopy;
 import org.eclipse.debug.core.ILaunchManager;
 import org.eclipse.debug.core.model.LaunchConfigurationDelegate;
-import org.eclipse.jface.preference.IPreferenceStore;
 import org.eclipse.linuxtools.internal.systemtap.ui.ide.actions.RunScriptChartHandler;
 import org.eclipse.linuxtools.internal.systemtap.ui.ide.actions.RunScriptHandler;
 import org.eclipse.linuxtools.internal.systemtap.ui.ide.preferences.IDEPreferenceConstants;
@@ -38,8 +38,7 @@ import org.eclipse.linuxtools.systemtap.graphingapi.core.datasets.IDataSet;
 import org.eclipse.linuxtools.systemtap.graphingapi.core.datasets.IDataSetParser;
 import org.eclipse.linuxtools.systemtap.graphingapi.core.structures.GraphData;
 import org.eclipse.linuxtools.systemtap.structures.process.SystemTapRuntimeProcessFactory;
-import org.eclipse.linuxtools.systemtap.ui.consolelog.internal.ConsoleLogPlugin;
-import org.eclipse.linuxtools.systemtap.ui.consolelog.preferences.ConsoleLogPreferenceConstants;
+import org.eclipse.linuxtools.systemtap.ui.consolelog.structures.RemoteScriptOptions;
 
 public class SystemTapScriptLaunchConfigurationDelegate extends
 		LaunchConfigurationDelegate {
@@ -106,8 +105,6 @@ public class SystemTapScriptLaunchConfigurationDelegate extends
 			throw new CoreException(new Status(IStatus.ERROR, getPluginID(), Messages.SystemTapScriptLaunchError_graph));
 		}
 
-		IPreferenceStore preferenceStore = ConsoleLogPlugin.getDefault().getPreferenceStore();
-
 		RunScriptHandler action;
 
 		boolean runWithChart = configuration.getAttribute(SystemTapScriptGraphOptionsTab.RUN_WITH_CHART, false);
@@ -137,22 +134,14 @@ public class SystemTapScriptLaunchConfigurationDelegate extends
 		}
 		action.setPath(scriptPath);
 
-		// User Name
-		String userName = configuration.getAttribute(SystemTapScriptLaunchConfigurationTab.USER_NAME_ATTR, ""); //$NON-NLS-1$
-		preferenceStore.setValue(ConsoleLogPreferenceConstants.SCP_USER, userName);
-
-		// User Password
-		String password = configuration.getAttribute(SystemTapScriptLaunchConfigurationTab.USER_PASS_ATTR, ""); //$NON-NLS-1$
-		preferenceStore.setValue(ConsoleLogPreferenceConstants.SCP_PASSWORD, password);
-
 		// Run locally and/or as current user.
 		boolean runAsCurrentUser = configuration.getAttribute(SystemTapScriptLaunchConfigurationTab.CURRENT_USER_ATTR, true);
 		boolean runLocal = configuration.getAttribute(SystemTapScriptLaunchConfigurationTab.LOCAL_HOST_ATTR, true);
-		action.setLocalScript(runLocal && runAsCurrentUser);
 
-		// Host Name.
-		String hostName = configuration.getAttribute(SystemTapScriptLaunchConfigurationTab.HOST_NAME_ATTR, "localhost"); //$NON-NLS-1$
-		preferenceStore.setValue(ConsoleLogPreferenceConstants.HOST_NAME, hostName);
+		action.setRemoteScriptOptions(runLocal && runAsCurrentUser ? null : new RemoteScriptOptions(
+						configuration.getAttribute(SystemTapScriptLaunchConfigurationTab.USER_NAME_ATTR, ""), //$NON-NLS-1$
+						configuration.getAttribute(SystemTapScriptLaunchConfigurationTab.USER_PASS_ATTR, ""), //$NON-NLS-1$
+						configuration.getAttribute(SystemTapScriptLaunchConfigurationTab.HOST_NAME_ATTR, "localhost"))); //$NON-NLS-1$
 
 		String value = configuration.getAttribute(IDEPreferenceConstants.STAP_CMD_OPTION[IDEPreferenceConstants.KEY], ""); //$NON-NLS-1$
 		if (!value.isEmpty()){
@@ -182,7 +171,11 @@ public class SystemTapScriptLaunchConfigurationDelegate extends
 		}
 
 		action.setLaunch((SystemTapScriptLaunch) launch);
-		action.execute(null);
+		try {
+			action.execute(null);
+		} catch (ExecutionException e) {
+			throw new CoreException(new Status(IStatus.ERROR, getPluginID(), e.getMessage()));
+		}
 	}
 
 }
diff --git a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/localization.properties b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/localization.properties
index 27d5729934..bc4662e88b 100644
--- a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/localization.properties
+++ b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/localization.properties
@@ -5,8 +5,8 @@ RunScriptAction.TapsetDirectoryRun=Can not run files in tapset directory\!
 RunScriptAction.Error=Error
 RunScriptHandler.couldNotOpenScriptFile=Could not open script file.
 RunScriptHandler.serverError=Error Connecting To Server
-RunScriptHandler.checkCredentials=Please check the supplied username, password and server address
-RunScriptHandler.ioError=IO Error with remote server
+RunScriptHandler.checkCredentials=Please check the supplied username, password and server address.
+RunScriptHandler.ioError=IO Error with remote server.
 RunScriptHandler.fileIOError=Error reading script file.
 
 KernelSourceAction.Insert=&Insert