Bug 291342 - [debug view] DSF session executor intermittently deadlocks during session termination

dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/launching/GdbLaunch.java

@@ -24,6 +24,7 @@ import org.eclipse.cdt.dsf.concurrent.RequestMonitor;
 import org.eclipse.cdt.dsf.concurrent.Sequence;
 import org.eclipse.cdt.dsf.concurrent.ThreadSafe;
 import org.eclipse.cdt.dsf.concurrent.ThreadSafeAndProhibitedFromDsfExecutor;
+import org.eclipse.cdt.dsf.debug.model.DsfLaunch;
 import org.eclipse.cdt.dsf.debug.model.DsfMemoryBlockRetrieval;
 import org.eclipse.cdt.dsf.debug.service.IDsfDebugServicesFactory;
 import org.eclipse.cdt.dsf.debug.service.IMemory.IMemoryDMContext;
@@ -47,7 +48,6 @@ import org.eclipse.core.runtime.Status;
 import org.eclipse.debug.core.DebugException;
 import org.eclipse.debug.core.DebugPlugin;
 import org.eclipse.debug.core.ILaunchConfiguration;
-import org.eclipse.debug.core.Launch;
 import org.eclipse.debug.core.model.IDisconnect;
 import org.eclipse.debug.core.model.IMemoryBlockRetrieval;
 import org.eclipse.debug.core.model.ISourceLocator;
@@ -57,7 +57,7 @@ import org.eclipse.debug.core.model.ITerminate;
  * The only object in the model that implements the traditional interfaces.
  */
 @ThreadSafe
-public class GdbLaunch extends Launch
+public class GdbLaunch extends DsfLaunch
     implements ITerminate, IDisconnect, ITracedLaunch
 {
     private DefaultDsfExecutor fExecutor;

dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/MIInferiorProcess.java

@@ -21,8 +21,6 @@ import java.io.PipedOutputStream;
 import java.util.concurrent.CancellationException;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.RejectedExecutionException;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.TimeoutException;
 
 import org.eclipse.cdt.dsf.concurrent.ConfinedToDsfExecutor;
 import org.eclipse.cdt.dsf.concurrent.DataRequestMonitor;
@@ -30,6 +28,8 @@ import org.eclipse.cdt.dsf.concurrent.DsfRunnable;
 import org.eclipse.cdt.dsf.concurrent.IDsfStatusConstants;
 import org.eclipse.cdt.dsf.concurrent.ImmediateExecutor;
 import org.eclipse.cdt.dsf.concurrent.Query;
+import org.eclipse.cdt.dsf.concurrent.ThreadSafe;
+import org.eclipse.cdt.dsf.concurrent.ThreadSafeAndProhibitedFromDsfExecutor;
 import org.eclipse.cdt.dsf.debug.service.IRunControl.IContainerDMContext;
 import org.eclipse.cdt.dsf.debug.service.IRunControl.IExecutionDMContext;
 import org.eclipse.cdt.dsf.debug.service.command.ICommandControlService;
@@ -103,10 +103,12 @@ public class MIInferiorProcess extends Process
      */
     private int fSuppressTargetOutputCounter = 0;
 
+    @ThreadSafe
     Integer fExitCode = null;
 
     private State fState = State.RUNNING;
     
+    @ConfinedToDsfExecutor("fSession#getExecutor")
     private String fInferiorPid = null;
 
     /**
@@ -199,6 +201,7 @@ public class MIInferiorProcess extends Process
         fDisposed = true;
     }
 
+    @ThreadSafe
     protected DsfSession getSession() {
         return fSession;
     }
@@ -206,8 +209,10 @@ public class MIInferiorProcess extends Process
     /**
      * @since 1.1
      */
+    @ConfinedToDsfExecutor("fSession#getExecutor")
     protected ICommandControlService getCommandControlService() { return fCommandControl; }
     
+    @ConfinedToDsfExecutor("fSession#getExecutor")
     protected boolean isDisposed() { return fDisposed; }
     
     @Override
@@ -225,7 +230,10 @@ public class MIInferiorProcess extends Process
         return fErrorStream;
     }
 
