Bug 355833: Thread filter effectively disables a breakpoint at the next session

dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/META-INF/MANIFEST.MF

@@ -3,7 +3,7 @@ Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName 
 Bundle-Vendor: %providerName
 Bundle-SymbolicName: org.eclipse.cdt.dsf.gdb.ui;singleton:=true
-Bundle-Version: 2.2.0.qualifier
+Bundle-Version: 2.2.1.qualifier
 Bundle-Activator: org.eclipse.cdt.dsf.gdb.internal.ui.GdbUIPlugin
 Bundle-Localization: plugin
 Require-Bundle: org.eclipse.ui,

dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/pom.xml

@@ -11,7 +11,7 @@
 		<relativePath>../../pom.xml</relativePath>
 	</parent>
 
-	<version>2.2.0-SNAPSHOT</version>
+	<version>2.2.1-SNAPSHOT</version>
 	<artifactId>org.eclipse.cdt.dsf.gdb.ui</artifactId>
 	<packaging>eclipse-plugin</packaging>
 </project>

dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/breakpoints/GdbThreadFilterEditor.java

@@ -306,23 +306,15 @@ public class GdbThreadFilterEditor {
         try {
             IContainerDMContext[] targets = filterExtension.getTargetFilters();
 
-            // TODO: Hack to properly initialize the target/thread list
-            // Should be done in filterExtension.initialize() but we don't know
-            // how to get the target list from an ICBreakpoint...
-            if (targets.length == 0) {
-            	targets = getDebugTargets();
-            	for (IContainerDMContext target : targets) {
-            		filterExtension.setTargetFilter(target);
-            	}
-            }
-            // TODO: End of hack
-
             for (int i = 0; i < targets.length; i++) {
                 IExecutionDMContext[] filteredThreads = filterExtension.getThreadFilters(targets[i]);
                 if (filteredThreads != null) {
-                    for (int j = 0; j < filteredThreads.length; ++j)
+                    for (int j = 0; j < filteredThreads.length; ++j) {
+                    	// Mark this thread as selected
                         fCheckHandler.checkThread(filteredThreads[j], true);
+                    }
                 } else {
+                	// Mark the entire process as selected
                     fCheckHandler.checkTarget(targets[i], true);
                 }
             }

dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/breakpoints/CBreakpointGdbThreadsFilterExtension.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2007, 2009 Wind River Systems and others.
+ * Copyright (c) 2007, 2011 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
@@ -35,8 +35,6 @@ public class CBreakpointGdbThreadsFilterExtension implements IDsfBreakpointExten
      * @see org.eclipse.cdt.debug.core.model.ICBreakpointExtension#initialize(org.eclipse.cdt.debug.core.model.ICBreakpoint)
      */
 	public void initialize(ICBreakpoint breakpoint) {
-		// TODO: Initialize fFilteredThreadsByTarget with current IContainerDMContext[]
-		// TODO: IRunControl?
     }
     
     /* (non-Javadoc)

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

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2007, 2010 Wind River and others.
+ * Copyright (c) 2007, 2011 Wind River 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
@@ -11,6 +11,7 @@
  *     Ericsson   - Added breakpoint filter support
  *     Ericsson   - Re-factored the service and put a few comments
  *     Ericsson   - Added Action support
+ *     Marc Khouzam (Ericsson) - Fix support for thread filter (Bug 355833)
  *******************************************************************************/
 
 package org.eclipse.cdt.dsf.mi.service;
@@ -423,6 +424,23 @@ public class MIBreakpointsManager extends AbstractDsfService implements IBreakpo
             determineDebuggerPath(dmc, attributes, new RequestMonitor(getExecutor(), countingRm) {
                 @Override
                 protected void handleSuccess() {                	
+                	// Before installing a breakpoint, set the target filter for that target.
+                	// Even if the breakpoint is disabled when we start, the target filter 
+                	// can be accessed by the user through the breakpoint properties UI, so
+                	// we must set it right now.
+                	// This is the reason we don't do this in 'installBreakpoint', which is not
+                	// called right away if the breakpoint is disabled.
+                	try {
+                		IContainerDMContext containerDmc = DMContexts.getAncestorOfType(dmc, IContainerDMContext.class);
+                		IDsfBreakpointExtension filterExt = getFilterExtension(breakpoint);
+                		if (filterExt.getThreadFilters(containerDmc) == null) {
+                			// Do this only if there wasn't already an entry, or else we would
+                			// erase the content of that previous entry.
+                			filterExt.setTargetFilter(containerDmc);
+                		}
+					} catch (CoreException e) {
+					}
+                	
                 	// Install only if the breakpoint is enabled at startup (Bug261082)
                     // Note that Tracepoints are not affected by "skip-all"
                 	boolean bpEnabled = attributes.get(ICBreakpoint.ENABLED).equals(true) &&
@@ -718,6 +736,14 @@ public class MIBreakpointsManager extends AbstractDsfService implements IBreakpo
         final Map<ICBreakpoint, Set<String>> threadsIDs = fBreakpointThreads.get(dmc);
         assert threadsIDs != null;
 
+        // Remove any target filter (if any)
+        try {
+        	IContainerDMContext containerDmc = DMContexts.getAncestorOfType(dmc, IContainerDMContext.class);
+        	getFilterExtension(breakpoint).removeTargetFilter(containerDmc);
+        }
+        catch( CoreException e ) {
+        }
+        
         // Remove breakpoint problem marker (if any)
         removeBreakpointProblemMarker(breakpoint);
 
@@ -1127,6 +1153,18 @@ public class MIBreakpointsManager extends AbstractDsfService implements IBreakpo
                                     new RequestMonitor(getExecutor(), countingRm) {
                                     @Override
                                     protected void handleSuccess() {
+                                    	// For a new breakpoint, set the target filter.
+                                    	try {
+                                    		IContainerDMContext containerDmc = DMContexts.getAncestorOfType(dmc, IContainerDMContext.class);
+                                    		IDsfBreakpointExtension filterExt = getFilterExtension((ICBreakpoint)breakpoint);
+                                    		if (filterExt.getThreadFilters(containerDmc) == null) {
+                                    			// Do this only if there wasn't already an entry, or else we would
+                                    			// erase the content of that previous entry.
+                                    			filterExt.setTargetFilter(containerDmc);
+                                    		}
+                    					} catch (CoreException e) {
+                    					}
+                                    	
                                         installBreakpoint(dmc, (ICBreakpoint) breakpoint,
                                             attrs, countingRm);
                                     }
@@ -1372,15 +1410,28 @@ public class MIBreakpointsManager extends AbstractDsfService implements IBreakpo
     	for (IBreakpointsTargetDMContext ctx : fPlatformBPs.keySet()) {
     		Map<ICBreakpoint, Map<String, Object>> breakpoints = fPlatformBPs.get(ctx);
             clearBreakpointStatus(breakpoints.keySet().toArray(new ICBreakpoint[breakpoints.size()]), ctx);
+            
+            // Also clear any target filter since we will not be calling uninstallBreakpoint() which
+            // usually does that work.
+            IContainerDMContext dmc = DMContexts.getAncestorOfType(ctx, IContainerDMContext.class);
+            clearTargetFilter(dmc, breakpoints.keySet());
     	}
     	// This will prevent Shutdown() from trying to remove bps from a
     	// backend that has already shutdown
         fPlatformBPs.clear();
     }
 
-    ///////////////////////////////////////////////////////////////////////////
+    private void clearTargetFilter(IContainerDMContext containerDmc, Set<ICBreakpoint> breakpoints) {
-    // Breakpoint status handling functions
+        // Remove any target filter (if any)
-    ///////////////////////////////////////////////////////////////////////////
+        try {
+        	for (ICBreakpoint bp : breakpoints) {
+        		getFilterExtension(bp).removeTargetFilter(containerDmc);
+        	}
+        }
+        catch( CoreException e ) {
+        }
+    }
+
 
     /**
      * @param bps
@@ -1653,39 +1704,26 @@ public class MIBreakpointsManager extends AbstractDsfService implements IBreakpo
     private Set<String> extractThreads(IBreakpointsTargetDMContext context, ICBreakpoint breakpoint) {
     	Set<String> results = new HashSet<String>();
     	
-        // Find the ancestor
+    	IExecutionDMContext[] threads = null;
-        List<IExecutionDMContext[]> threads = new ArrayList<IExecutionDMContext[]>(1);
-
     	try {
-            // Retrieve the targets
+    		IContainerDMContext containerDmc = DMContexts.getAncestorOfType(context, IContainerDMContext.class);
-            IDsfBreakpointExtension filterExtension = getFilterExtension(breakpoint);
+			threads = getFilterExtension(breakpoint).getThreadFilters(containerDmc);
-            IContainerDMContext[] targets = filterExtension.getTargetFilters();
+		} catch (CoreException e) {
+		}
 
-            // If no target is present, breakpoint applies to all.
+    	if (threads == null || threads.length == 0) {
-            if (targets.length == 0) {
             results.add("0"); //$NON-NLS-1$    
             return results;
         }
     	
-            // Extract the thread IDs (if there is none, we are covered)
-            for (IContainerDMContext ctxt : targets) {
-                if (ctxt.equals(context) || DMContexts.isAncestorOf(ctxt, context)) {
-                    threads.add(filterExtension.getThreadFilters(ctxt));
-                }
-            }
-        } catch (CoreException e1) {
-        }
-
         if (supportsThreads(breakpoint)) {
-            for (IExecutionDMContext[] targetThreads : threads) {
+            for (IExecutionDMContext thread : threads) {
-                if (targetThreads != null) {
-                    for (IExecutionDMContext thread : targetThreads) {
             	if (thread instanceof IMIExecutionDMContext) {
-                        	IMIExecutionDMContext dmc = (IMIExecutionDMContext) thread;
+            		results.add(Integer.toString(((IMIExecutionDMContext)thread).getThreadId()));
-                            results.add(((Integer) dmc.getThreadId()).toString());
-                        }
-                    }
                 } else {
+                	// If any of the threads is not an IMIExecutionDMContext,
+                	// we don't support thread filters at all.
+                	results.clear();
                     results.add("0"); //$NON-NLS-1$    
                     break;
                 }

dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/debug/service/IDsfBreakpointExtension.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2007, 2009 Wind River Systems and others.
+ * Copyright (c) 2007, 2011 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
@@ -7,6 +7,7 @@
  * 
  * Contributors:
  *     Wind River Systems - initial API and implementation
+ *     Marc Khouzam (Ericsson) - Adding javadoc (Bug 355833)
  *******************************************************************************/
 package org.eclipse.cdt.dsf.debug.service;
 
@@ -35,12 +36,60 @@ import org.eclipse.core.runtime.CoreException;
  */
 public interface IDsfBreakpointExtension extends ICBreakpointExtension {
    
-    public void setTargetFilter( IContainerDMContext target ) throws CoreException;
+    /**
-    public void removeTargetFilter( IContainerDMContext target ) throws CoreException;
+     * Add the given target to the list of this breakpoint's targets.    
+     * Target filters are not persisted across workbench invocations.
+     * 
+     * @param target the container to add to the list of this breakpoint's targets.
+     * @throws CoreException if unable to set the target filter
+     */
+	public void setTargetFilter(IContainerDMContext target) throws CoreException;
+	
+    /**
+     * Removes the given target from the breakpoint's target list.
+     * The breakpoint has no effect in the given target. 
+     * 
+     * @param target the container filter to be removed
+     * @exception CoreException if unable to remove the target filter
+     */
+    public void removeTargetFilter(IContainerDMContext target) throws CoreException;
+    
+    /**
+     * Returns all target filters set on this breakpoint.
+     * 
+     * @return the targets that this breakpoint is restricted to
+     * @exception CoreException if unable to determine this breakpoint's target filters
+     */
     public IContainerDMContext[] getTargetFilters() throws CoreException;
 
-    public void setThreadFilters( IExecutionDMContext[] threads ) throws CoreException;
+    /**
-    public void removeThreadFilters( IExecutionDMContext[] threads ) throws CoreException;
+     * Restricts this breakpoint to suspend only in the given threads 
-    public IExecutionDMContext[] getThreadFilters( IContainerDMContext target ) throws CoreException;
+     * when encountered in the given threads' target. 
+     * All threads must be from the same target.
+     * Thread filters are not persisted across workbench invocations.
+     * 
+     * @param threads the thread filters to be set
+     * @exception CoreException if unable to set the thread filters
+     */
+    public void setThreadFilters(IExecutionDMContext[] threads) throws CoreException;
 
+    /**
+     * Removes this breakpoint's thread filters in the given target, if any. 
+     * Has no effect if this breakpoint does not have filters in the given target.
+     * All threads must be from the same target.
+     * 
+     * @param threads the thread filters to be removed
+     * @exception CoreException if unable to remove the thread filter
+     */
+    public void removeThreadFilters(IExecutionDMContext[] threads) throws CoreException;
+    
+    /**
+     * Returns the threads in the given target in which this breakpoint
+     * is enabled or <code>null</code> if this breakpoint is enabled in
+     * all threads in the given target.
+     * 
+     * @return the threads in the given target that this breakpoint is enabled for
+     * @exception CoreException if unable to determine this breakpoint's thread filters
+     */
+    public IExecutionDMContext[] getThreadFilters(IContainerDMContext target) throws CoreException;
 }