Bug 102929: Race conditions when setting breakpoints at launch. The initial breakpoint settings, stop in main and resume are batched in one job. This garantees the right order of commands sent to the backend.

4 files changed, 101 insertions, 141 deletions

diff --git a/debug/org.eclipse.cdt.debug.core/ChangeLog b/debug/org.eclipse.cdt.debug.core/ChangeLog
index cd45b002e03..3a4f97e6711 100644
--- a/debug/org.eclipse.cdt.debug.core/ChangeLog
+++ b/debug/org.eclipse.cdt.debug.core/ChangeLog
@@ -1,3 +1,11 @@
+2005-07-26 Mikhail Khodjaiants

+	Bug 102929: Race conditions when setting breakpoints at launch.

+	The initial breakpoint settings, stop in main and resume are batched 

+	in one job. This garantees the right order of commands sent to the backend.

+	* CDIDebugModel.java

+	* CBreakpointManager.java

+	* CDebugTarget.java

+

 2005-07-22 Alain Magloire

 	Fix to Copyright.

 	* src/org/eclipse/cdt/debug/internal/core/CBreakpointManager.java

diff --git a/debug/org.eclipse.cdt.debug.core/src/org/eclipse/cdt/debug/core/CDIDebugModel.java b/debug/org.eclipse.cdt.debug.core/src/org/eclipse/cdt/debug/core/CDIDebugModel.java
index f7be4e426ff..3ddbd2c8f16 100644
--- a/debug/org.eclipse.cdt.debug.core/src/org/eclipse/cdt/debug/core/CDIDebugModel.java
+++ b/debug/org.eclipse.cdt.debug.core/src/org/eclipse/cdt/debug/core/CDIDebugModel.java
@@ -11,9 +11,7 @@
 package org.eclipse.cdt.debug.core;
 
 import java.io.IOException;
-import java.text.MessageFormat;
 import java.util.HashMap;
-
 import org.eclipse.cdt.core.CCorePlugin;
 import org.eclipse.cdt.core.IAddress;
 import org.eclipse.cdt.core.IBinaryParser;
@@ -21,15 +19,12 @@ import org.eclipse.cdt.core.ICExtensionReference;
 import org.eclipse.cdt.core.IBinaryParser.IBinaryExecutable;
 import org.eclipse.cdt.core.IBinaryParser.IBinaryFile;
 import org.eclipse.cdt.core.IBinaryParser.IBinaryObject;
-import org.eclipse.cdt.debug.core.cdi.ICDILocation;
 import org.eclipse.cdt.debug.core.cdi.model.ICDITarget;
-import org.eclipse.cdt.debug.core.cdi.model.ICDITargetConfiguration;
 import org.eclipse.cdt.debug.core.model.ICAddressBreakpoint;
 import org.eclipse.cdt.debug.core.model.ICBreakpoint;
 import org.eclipse.cdt.debug.core.model.ICFunctionBreakpoint;
 import org.eclipse.cdt.debug.core.model.ICLineBreakpoint;
 import org.eclipse.cdt.debug.core.model.ICWatchpoint;
-import org.eclipse.cdt.debug.internal.core.ICDebugInternalConstants;
 import org.eclipse.cdt.debug.internal.core.breakpoints.CAddressBreakpoint;
 import org.eclipse.cdt.debug.internal.core.breakpoints.CFunctionBreakpoint;
 import org.eclipse.cdt.debug.internal.core.breakpoints.CLineBreakpoint;
