Systemtap: Improve BrowserView UI loading.

Remove a number of race conditions that caused errors in the
contents of BrowserViews (Function view, Probe Alias view).
Also ensure that parser processes are terminated once they
are cancelled, and make some other minor UI and logic adjustments.

Change-Id: I4b65c5a7dd9622db5a10dc3822f0ea2fdec430bb
Signed-off-by: Andrew Ferrazzutti <aferrazz@redhat.com>
Reviewed-on: https://git.eclipse.org/r/31139
Tested-by: Hudson CI
Reviewed-by: Alexander Kurtakov <akurtako@redhat.com>
Tested-by: Alexander Kurtakov <akurtako@redhat.com>

7 files changed, 61 insertions, 25 deletions

diff --git a/profiling/org.eclipse.linuxtools.tools.launch.core/src/org/eclipse/linuxtools/tools/launch/core/factory/LinuxtoolsProcessFactory.java b/profiling/org.eclipse.linuxtools.tools.launch.core/src/org/eclipse/linuxtools/tools/launch/core/factory/LinuxtoolsProcessFactory.java
index 3ecded6443..e56ab9808f 100644
--- a/profiling/org.eclipse.linuxtools.tools.launch.core/src/org/eclipse/linuxtools/tools/launch/core/factory/LinuxtoolsProcessFactory.java
+++ b/profiling/org.eclipse.linuxtools.tools.launch.core/src/org/eclipse/linuxtools/tools/launch/core/factory/LinuxtoolsProcessFactory.java
@@ -196,7 +196,8 @@ public abstract class LinuxtoolsProcessFactory {
 
     /**
      * Runs a command on the given host using the given
-     * credentials and waits for the process to finish executing.
+     * credentials and waits for the process to finish executing, or until
+     * the executing thread is interrupted.
      *
      * @param args The command to run, followed by a list of optional arguments.
      * @param out A stream for the command's standard output.
@@ -216,7 +217,8 @@ public abstract class LinuxtoolsProcessFactory {
 
     /**
      * Runs a command on the given host using the given
-     * credentials and waits for the process to finish executing.
+     * credentials and waits for the process to finish executing, or until
+     * the executing thread is interrupted.
      *
      * @param args The command to run, followed by a list of optional arguments.
      * @param out A stream for the command's standard output.
diff --git a/systemtap/org.eclipse.linuxtools.systemtap.structures/src/org/eclipse/linuxtools/systemtap/structures/process/SystemtapProcessFactory.java b/systemtap/org.eclipse.linuxtools.systemtap.structures/src/org/eclipse/linuxtools/systemtap/structures/process/SystemtapProcessFactory.java
index 22587105f2..3624243c9c 100644
--- a/systemtap/org.eclipse.linuxtools.systemtap.structures/src/org/eclipse/linuxtools/systemtap/structures/process/SystemtapProcessFactory.java
+++ b/systemtap/org.eclipse.linuxtools.systemtap.structures/src/org/eclipse/linuxtools/systemtap/structures/process/SystemtapProcessFactory.java
@@ -61,7 +61,8 @@ public class SystemtapProcessFactory {
 
     /**
      * Runs stap with the given arguments on the given host using the given
-     * credentials and waits for the process to finish executing.
+     * credentials and waits for the process to finish executing, or until
+     * the executing thread is interrupted.
      *
      * @param user the user name to use on the remote machine.
      * @param host the host where the systemtap process will be run.
@@ -77,7 +78,8 @@ public class SystemtapProcessFactory {
 
     /**
      * Runs stap with the given arguments on the given host using the given
-     * credentials and waits for the process to finish executing.
+     * credentials and waits for the process to finish executing, or until
+     * the executing thread is interrupted.
      *
      * @param user the user name to use on the remote machine.
      * @param host the host where the systemtap process will be run.
diff --git a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/IDECloseMonitor.java b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/IDECloseMonitor.java
index 92c7520dee..0fe90bec3f 100644
--- a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/IDECloseMonitor.java
+++ b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/IDECloseMonitor.java
@@ -11,8 +11,6 @@
 
 package org.eclipse.linuxtools.internal.systemtap.ui.ide;
 
-import java.text.MessageFormat;
-
 import org.eclipse.jface.dialogs.MessageDialog;
 import org.eclipse.linuxtools.systemtap.ui.consolelog.structures.ScriptConsole;
 import org.eclipse.ui.IWorkbench;
@@ -30,12 +28,13 @@ public class IDECloseMonitor implements IWorkbenchListener {
     @Override
     public boolean preShutdown(IWorkbench workbench, boolean forced) {
         boolean close = true;
-        if(!forced) {
+        if (!forced) {
             IWorkbenchWindow window = workbench.getActiveWorkbenchWindow();
 
-            if(ScriptConsole.anyRunning()) {
-                String msg = MessageFormat.format(Localization.getString("IDECloseMonitor.StillRunning"),(Object[]) null); //$NON-NLS-1$
-                close = MessageDialog.openQuestion(window.getShell(), "Closing...", msg); //$NON-NLS-1$
+            if (ScriptConsole.anyRunning()) {
+                close = MessageDialog.openQuestion(window.getShell(),
+                        Localization.getString("IDECloseMonitor.StillRunningTitle"), //$NON-NLS-1$
+                        Localization.getString("IDECloseMonitor.StillRunning")); //$NON-NLS-1$
             }
         }
         return close;
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 76784fb0ae..25bc454047 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
@@ -33,6 +33,7 @@ DefinitionHandler.FileIOErrorMessage=Unable to open definition file.
 CEditor.ProbeInsertFailed=Probe Insertion Failure
 CEditor.CanNotProbeLine=Stap cannot insert a probe on this line of code.
 
+IDECloseMonitor.StillRunningTitle=Closing...
 IDECloseMonitor.StillRunning=There are scripts still running, are you sure you want to close?
 
 TapsetsPreferencePage.AdditionalTapsetsCategory=Additional tapsets to be used with every script
diff --git a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/ProbeParser.java b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/ProbeParser.java
index d2ab9fc06f..3bf8c17681 100644
--- a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/ProbeParser.java
+++ b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/ProbeParser.java
@@ -141,7 +141,7 @@ public final class ProbeParser extends TreeTapsetParser {
             TreeNode groupNode = null;
             while (st.hasNextLine()) {
                 if (monitor.isCanceled()) {
-                    canceled = false;
+                    canceled = true;
                     break;
                 }
                 String tokenString = st.nextLine();
diff --git a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/TapsetParser.java b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/TapsetParser.java
index efdfb9a31b..29574fab3c 100644
--- a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/TapsetParser.java
+++ b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/structures/TapsetParser.java
@@ -63,7 +63,10 @@ public abstract class TapsetParser extends Job {
 
     @Override
     protected void canceling() {
-        getThread().interrupt();
+        Thread thread = getThread();
+        if (thread != null) {
+            thread.interrupt();
+        }
     }
 
     /**
@@ -202,6 +205,8 @@ public abstract class TapsetParser extends Job {
                 port, EnvironmentVariablesPreferencePage.getEnvironmentVariables());
         if (channel == null) {
             displayError(Messages.TapsetParser_CannotRunStapTitle, Messages.TapsetParser_CannotRunStapMessage);
+        } else {
+            channel.getSession().disconnect();
         }
 
         return (!getErrors ? str : strErr).toString();
diff --git a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/views/TapsetBrowserView.java b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/views/TapsetBrowserView.java
index 53003ddafe..6fed41f592 100644
--- a/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/views/TapsetBrowserView.java
+++ b/systemtap/org.eclipse.linuxtools.systemtap.ui.ide/src/org/eclipse/linuxtools/internal/systemtap/ui/ide/views/TapsetBrowserView.java
@@ -34,23 +34,26 @@ public abstract class TapsetBrowserView extends BrowserView {
 
         @Override
         public void aboutToRun(IJobChangeEvent event) {
-            displayLoadingMessage();
+            synchronized (TapsetBrowserView.this) {
+                displayLoadingMessage();
+            }
         }
 
         @Override
         public void done(IJobChangeEvent event) {
-            if (event.getResult().isOK()) {
-                displayContents();
-            } else {
-                setViewerInput(null);
-                setRefreshable(true);
+            synchronized (TapsetBrowserView.this) {
+                if (event.getResult().isOK()) {
+                    displayContents();
+                } else {
+                    displayCancelContents();
+                }
             }
         }
 
     };
 
     /**
-     * Create a new {@link BrowserView} for displaying tapset contents, which will
+     * Creates a new {@link BrowserView} for displaying tapset contents, which will
      * be provided by an externally-run {@link TapsetParser}.
      * @param job The parser used to obtain the tapset contents this view will display.
      */
@@ -62,18 +65,32 @@ public abstract class TapsetBrowserView extends BrowserView {
     @Override
     public void createPartControl(Composite parent) {
         super.createPartControl(parent);
+        parser.addJobChangeListener(viewUpdater);
+        updateContents();
+        makeActions();
+    }
 
+    /**
+     * Updates the contents of this view without using the job listener.
+     * Use this when the result of the parser may be missed by the listener.
+     */
+    private synchronized void updateContents() {
         IStatus result = parser.getResult();
-        if (result != null && result.isOK()) {
-            displayContents();
+        if (result != null) {
+            if (result.isOK()) {
+                displayContents();
+            } else {
+                displayCancelContents();
+            }
         } else {
             displayLoadingMessage();
         }
-
-        parser.addJobChangeListener(viewUpdater);
-        makeActions();
     }
 
+    /**
+     * Displays a loading message in the view and sets the view as refreshable.
+     * Automatically called whenever a parse job restarts; should not be called by clients.
+     */
     @Override
     protected void displayLoadingMessage() {
         super.displayLoadingMessage();
@@ -82,11 +99,21 @@ public abstract class TapsetBrowserView extends BrowserView {
 
     /**
      * Populates the view with its contents obtained by the most recent run of {@link #parser}.
+     * Automatically called whenever a parse job succeeds; should not be called by clients.
      */
     abstract protected void displayContents();
 
     /**
-     * Rerun the tapset parser to refresh the list of both probes and functions.
+     * Clears the view and sets it as refreshable when the {@link parser} job fails or is canceled.
+     * Automatically called whenever a parse job fails; should not be called by clients.
+     */
+    protected void displayCancelContents() {
+        setViewerInput(null);
+        setRefreshable(true);
+    }
+
+    /**
+     * Reruns the tapset parser to refresh the list of both probes and functions.
      */
     @Override
     protected void refresh() {