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.

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

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();
+				((CDebugTarget)target[0]).start( stop, resumeTarget );
-				if ( config.supportsBreakpoints() && stop ) {
-					stopInMain( (CDebugTarget)target[0] );
-				}
-				if ( config.supportsResume() && resumeTarget ) {
-					target[0].resume();
-				}
 			}
 		};
 		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
 	 */

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 ) {
+		setBreakpointsOnTarget( breakpoints );
-			if ( breakpoints[i] instanceof ICBreakpoint && isTargetBreakpoint( (ICBreakpoint)breakpoints[i] ) )
-				breakpointAdded0( breakpoints[i] );			
-		}
 	}
 
 	/* (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,17 +521,67 @@ public class CBreakpointManager implements IBreakpointsListener, IBreakpointMana
 		getBreakpointNotifier().breakpointsRemoved( getDebugTarget(), breakpoints );
 	}
 
-	private void setBreakpoint( ICBreakpoint 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] );
+				}
+			}
+		}
+		return (ICBreakpoint[])list.toArray( new ICBreakpoint[list.size()] );
+	}
+
+	private void setBreakpointsOnTarget( IBreakpoint[] breakpoints ) {
+		final ICBreakpoint[] bkpts = register( breakpoints );
+		if ( bkpts.length > 0 ) {
+			DebugPlugin.getDefault().asyncExec( new Runnable() {				
+				public void run() {
+					setBreakpointsOnTarget0( bkpts );
+				}
+			} );
+		}
+	}
+
+	protected void setBreakpointsOnTarget0( ICBreakpoint[] breakpoints ) {
+		ICDITarget cdiTarget = getCDITarget();
+		for ( int i = 0; i < breakpoints.length; ++i ) {
 			try {
-			if ( !getBreakpointMap().isRegistered( breakpoint ) ) {
+				ICDIBreakpoint b = null;
-				if ( breakpoint instanceof ICFunctionBreakpoint )
+				if ( breakpoints[i] instanceof ICFunctionBreakpoint ) {
-					setFunctionBreakpoint( (ICFunctionBreakpoint)breakpoint );
+					ICFunctionBreakpoint breakpoint = (ICFunctionBreakpoint)breakpoints[i]; 
-				else if ( breakpoint instanceof ICAddressBreakpoint )
+					String function = breakpoint.getFunction();
-					setAddressBreakpoint( (ICAddressBreakpoint)breakpoint );
+					String fileName = breakpoint.getFileName();
-				else if ( breakpoint instanceof ICLineBreakpoint )
+					ICDIFunctionLocation location = cdiTarget.createFunctionLocation( fileName, function );
-					setLineBreakpoint( (ICLineBreakpoint)breakpoint );
+					ICDICondition condition = createCondition( breakpoint );
-				else if ( breakpoint instanceof ICWatchpoint )
+					b = cdiTarget.setFunctionBreakpoint( ICDIBreakpoint.REGULAR, location, condition, true );								
-					setWatchpoint( (ICWatchpoint)breakpoint );
+				} 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 ) {
@@ -545,96 +591,6 @@ public class CBreakpointManager implements IBreakpointsListener, IBreakpointMana
 			catch( CDIException e ) {
 			}
 		}
-
-	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 ) {
-			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 ) {
-					} 
-				}
-			} );
-		}
-	}
-
-	private void setFunctionBreakpoint( ICFunctionBreakpoint breakpoint ) throws CDIException, CoreException {
-		boolean enabled = breakpoint.isEnabled();
-		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 ) {
-					} 
-				}
-			} );
-		}
-	}
-
-	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() {
@@ -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] );
 			}
 		}
-	}
+		ICBreakpoint[] breakpoints = register( bps );
-
+		setBreakpointsOnTarget0( breakpoints );
-	private void breakpointAdded0( IBreakpoint breakpoint ) {
-		if ( !isTargetAvailable() )
-			return;
-		if ( breakpoint instanceof ICAddressBreakpoint && !supportsAddressBreakpoint( (ICAddressBreakpoint)breakpoint ) )
-			return;
-		setBreakpoint( (ICBreakpoint)breakpoint );
 	}
 
 	private boolean isTargetBreakpoint( ICBreakpoint breakpoint ) {

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 ) );
+			}
+		}
+	}
 }