-    public synchronized void waitForSync() throws InterruptedException {
+    @ThreadSafeAndProhibitedFromDsfExecutor("fSession#getExecutor")
+    public void waitForSync() throws InterruptedException {
+        assert !getSession().getExecutor().isInExecutorThread();
+        
         while (getState() != State.TERMINATED) {
             wait(100);
         }        
@@ -234,37 +242,24 @@ public class MIInferiorProcess extends Process
     /**
      * @see java.lang.Process#waitFor()
      */
+    @ThreadSafeAndProhibitedFromDsfExecutor("fSession#getExecutor")
     @Override
     public int waitFor() throws InterruptedException {
+        assert !getSession().getExecutor().isInExecutorThread();
+        
         waitForSync();
         return exitValue();
     }
 
-	/**
-	 * Thrown by {@link MIInferiorProcess#exitValue()} when it is unable to
-	 * determine the exit value.
-	 * @since 2.1
-	 */
-    public static class ExitValueUnavailableException extends IllegalThreadStateException {
-		private static final long serialVersionUID = 1L;
-
-		public ExitValueUnavailableException(String s) {
-    		super(s);
-    	}
-    }
-
-	/**
-	 * Will throw an {@link ExitValueUnavailableException} if the DSF session
-	 * executor is unavailable (for more than 300ms). We do not wait
-	 * indefinitely, as our caller may have a lock that the active
-	 * Runnable/Callable is waiting for, and that could cause a deadlock.
-	 * 
-	 * @see java.lang.Process#exitValue()
-	 */
+    @ThreadSafeAndProhibitedFromDsfExecutor("fSession#getExecutor")
     @Override
     public int exitValue() {
-        if (fExitCode != null) {
-            return fExitCode;
+        assert !getSession().getExecutor().isInExecutorThread();
+        
+        synchronized (this) {
+            if (fExitCode != null) {
+                return fExitCode;
+            }
         }
         
         try {
@@ -301,13 +296,10 @@ public class MIInferiorProcess extends Process
             };
             fSession.getExecutor().execute(exitCodeQuery);
 
-            try {
-				// Don't wait indefinitely. See bugzilla 291342
-				fExitCode = exitCodeQuery.get(300, TimeUnit.MILLISECONDS);
-			} catch (TimeoutException e) {
-				exitCodeQuery.cancel(false);
-				throw new ExitValueUnavailableException("Timed out waiting for availability of DSF executor."); //$NON-NLS-1$
-			}
+            int exitCode = exitCodeQuery.get(); 
+            synchronized(this) {
+                fExitCode = exitCode;
+            }
             return fExitCode;
         } catch (RejectedExecutionException e) {
         } catch (InterruptedException e) {
@@ -375,10 +367,12 @@ public class MIInferiorProcess extends Process
         );         
     }
 
-    public State getState() { 
+    @ThreadSafe
+    public synchronized State getState() { 
         return fState;
     }
 
+    @ConfinedToDsfExecutor("fSession#getExecutor")
     public IExecutionDMContext getExecutionContext() {
         return fContainerDMContext;
     }
@@ -386,10 +380,12 @@ public class MIInferiorProcess extends Process
     /**
 	 * @since 1.1
 	 */
+    @ConfinedToDsfExecutor("fSession#getExecutor")
     public void setContainerContext(IContainerDMContext containerDmc) {
     	fContainerDMContext = containerDmc;
     }
     
+    @ConfinedToDsfExecutor("fSession#getExecutor")
     synchronized void setState(State state) {
         if (fState == State.TERMINATED) return;
         fState = state;
@@ -420,6 +416,7 @@ public class MIInferiorProcess extends Process
     /**
      * @since 1.1
      */
+    @ConfinedToDsfExecutor("fSession#getExecutor")
     public String getPid() { 
     	return fInferiorPid;
     }
@@ -427,6 +424,7 @@ public class MIInferiorProcess extends Process
     /**
      * @since 1.1
      */
+    @ConfinedToDsfExecutor("fSession#getExecutor")
     public void setPid(String pid) { 
     	fInferiorPid = pid;
     }

dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/debug/model/DsfLaunch.java

@@ -0,0 +1,63 @@
+/*******************************************************************************
+ * Copyright (c) 2008 Wind River Systems and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ * 
+ * Contributors:
+ *     Wind River Systems - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.cdt.dsf.debug.model;
+
+import org.eclipse.core.runtime.IProgressMonitor;
+import org.eclipse.core.runtime.IStatus;
+import org.eclipse.core.runtime.Status;
+import org.eclipse.core.runtime.jobs.Job;
+import org.eclipse.debug.core.ILaunchConfiguration;
+import org.eclipse.debug.core.Launch;
+import org.eclipse.debug.core.model.ISourceLocator;
+
+/**
+ * A Launch class to use for debuggers using the DSF.  This base class
+ * ensures that changed and terminated listeners are called using a 
+ * job, and thus not on a DSF services' session thread. 
+ * 
+ * @since 2.1
+ */
+public class DsfLaunch extends Launch {
+
+    public DsfLaunch(ILaunchConfiguration launchConfiguration, String mode, ISourceLocator locator) {
+        super(launchConfiguration, mode, locator);
+    }
+    
+    @Override
+    protected void fireChanged() {
+        new Job("Dispatch DSF Launch Changed event.") { //$NON-NLS-1$
+            { 
+                setSystem(true);
+            }
+            
+            @Override
+            protected IStatus run(IProgressMonitor monitor) {
+                DsfLaunch.super.fireChanged();
+                return Status.OK_STATUS;
+            }
+        };
+    }
+    
+    @Override
+    protected void fireTerminate() {
+        new Job("Dispatch DSF Launch Terminate event.") { //$NON-NLS-1$
+            { 
+                setSystem(true);
+            }
+            
+            @Override
+            protected IStatus run(IProgressMonitor monitor) {
+                DsfLaunch.super.fireTerminate();
+                return Status.OK_STATUS;
+            }
+        };
+    }
+}

dsf/org.eclipse.cdt.examples.dsf.pda/src/org/eclipse/cdt/examples/dsf/pda/launch/PDALaunch.java

@@ -16,6 +16,7 @@ import org.eclipse.cdt.dsf.concurrent.ImmediateExecutor;
 import org.eclipse.cdt.dsf.concurrent.RequestMonitor;
 import org.eclipse.cdt.dsf.concurrent.Sequence;
 import org.eclipse.cdt.dsf.concurrent.ThreadSafe;
+import org.eclipse.cdt.dsf.debug.model.DsfLaunch;
 import org.eclipse.cdt.dsf.service.DsfServiceEventHandler;
 import org.eclipse.cdt.dsf.service.DsfSession;
 import org.eclipse.cdt.examples.dsf.pda.PDAPlugin;
@@ -43,7 +44,7 @@ import org.eclipse.debug.core.model.ITerminate;
  * </p>
  */
 @ThreadSafe
-public class PDALaunch extends Launch
+public class PDALaunch extends DsfLaunch
 implements ITerminate
 {   
     // DSF executor and session.  Both are created and shutdown by the launch.