Bug 581462: Sometimes a script engine stalls and seems to be deadlockedHEADmaster

Change-Id: I3debb461b28eeb91a96c7fa159499dd5fc1240c9

2 files changed, 38 insertions, 30 deletions

diff --git a/plugins/org.eclipse.ease/src/org/eclipse/ease/AbstractReplScriptEngine.java b/plugins/org.eclipse.ease/src/org/eclipse/ease/AbstractReplScriptEngine.java
index 061b1c4c..90c3ad0d 100644
--- a/plugins/org.eclipse.ease/src/org/eclipse/ease/AbstractReplScriptEngine.java
+++ b/plugins/org.eclipse.ease/src/org/eclipse/ease/AbstractReplScriptEngine.java
@@ -55,8 +55,8 @@ public abstract class AbstractReplScriptEngine extends AbstractScriptEngine impl
 			// we can only set this before the engine got started
 			setSystem(!terminate);
 
-		synchronized (this) {
-			notifyAll();
+		synchronized (getScheduledScripts()) {
+			getScheduledScripts().notifyAll();
 		}
 	}
 
diff --git a/plugins/org.eclipse.ease/src/org/eclipse/ease/AbstractScriptEngine.java b/plugins/org.eclipse.ease/src/org/eclipse/ease/AbstractScriptEngine.java
index 42d79c47..24b93bd3 100644
--- a/plugins/org.eclipse.ease/src/org/eclipse/ease/AbstractScriptEngine.java
+++ b/plugins/org.eclipse.ease/src/org/eclipse/ease/AbstractScriptEngine.java
@@ -82,8 +82,8 @@ public abstract class AbstractScriptEngine extends Job implements IScriptEngine
 		}
 	}
 
-	/** List of code junks to be executed. */
-	private final List<Script> fScheduledScripts = Collections.synchronizedList(new ArrayList<Script>());
+	/** List of code parts to be executed. */
+	private final List<Script> fScheduledScripts = new ArrayList<>();
 
 	private final ListenerList<IExecutionListener> fExecutionListeners = new ListenerList<>();
 
@@ -135,10 +135,9 @@ public abstract class AbstractScriptEngine extends Job implements IScriptEngine
 	public final ScriptResult execute(final Object content) {
 		final Script script = (content instanceof Script) ? (Script) content : new Script(content);
 
-		fScheduledScripts.add(script);
-
-		synchronized (this) {
-			notifyAll();
+		synchronized (fScheduledScripts) {
+			fScheduledScripts.add(script);
+			fScheduledScripts.notifyAll();
 		}
 
 		return script.getResult();
@@ -237,19 +236,17 @@ public abstract class AbstractScriptEngine extends Job implements IScriptEngine
 			// main loop
 			while (!shallTerminate()) {
 
-				// execute code
-				if (!fScheduledScripts.isEmpty()) {
-					final Script piece = fScheduledScripts.remove(0);
-					inject(piece, true, false);
-
-				} else {
-					synchronized (this) {
-						try {
-							Logger.trace(Activator.PLUGIN_ID, TRACE_SCRIPT_ENGINE, "Engine idle: " + getName());
-							wait();
-						} catch (final InterruptedException e) {
+				try {
+					synchronized (fScheduledScripts) {
+						waitForNextScript();
+
+						if (!fScheduledScripts.isEmpty()) {
+							final Script piece = fScheduledScripts.remove(0);
+							inject(piece, true, false);
 						}
 					}
+				} catch (final InterruptedException e) {
+					// waiting got interrupted, quite likely a shutdown
 				}
 			}
 		}
@@ -260,6 +257,15 @@ public abstract class AbstractScriptEngine extends Job implements IScriptEngine
 		return cleanupRun(returnStatus);
 	}
 
+	private void waitForNextScript() throws InterruptedException {
+		synchronized (fScheduledScripts) {
+			while ((fScheduledScripts.isEmpty()) && (!shallTerminate())) {
+				Logger.trace(Activator.PLUGIN_ID, TRACE_SCRIPT_ENGINE, "Engine idle: " + getName());
+				fScheduledScripts.wait();
+			}
+		}
+	}
+
 	private IStatus setupRun() {
 		Logger.trace(Activator.PLUGIN_ID, TRACE_SCRIPT_ENGINE, "Engine started: " + getName());
 
@@ -294,9 +300,9 @@ public abstract class AbstractScriptEngine extends Job implements IScriptEngine
 		synchronized (fScheduledScripts) {
 			for (final Script script : fScheduledScripts)
 				script.setException(new ScriptExecutionException("Engine got terminated"));
-		}
 
-		fScheduledScripts.clear();
+			fScheduledScripts.clear();
+		}
 
 		notifyExecutionListeners(null, IExecutionListener.ENGINE_END);
 
@@ -309,8 +315,8 @@ public abstract class AbstractScriptEngine extends Job implements IScriptEngine
 				returnStatus = new Status(IStatus.ERROR, Activator.PLUGIN_ID, "Could not teardown script engine", e);
 			}
 		} finally {
-			synchronized (this) {
-				notifyAll();
+			synchronized (fScheduledScripts) {
+				fScheduledScripts.notifyAll();
 			}
 
 			closeStreams();
@@ -365,7 +371,9 @@ public abstract class AbstractScriptEngine extends Job implements IScriptEngine
 	 * @return <code>true</code> when termination is requested or there is no more work to be done
 	 */
 	protected boolean shallTerminate() {
-		return getMonitor().isCanceled() || fScheduledScripts.isEmpty();
+		synchronized (fScheduledScripts) {
+			return getMonitor().isCanceled() || fScheduledScripts.isEmpty();
+		}
 	}
 
 	@Override
@@ -376,8 +384,8 @@ public abstract class AbstractScriptEngine extends Job implements IScriptEngine
 
 		terminateCurrent();
 
-		synchronized (this) {
-			notifyAll();
+		synchronized (fScheduledScripts) {
+			fScheduledScripts.notifyAll();
 		}
 	}
 
@@ -392,9 +400,9 @@ public abstract class AbstractScriptEngine extends Job implements IScriptEngine
 		if (!Thread.currentThread().equals(getThread())) {
 			// we cannot join our own thread
 
-			synchronized (this) {
+			synchronized (fScheduledScripts) {
 				while (!isFinished())
-					wait(1000);
+					fScheduledScripts.wait(1000);
 			}
 		}
 	}
@@ -404,9 +412,9 @@ public abstract class AbstractScriptEngine extends Job implements IScriptEngine
 		if (!Thread.currentThread().equals(getThread())) {
 			// we cannot join our own thread
 
-			synchronized (this) {
+			synchronized (fScheduledScripts) {
 				if (!isFinished())
-					wait(timeout);
+					fScheduledScripts.wait(timeout);
 			}
 		}
 	}