From 9a35a91bd07af3656533617287649c21550fa644 Mon Sep 17 00:00:00 2001 From: Pawel Piech Date: Wed, 7 Oct 2009 17:54:22 +0000 Subject: [PATCH] Bug 291342 - [debug view] DSF session executor intermittently deadlocks during session termination --- .../cdt/dsf/gdb/launching/GdbLaunch.java | 4 +- .../mi/service/command/MIInferiorProcess.java | 66 +++++++++---------- .../cdt/dsf/debug/model/DsfLaunch.java | 63 ++++++++++++++++++ .../examples/dsf/pda/launch/PDALaunch.java | 3 +- 4 files changed, 99 insertions(+), 37 deletions(-) create mode 100644 dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/debug/model/DsfLaunch.java diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/launching/GdbLaunch.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/launching/GdbLaunch.java index 261aff1f991..56c0ce638f1 100644 --- a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/launching/GdbLaunch.java +++ b/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; diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/MIInferiorProcess.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/MIInferiorProcess.java index 431436cdd01..b2245d11515 100644 --- a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/MIInferiorProcess.java +++ b/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; } diff --git a/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/debug/model/DsfLaunch.java b/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/debug/model/DsfLaunch.java new file mode 100644 index 00000000000..78fadeaad77 --- /dev/null +++ b/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; + } + }; + } +} diff --git a/dsf/org.eclipse.cdt.examples.dsf.pda/src/org/eclipse/cdt/examples/dsf/pda/launch/PDALaunch.java b/dsf/org.eclipse.cdt.examples.dsf.pda/src/org/eclipse/cdt/examples/dsf/pda/launch/PDALaunch.java index 2f510f65fd1..5ef55949aff 100644 --- a/dsf/org.eclipse.cdt.examples.dsf.pda/src/org/eclipse/cdt/examples/dsf/pda/launch/PDALaunch.java +++ b/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; *

*/ @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.