Attachment 202687 Details for Bug 355833 – Cleanup of thread filtering (master branch)

Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

[patch] Cleanup of thread filtering (master branch)

z.patch (text/plain), 20.39 KB, created by Marc Khouzam

on 2011-09-02 14:09:07 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: Marc Khouzam

Created: 2011-09-02 14:09:07 EDT

Size: 20.39 KB

patch

obsolete

>From 96ef71524156df99aa54d51843aafc57cbacd95e Fri, 2 Sep 2011 13:46:13 -0400
>From: Marc Khouzam <marc.khouzam@ericsson.com>
>Date: Fri, 2 Sep 2011 13:45:50 -0400
>Subject: [PATCH] Bug 355833: Thread filter effectively disables a breakpoint at the next session
>
>diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/META-INF/MANIFEST.MF b/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/META-INF/MANIFEST.MF
>index 7094644..ec4f8c3 100644
>--- a/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/META-INF/MANIFEST.MF
>+++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/META-INF/MANIFEST.MF
>@@ -3,7 +3,7 @@
> 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.3.0.qualifier
> Bundle-Activator: org.eclipse.cdt.dsf.gdb.internal.ui.GdbUIPlugin
> Bundle-Localization: plugin
> Require-Bundle: org.eclipse.ui,
>diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/breakpoints/CBreakpointGdbThreadFilterPage.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/breakpoints/CBreakpointGdbThreadFilterPage.java
>index 9be5996..f519b8e 100644
>--- a/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/breakpoints/CBreakpointGdbThreadFilterPage.java
>+++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/breakpoints/CBreakpointGdbThreadFilterPage.java
>@@ -1,5 +1,5 @@
> /*******************************************************************************
>- * Copyright (c) 2004, 2009 QNX Software Systems and others.
>+ * Copyright (c) 2004, 2011 QNX Software 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,10 +7,12 @@
>  *
>  * Contributors:
>  * QNX Software Systems - Initial API and implementation
>+ * Marc Khouzam (Ericsson) - Use IDsfBreakpointExtension directly  (Bug 355833)
>  *******************************************************************************/
> package org.eclipse.cdt.dsf.gdb.internal.ui.breakpoints; 
> 
> import org.eclipse.cdt.debug.core.model.ICBreakpoint;
>+import org.eclipse.cdt.dsf.debug.service.IDsfBreakpointExtension;
> import org.eclipse.cdt.dsf.gdb.breakpoints.CBreakpointGdbThreadsFilterExtension;
> import org.eclipse.cdt.dsf.gdb.launching.GdbLaunchDelegate;
> import org.eclipse.core.runtime.CoreException;
>@@ -44,12 +46,12 @@
>         return (ICBreakpoint)getElement().getAdapter(ICBreakpoint.class);
>     }
> 
>-	public CBreakpointGdbThreadsFilterExtension getFilterExtension() {
>+	public IDsfBreakpointExtension getFilterExtension() {
> 	    ICBreakpoint bp = getBreakpoint();
> 	    if (bp != null) {
> 	        try {
>-	        	CBreakpointGdbThreadsFilterExtension filter =
>-	        		(CBreakpointGdbThreadsFilterExtension) bp.getExtension(
>+	        	IDsfBreakpointExtension filter =
>+	        		(IDsfBreakpointExtension) bp.getExtension(
> 	        				GdbLaunchDelegate.GDB_DEBUG_MODEL_ID, CBreakpointGdbThreadsFilterExtension.class);
> 	        	filter.initialize(bp);
> 	        	return filter;
>diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/breakpoints/GdbThreadFilterEditor.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/breakpoints/GdbThreadFilterEditor.java
>index 22cbc9c..120215a 100644
>--- a/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/breakpoints/GdbThreadFilterEditor.java
>+++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/breakpoints/GdbThreadFilterEditor.java
>@@ -24,6 +24,7 @@
> import org.eclipse.cdt.dsf.concurrent.Query;
> import org.eclipse.cdt.dsf.datamodel.DMContexts;
> import org.eclipse.cdt.dsf.datamodel.IDMContext;
>+import org.eclipse.cdt.dsf.debug.service.IDsfBreakpointExtension;
> import org.eclipse.cdt.dsf.debug.service.IProcesses;
> import org.eclipse.cdt.dsf.debug.service.IProcesses.IProcessDMContext;
> import org.eclipse.cdt.dsf.debug.service.IProcesses.IThreadDMContext;
>@@ -32,7 +33,6 @@
> 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;
>-import org.eclipse.cdt.dsf.gdb.breakpoints.CBreakpointGdbThreadsFilterExtension;
> import org.eclipse.cdt.dsf.gdb.internal.ui.GdbUIPlugin;
> import org.eclipse.cdt.dsf.gdb.launching.GdbLaunch;
> import org.eclipse.cdt.dsf.mi.service.IMIExecutionDMContext;
>@@ -302,27 +302,19 @@
>      * a thread filter in a given thread, that thread should be checked.
>      */
>     protected void setInitialCheckedState() {
>-        CBreakpointGdbThreadsFilterExtension filterExtension = fPage.getFilterExtension();
>+    	IDsfBreakpointExtension filterExtension = fPage.getFilterExtension();
>         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);
>                 }
>             }
>@@ -332,7 +324,7 @@
>     }
> 
>     protected void doStore() {
>-        CBreakpointGdbThreadsFilterExtension filterExtension = fPage.getFilterExtension();
>+    	IDsfBreakpointExtension filterExtension = fPage.getFilterExtension();
>         IContainerDMContext[] targets = getDebugTargets();
>         for (int i = 0; i < targets.length; ++i) {
>             try {
>diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/breakpoints/CBreakpointGdbThreadsFilterExtension.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/breakpoints/CBreakpointGdbThreadsFilterExtension.java
>index 3b61d81..74d59bb 100644
>--- a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/breakpoints/CBreakpointGdbThreadsFilterExtension.java
>+++ b/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 @@
>      * @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)
>diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIBreakpointsManager.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIBreakpointsManager.java
>index 8350f96..40010e2 100644
>--- a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIBreakpointsManager.java
>+++ b/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;
>@@ -422,7 +423,24 @@
>             // Upon determining the debuggerPath, the breakpoint is installed
>             determineDebuggerPath(dmc, attributes, new RequestMonitor(getExecutor(), countingRm) {
>                 @Override
>-                protected void handleSuccess() {
>+                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 @@
>         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 @@
>                                     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 @@
>     	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();
>     }
> 
>-    ///////////////////////////////////////////////////////////////////////////
>-    // Breakpoint status handling functions
>-    ///////////////////////////////////////////////////////////////////////////
>+    private void clearTargetFilter(IContainerDMContext containerDmc, Set<ICBreakpoint> breakpoints) {
>+        // Remove any target filter (if any)
>+        try {
>+        	for (ICBreakpoint bp : breakpoints) {
>+        		getFilterExtension(bp).removeTargetFilter(containerDmc);
>+        	}
>+        }
>+        catch( CoreException e ) {
>+        }
>+    }
>+
> 
>     /**
>      * @param bps
>@@ -1651,41 +1702,28 @@
>      * @return
>      */
>     private Set<String> extractThreads(IBreakpointsTargetDMContext context, ICBreakpoint breakpoint) {
>-        Set<String> results = new HashSet<String>();
>+    	Set<String> results = new HashSet<String>();
>+    	
>+    	IExecutionDMContext[] threads = null;
>+    	try {
>+    		IContainerDMContext containerDmc = DMContexts.getAncestorOfType(context, IContainerDMContext.class);
>+			threads = getFilterExtension(breakpoint).getThreadFilters(containerDmc);
>+		} catch (CoreException e) {
>+		}
> 
>-        // Find the ancestor
>-        List<IExecutionDMContext[]> threads = new ArrayList<IExecutionDMContext[]>(1);
>-
>-        try {
>-            // Retrieve the targets
>-            IDsfBreakpointExtension filterExtension = getFilterExtension(breakpoint);
>-            IContainerDMContext[] targets = filterExtension.getTargetFilters();
>-
>-            // If no target is present, breakpoint applies to all.
>-            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 (threads == null || threads.length == 0) {
>+            results.add("0"); //$NON-NLS-1$    
>+            return results;
>         }
>-
>+    	
>         if (supportsThreads(breakpoint)) {
>-            for (IExecutionDMContext[] targetThreads : threads) {
>-                if (targetThreads != null) {
>-                    for (IExecutionDMContext thread : targetThreads) {
>-                        if (thread instanceof IMIExecutionDMContext) {
>-                        	IMIExecutionDMContext dmc = (IMIExecutionDMContext) thread;
>-                            results.add(((Integer) dmc.getThreadId()).toString());
>-                        }
>-                    }
>+            for (IExecutionDMContext thread : threads) {
>+            	if (thread instanceof IMIExecutionDMContext) {
>+            		results.add(Integer.toString(((IMIExecutionDMContext)thread).getThreadId()));
>                 } 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;
>                 }
>diff --git a/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/debug/service/IDsfBreakpointExtension.java b/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/debug/service/IDsfBreakpointExtension.java
>index ff0f92f..c9f3e1a 100644
>--- a/dsf/org.eclipse.cdt.dsf/src/org/eclipse/cdt/dsf/debug/service/IDsfBreakpointExtension.java
>+++ b/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 @@
>  */
> 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;
>-    public IExecutionDMContext[] getThreadFilters( IContainerDMContext target ) throws CoreException;
>+    /**
>+     * Restricts this breakpoint to suspend only in the given threads 
>+     * 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;
> }

Flags:

marc.khouzam: iplog-

Actions: View | Diff

Attachments on bug 355833: 202687 | 202690