@@ -94,13 +89,7 @@ public class CDIDebugModel {
 			public void run( IProgressMonitor m ) throws CoreException {
 				boolean stop = launch.getLaunchConfiguration().getAttribute( ICDTLaunchConfigurationConstants.ATTR_DEBUGGER_STOP_AT_MAIN, false );
 				target[0] = new CDebugTarget( launch, project, cdiTarget, name, debuggeeProcess, file, allowTerminate, allowDisconnect );
-				ICDITargetConfiguration config = cdiTarget.getConfiguration();
-				if ( config.supportsBreakpoints() && stop ) {
-					stopInMain( (CDebugTarget)target[0] );
-				}
-				if ( config.supportsResume() && resumeTarget ) {
-					target[0].resume();
-				}
+				((CDebugTarget)target[0]).start( stop, resumeTarget );
 			}
 		};
 		try {
@@ -461,21 +450,6 @@ public class CDIDebugModel {
 		return null;
 	}
 
-	protected static void stopInMain( CDebugTarget target ) throws DebugException {
-		ICDILocation location = target.getCDITarget().createFunctionLocation( "", "main" ); //$NON-NLS-1$ //$NON-NLS-2$
-		try {
-			target.setInternalTemporaryBreakpoint( location );
-		}
-		catch( DebugException e ) {
-			String message = MessageFormat.format( DebugCoreMessages.getString( "CDebugModel.0" ), new String[]{ e.getStatus().getMessage() } ); //$NON-NLS-1$
-			IStatus newStatus = new Status( IStatus.WARNING, e.getStatus().getPlugin(), ICDebugInternalConstants.STATUS_CODE_QUESTION, message, null );
-			if ( !CDebugUtils.question( newStatus, target ) ) {
-				target.terminate();
-				throw new DebugException( new Status( IStatus.OK, e.getStatus().getPlugin(), e.getStatus().getCode(), e.getStatus().getMessage(), null ) );
-			}
-		}
-	}
-
 	/**
 	 * @deprecated
 	 */
diff --git a/debug/org.eclipse.cdt.debug.core/src/org/eclipse/cdt/debug/internal/core/CBreakpointManager.java b/debug/org.eclipse.cdt.debug.core/src/org/eclipse/cdt/debug/internal/core/CBreakpointManager.java
index b4c1109f67e..00c2f30d9dd 100644
--- a/debug/org.eclipse.cdt.debug.core/src/org/eclipse/cdt/debug/internal/core/CBreakpointManager.java
+++ b/debug/org.eclipse.cdt.debug.core/src/org/eclipse/cdt/debug/internal/core/CBreakpointManager.java
@@ -27,7 +27,6 @@ import org.eclipse.cdt.debug.core.cdi.ICDIAddressLocation;
 import org.eclipse.cdt.debug.core.cdi.ICDICondition;
 import org.eclipse.cdt.debug.core.cdi.ICDIFunctionLocation;
 import org.eclipse.cdt.debug.core.cdi.ICDILineLocation;
-import org.eclipse.cdt.debug.core.cdi.ICDILocation;
 import org.eclipse.cdt.debug.core.cdi.ICDILocator;
 import org.eclipse.cdt.debug.core.cdi.event.ICDIChangedEvent;
 import org.eclipse.cdt.debug.core.cdi.event.ICDICreatedEvent;
@@ -217,10 +216,7 @@ public class CBreakpointManager implements IBreakpointsListener, IBreakpointMana
 	public void breakpointsAdded( IBreakpoint[] breakpoints ) {
 		if ( !isTargetAvailable() )
 			return;
-		for ( int i = 0; i < breakpoints.length; ++i ) {
-			if ( breakpoints[i] instanceof ICBreakpoint && isTargetBreakpoint( (ICBreakpoint)breakpoints[i] ) )
-				breakpointAdded0( breakpoints[i] );			
-		}
+		setBreakpointsOnTarget( breakpoints );
 	}
 
 	/* (non-Javadoc)
@@ -282,7 +278,7 @@ public class CBreakpointManager implements IBreakpointsListener, IBreakpointMana
 		breakpointsRemoved( (ICBreakpoint[])removeList.toArray( new ICBreakpoint[removeList.size()] ), new IMarkerDelta[0] );
 		breakpointsAdded( (ICBreakpoint[])installList.toArray( new ICBreakpoint[removeList.size()] ) );
 		for ( int i = 0; i < breakpoints.length; ++i ) {
-			if ( !(breakpoints[i] instanceof ICBreakpoint) || !isTargetAvailable() )
+			if ( breakpoints[i] instanceof ICBreakpoint && isTargetAvailable() )
 				changeBreakpointProperties( (ICBreakpoint)breakpoints[i], deltas[i] );
 		}
 	}
@@ -525,118 +521,78 @@ public class CBreakpointManager implements IBreakpointsListener, IBreakpointMana
 		getBreakpointNotifier().breakpointsRemoved( getDebugTarget(), breakpoints );
 	}
 
-	private void setBreakpoint( ICBreakpoint breakpoint ) {
-		try {
-			if ( !getBreakpointMap().isRegistered( breakpoint ) ) {
-				if ( breakpoint instanceof ICFunctionBreakpoint )
-					setFunctionBreakpoint( (ICFunctionBreakpoint)breakpoint );
-				else if ( breakpoint instanceof ICAddressBreakpoint )
-					setAddressBreakpoint( (ICAddressBreakpoint)breakpoint );
-				else if ( breakpoint instanceof ICLineBreakpoint )
-					setLineBreakpoint( (ICLineBreakpoint)breakpoint );
-				else if ( breakpoint instanceof ICWatchpoint )
-					setWatchpoint( (ICWatchpoint)breakpoint );
+	private ICBreakpoint[] register( IBreakpoint[] breakpoints ) {
+		ArrayList list = new ArrayList( breakpoints.length );
+		synchronized ( getBreakpointMap() ) {
+			for ( int i = 0; i < breakpoints.length; ++i ) {
+				if ( breakpoints[i] instanceof ICBreakpoint && isTargetBreakpoint( (ICBreakpoint)breakpoints[i] ) && !(getBreakpointMap().isRegistered( (ICBreakpoint)breakpoints[i] )) ) {
+					getBreakpointMap().register( (ICBreakpoint)breakpoints[i] );
+					list.add( breakpoints[i] );
+				}
 			}
 		}
-		catch( CoreException e ) {
-		}
-		catch( NumberFormatException e ) {
-		}
-		catch( CDIException e ) {
-		}
+		return (ICBreakpoint[])list.toArray( new ICBreakpoint[list.size()] );
 	}
 
-	private void setLocationBreakpointOnTarget( final ICBreakpoint breakpoint, final ICDITarget target, final ICDILocation location, final ICDICondition condition, final boolean enabled ) {
-		boolean registered = false;
-		synchronized ( getBreakpointMap() ) {
-			if ( !(registered = getBreakpointMap().isRegistered( breakpoint )) ) {
-				getBreakpointMap().register( breakpoint );
-			}
-		}
-		if ( !registered ) {
+	private void setBreakpointsOnTarget( IBreakpoint[] breakpoints ) {
+		final ICBreakpoint[] bkpts = register( breakpoints );
+		if ( bkpts.length > 0 ) {
 			DebugPlugin.getDefault().asyncExec( new Runnable() {				
 				public void run() {
-					try {
-						if ( breakpoint instanceof ICFunctionBreakpoint ) {
-							target.setFunctionBreakpoint( ICDIBreakpoint.REGULAR,
-									(ICDIFunctionLocation)location, condition, true );								
-						} else if ( breakpoint instanceof ICAddressBreakpoint ) {
-							target.setAddressBreakpoint( ICDIBreakpoint.REGULAR,
-									(ICDIAddressLocation)location, condition, true );
-							
-						} else if ( breakpoint instanceof ICLineBreakpoint ) {
-							target.setLineBreakpoint( ICDIBreakpoint.REGULAR,
-									(ICDILineLocation)location, condition, true );
-						}
-					}
-					catch( CDIException e ) {
-					} 
+					setBreakpointsOnTarget0( bkpts );
 				}
 			} );
 		}
 	}
 
-	private void setFunctionBreakpoint( ICFunctionBreakpoint breakpoint ) throws CDIException, CoreException {
-		boolean enabled = breakpoint.isEnabled();
+	protected void setBreakpointsOnTarget0( ICBreakpoint[] breakpoints ) {
 		ICDITarget cdiTarget = getCDITarget();
-		String function = breakpoint.getFunction();
-		String fileName = breakpoint.getFileName();
-		ICDIFunctionLocation location = cdiTarget.createFunctionLocation( fileName, function );
-		ICDICondition condition = createCondition( breakpoint );
-		setLocationBreakpointOnTarget( breakpoint, cdiTarget, location, condition, enabled );
-	}
-
-	private void setAddressBreakpoint( ICAddressBreakpoint breakpoint ) throws CDIException, CoreException, NumberFormatException {
-		final boolean enabled = breakpoint.isEnabled();
-		final ICDITarget cdiTarget = getCDITarget();
-		String address = breakpoint.getAddress();
-		final ICDIAddressLocation location = cdiTarget.createAddressLocation( new BigInteger ( ( address.startsWith( "0x" ) ) ? address.substring( 2 ) : address, 16 ) ); //$NON-NLS-1$
-		final ICDICondition condition = createCondition( breakpoint );
-		setLocationBreakpointOnTarget( breakpoint, cdiTarget, location, condition, enabled );
-	}
-
-	private void setLineBreakpoint( ICLineBreakpoint breakpoint ) throws CDIException, CoreException {
-		boolean enabled = breakpoint.isEnabled();
-		ICDITarget cdiTarget = getCDITarget();
-		String handle = breakpoint.getSourceHandle();
-		IPath path = convertPath( handle );
-		ICDILineLocation location = cdiTarget.createLineLocation( path.lastSegment()/*path.toPortableString()*/, breakpoint.getLineNumber() );
-		ICDICondition condition = createCondition( breakpoint );
-		setLocationBreakpointOnTarget( breakpoint, cdiTarget, location, condition, enabled );
-	}
-
-	private void setWatchpointOnTarget( final ICWatchpoint watchpoint, final ICDITarget target, final int accessType, final String expression, final ICDICondition condition, final boolean enabled ) {
-		boolean registered = false;
-		synchronized ( getBreakpointMap() ) {
-			if ( !(registered = getBreakpointMap().isRegistered( watchpoint )) ) {
-				getBreakpointMap().register( watchpoint );
-			}
-		}
-		if ( !registered ) {
-			DebugPlugin.getDefault().asyncExec( new Runnable() {				
-				public void run() {
-					try {
-							target.setWatchpoint( ICDIBreakpoint.REGULAR, accessType, expression, condition );
-					}
-					catch( CDIException e ) {
-					} 
+		for ( int i = 0; i < breakpoints.length; ++i ) {
+			try {
+				ICDIBreakpoint b = null;
+				if ( breakpoints[i] instanceof ICFunctionBreakpoint ) {
+					ICFunctionBreakpoint breakpoint = (ICFunctionBreakpoint)breakpoints[i]; 
+					String function = breakpoint.getFunction();
+					String fileName = breakpoint.getFileName();
+					ICDIFunctionLocation location = cdiTarget.createFunctionLocation( fileName, function );
+					ICDICondition condition = createCondition( breakpoint );
+					b = cdiTarget.setFunctionBreakpoint( ICDIBreakpoint.REGULAR, location, condition, true );								
+				} else if ( breakpoints[i] instanceof ICAddressBreakpoint ) {
+					ICAddressBreakpoint breakpoint = (ICAddressBreakpoint)breakpoints[i]; 
+					String address = breakpoint.getAddress();
+					ICDIAddressLocation location = cdiTarget.createAddressLocation( new BigInteger ( ( address.startsWith( "0x" ) ) ? address.substring( 2 ) : address, 16 ) ); //$NON-NLS-1$
+					ICDICondition condition = createCondition( breakpoint );
+					b = cdiTarget.setAddressBreakpoint( ICDIBreakpoint.REGULAR, location, condition, true );					
+				} else if ( breakpoints[i] instanceof ICLineBreakpoint ) {
+					ICLineBreakpoint breakpoint = (ICLineBreakpoint)breakpoints[i]; 
+					String handle = breakpoint.getSourceHandle();
+					IPath path = convertPath( handle );
+					ICDILineLocation location = cdiTarget.createLineLocation( path.lastSegment()/*path.toPortableString()*/, breakpoint.getLineNumber() );
+					ICDICondition condition = createCondition( breakpoint );
+					b = cdiTarget.setLineBreakpoint( ICDIBreakpoint.REGULAR, location, condition, true );
+				} else if ( breakpoints[i] instanceof ICWatchpoint ) {
+					ICWatchpoint watchpoint = (ICWatchpoint)breakpoints[i];
+					int accessType = 0;
+					accessType |= (watchpoint.isWriteType()) ? ICDIWatchpoint.WRITE : 0;
+					accessType |= (watchpoint.isReadType()) ? ICDIWatchpoint.READ : 0;
+					String expression = watchpoint.getExpression();
+					ICDICondition condition = createCondition( watchpoint );
+					b = cdiTarget.setWatchpoint( ICDIBreakpoint.REGULAR, accessType, expression, condition );
+				}
+				// Hack: see bug 105196: [CDI]: Add "enabled" flag to the "set...Breakpoint" methods 
+				if ( b != null && b.isEnabled() != breakpoints[i].isEnabled() ) {
+					b.setEnabled( breakpoints[i].isEnabled() );
 				}
-			} );
+			}
+			catch( CoreException e ) {
+			}
+			catch( NumberFormatException e ) {
+			}
+			catch( CDIException e ) {
+			}
 		}
 	}
 
-	private void setWatchpoint( ICWatchpoint watchpoint ) throws CDIException, CoreException {
-		final boolean enabled = watchpoint.isEnabled();
-		final ICDITarget cdiTarget = getCDITarget();
-		int accessType = 0;
-		accessType |= (watchpoint.isWriteType()) ? ICDIWatchpoint.WRITE : 0;
-		accessType |= (watchpoint.isReadType()) ? ICDIWatchpoint.READ : 0;
-		final int accessType1 = accessType;
-		final String expression = watchpoint.getExpression();
-		final ICDICondition condition = createCondition( watchpoint );
-		setWatchpointOnTarget( watchpoint, cdiTarget, accessType1, expression, condition, enabled );
-	}
-
 	protected ICDITarget getCDITarget() {
 		return getDebugTarget().getCDITarget();
 	}
@@ -803,7 +759,7 @@ public class CBreakpointManager implements IBreakpointsListener, IBreakpointMana
 	private void changeBreakpointProperties( ICBreakpoint breakpoint, ICDIBreakpoint cdiBreakpoint ) {
 		Boolean enabled = null;
 		try {
-			if ( cdiBreakpoint.isEnabled() != !breakpoint.isEnabled() )
+			if ( cdiBreakpoint.isEnabled() != breakpoint.isEnabled() )
 				enabled = Boolean.valueOf( breakpoint.isEnabled() );
 		}
 		catch( CDIException e ) {
@@ -845,7 +801,7 @@ public class CBreakpointManager implements IBreakpointsListener, IBreakpointMana
 		} );			
 	}
 
-	public void setBreakpoints() {
+	public void setInitialBreakpoints() {
 		IBreakpointManager manager = DebugPlugin.getDefault().getBreakpointManager();
 		IBreakpoint[] bps = manager.getBreakpoints( CDIDebugModel.getPluginIdentifier() );
 		for( int i = 0; i < bps.length; i++ ) {
@@ -858,17 +814,10 @@ public class CBreakpointManager implements IBreakpointsListener, IBreakpointMana
 					catch( CoreException e ) {
 					}
 				}
-				breakpointAdded0( bps[i] );
 			}
 		}
-	}
-
-	private void breakpointAdded0( IBreakpoint breakpoint ) {
-		if ( !isTargetAvailable() )
-			return;
-		if ( breakpoint instanceof ICAddressBreakpoint && !supportsAddressBreakpoint( (ICAddressBreakpoint)breakpoint ) )
-			return;
-		setBreakpoint( (ICBreakpoint)breakpoint );
+		ICBreakpoint[] breakpoints = register( bps );
+		setBreakpointsOnTarget0( breakpoints );
 	}
 
 	private boolean isTargetBreakpoint( ICBreakpoint breakpoint ) {
diff --git a/debug/org.eclipse.cdt.debug.core/src/org/eclipse/cdt/debug/internal/core/model/CDebugTarget.java b/debug/org.eclipse.cdt.debug.core/src/org/eclipse/cdt/debug/internal/core/model/CDebugTarget.java
index 1a09e0f4329..084af2c8d56 100644
--- a/debug/org.eclipse.cdt.debug.core/src/org/eclipse/cdt/debug/internal/core/model/CDebugTarget.java
+++ b/debug/org.eclipse.cdt.debug.core/src/org/eclipse/cdt/debug/internal/core/model/CDebugTarget.java
@@ -11,6 +11,7 @@
 package org.eclipse.cdt.debug.internal.core.model;
 
 import java.io.File;
+import java.text.MessageFormat;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
@@ -23,6 +24,7 @@ import org.eclipse.cdt.core.IBinaryParser.ISymbol;
 import org.eclipse.cdt.debug.core.CDIDebugModel;
 import org.eclipse.cdt.debug.core.CDebugCorePlugin;
 import org.eclipse.cdt.debug.core.CDebugUtils;
+import org.eclipse.cdt.debug.core.DebugCoreMessages;
 import org.eclipse.cdt.debug.core.ICDebugConstants;
 import org.eclipse.cdt.debug.core.ICGlobalVariableManager;
 import org.eclipse.cdt.debug.core.cdi.CDIException;
@@ -262,7 +264,19 @@ public class CDebugTarget extends CDebugElement implements ICDebugTarget, ICDIEv
 
 	private void initializeBreakpoints() {
 		getBreakpointManager().initialize();
-		getBreakpointManager().setBreakpoints();
+	}
+
+	public void start( boolean stopInMain, boolean resume ) throws DebugException {
+		ICDITargetConfiguration config = getConfiguration();
+		if ( config.supportsBreakpoints() ) {
+			getBreakpointManager().setInitialBreakpoints();
+			if ( stopInMain ) {
+				stopInMain();
+			}
+		}
+		if ( config.supportsResume() && resume ) {
+			resume();
+		}
 	}
 
 	/**
@@ -1684,4 +1698,19 @@ public class CDebugTarget extends CDebugElement implements ICDebugTarget, ICDIEv
 	protected void skipBreakpoints( boolean enabled ) {
 		getBreakpointManager().skipBreakpoints( enabled );
 	}
+
+	protected void stopInMain() throws DebugException {
+		ICDILocation location = getCDITarget().createFunctionLocation( "", "main" ); //$NON-NLS-1$ //$NON-NLS-2$
+		try {
+			setInternalTemporaryBreakpoint( location );
+		}
+		catch( DebugException e ) {
+			String message = MessageFormat.format( DebugCoreMessages.getString( "CDebugModel.0" ), new String[]{ e.getStatus().getMessage() } ); //$NON-NLS-1$
+			IStatus newStatus = new Status( IStatus.WARNING, e.getStatus().getPlugin(), ICDebugInternalConstants.STATUS_CODE_QUESTION, message, null );
+			if ( !CDebugUtils.question( newStatus, this ) ) {
+				terminate();
+				throw new DebugException( new Status( IStatus.OK, e.getStatus().getPlugin(), e.getStatus().getCode(), e.getStatus().getMessage(), null ) );
+			}
+		}
+	}
 }