Bug 245749 Avoid race condition by filling thread to group map of IMIProcesses service directly using new addThreadId() and removeThreadId() methods

plugins/org.eclipse.dd.gdb/src/org/eclipse/dd/gdb/internal/provisional/service/GDBProcesses.java

@@ -182,8 +182,10 @@ public class GDBProcesses extends MIProcesses {
 				// This service version only handles a single process to debug, therefore, we can simply
 				// create the context describing this process ourselves.
 				ICommandControlDMContext controlDmc = DMContexts.getAncestorOfType(dmc, ICommandControlDMContext.class);
-				IProcessDMContext procDmc = createProcessContext(controlDmc, inferiorProcess.getPid());
+				// Get the groupId properly for the case of an attach
-				IMIExecutionGroupDMContext newGroupDmc = createExecutionGroupContext(procDmc, inferiorProcess.getPid());
+				String groupId = getExecutionGroupIdFromThread(null);
+				IProcessDMContext procDmc = createProcessContext(controlDmc, groupId);
+				IMIExecutionGroupDMContext newGroupDmc = createExecutionGroupContext(procDmc, groupId);
 				rm.setData(new IContainerDMContext[] {newGroupDmc});
 				rm.done();
 			} else {
@@ -263,11 +265,18 @@ public class GDBProcesses extends MIProcesses {
 	
     @Override
 	public String getExecutionGroupIdFromThread(String threadId) {
+    	// We need to properly return the groupId based on the pid
+    	// to properly handle the case of an attach.  See bug 244749
+    	String groupId = null;
 		MIInferiorProcess inferiorProcess = fGdb.getInferiorProcess();
 	    if (inferiorProcess != null) {
-	    	return inferiorProcess.getPid();
+	    	groupId = inferiorProcess.getPid();
+	    }
+	    if (groupId != null) {
+	    	return groupId;
+	    } else {
+	    	return super.getExecutionGroupIdFromThread(threadId);
 	    }
-	    return null;
     }
 
 }

plugins/org.eclipse.dd.gdb/src/org/eclipse/dd/gdb/internal/provisional/service/GDBProcesses_7_0.java

@@ -49,8 +49,6 @@ import org.eclipse.dd.mi.service.command.commands.MIListThreadGroups;
 import org.eclipse.dd.mi.service.command.commands.MITargetAttach;
 import org.eclipse.dd.mi.service.command.commands.MITargetDetach;
 import org.eclipse.dd.mi.service.command.commands.MIThreadInfo;
-import org.eclipse.dd.mi.service.command.events.IMIDMEvent;
-import org.eclipse.dd.mi.service.command.events.MIThreadCreatedEvent;
 import org.eclipse.dd.mi.service.command.events.MIThreadGroupCreatedEvent;
 import org.eclipse.dd.mi.service.command.events.MIThreadGroupExitedEvent;
 import org.eclipse.dd.mi.service.command.output.IThreadInfo;
@@ -736,17 +734,6 @@ public class GDBProcesses_7_0 extends AbstractDsfService implements IMIProcesses
     	if (e instanceof ExecutionGroupStartedDMEvent) {
     		fContainerCommandCache.reset();
     	} else {
-    		// HACK figure out the thread and the group ids
-    		// I had to HACK GDB for this
-    		if (e instanceof IMIDMEvent) {
-        		String threadId = ((MIThreadCreatedEvent)((IMIDMEvent)e).getMIEvent()).getStrId();
-        		IContainerDMContext ctx = ((MIThreadCreatedEvent)((IMIDMEvent)e).getMIEvent()).getDMContext();
-        		if (ctx instanceof IMIExecutionGroupDMContext) {
-        			String groupId = ((IMIExecutionGroupDMContext)ctx).getGroupId();
-              		 fGroupIdMap.put(threadId, groupId);
-        		}
-    		}
-    		// END HACK
     		fThreadCommandCache.reset();
     	}
 	}
@@ -757,14 +744,6 @@ public class GDBProcesses_7_0 extends AbstractDsfService implements IMIProcesses
     	if (e instanceof ExecutionGroupExitedDMEvent) {
     		fContainerCommandCache.reset();
     	} else {
-    		// HACK figure out the thread and the group ids
-    		// I had to HACK GDB for this
-    		if (e instanceof IMIDMEvent) {
-        		String threadId = ((MIThreadCreatedEvent)((IMIDMEvent)e).getMIEvent()).getStrId();
-                fGroupIdMap.remove(threadId);
-    		}
-    		// END HACK
- 
     		fThreadCommandCache.reset();
     	}
     }
@@ -774,4 +753,12 @@ public class GDBProcesses_7_0 extends AbstractDsfService implements IMIProcesses
 		fThreadCommandCache.reset(context);
 	}
 
+	public void addThreadId(String threadId, String groupId) {
+ 		 fGroupIdMap.put(threadId, groupId);
+	}
+
+	public void removeThreadId(String threadId) {
+        fGroupIdMap.remove(threadId);		
+	}
+    
 }

plugins/org.eclipse.dd.gdb/src/org/eclipse/dd/gdb/internal/provisional/service/command/GDBControl.java

@@ -432,15 +432,6 @@ public class GDBControl extends AbstractMIControl implements IGDBControl {
     
     public IPath getExecutablePath() { return fExecPath; }
         
-    public void getInferiorProcessId(DataRequestMonitor<String> rm) {
-		String pid = null;
-		if (fInferiorProcess != null) {
-    		pid = fInferiorProcess.getPid();
-    	}
-		rm.setData(pid);
-    	rm.done();
-    }
-        
     @DsfServiceEventHandler 
     public void eventDispatched(ICommandControlShutdownDMEvent e) {
         // Handle our "GDB Exited" event and stop processing commands.

plugins/org.eclipse.dd.gdb/src/org/eclipse/dd/gdb/internal/provisional/service/command/GDBControl_7_0.java

@@ -433,15 +433,6 @@ public class GDBControl_7_0 extends AbstractMIControl implements IGDBControl {
     
     public IPath getExecutablePath() { return fExecPath; }
         
-    public void getInferiorProcessId(DataRequestMonitor<String> rm) {
-		String pid = null;
-		if (fInferiorProcess != null) {
-    		pid = fInferiorProcess.getPid();
-    	}
-		rm.setData(pid);
-    	rm.done();
-    }
-        
     @DsfServiceEventHandler 
     public void eventDispatched(ICommandControlShutdownDMEvent e) {
         // Handle our "GDB Exited" event and stop processing commands.

plugins/org.eclipse.dd.gdb/src/org/eclipse/dd/gdb/internal/provisional/service/command/IGDBControl.java

@@ -11,7 +11,6 @@
 package org.eclipse.dd.gdb.internal.provisional.service.command;
 
 import org.eclipse.core.runtime.IPath;
-import org.eclipse.dd.dsf.concurrent.DataRequestMonitor;
 import org.eclipse.dd.dsf.concurrent.RequestMonitor;
 import org.eclipse.dd.dsf.debug.service.command.ICommandControlService;
 import org.eclipse.dd.gdb.internal.provisional.launching.GdbLaunch;
@@ -55,6 +54,4 @@ public interface IGDBControl extends ICommandControlService {
 	int getGDBExitCode();
 
 	IPath getExecutablePath();
-
-	void getInferiorProcessId(DataRequestMonitor<String> rm);
 }

plugins/org.eclipse.dd.mi/src/org/eclipse/dd/mi/service/IMIProcesses.java

@@ -54,6 +54,29 @@ public interface IMIProcesses extends IProcesses
     IMIExecutionGroupDMContext createExecutionGroupContext(IProcessDMContext processDmc,
     												       String groupId);
     
+    /**
+     * Retrieve the groupId to which this threadId belongs
+     * 
+     * @param threadId The ID of the thread
+     * @return The ID of the group to which the specified thread belongs
+     */
     String getExecutionGroupIdFromThread(String threadId);
+    
+    /**
+     * This method should be called when a new thread is created.  It allows
+     * to keep track of the thread to group relationship.
+     * 
+     * @param threadId The ID of the new thread
+     * @param groupId The ID of the group to which the new thread belongs
+     */
+    void addThreadId(String threadId, String groupId);
+
+    /**
+     * This method should be called when a thread exits.  It is meant
+     * to remove the thread to group entry.
+     * 
+     * @param threadId The ID of the thread that exited
+     */
+    void removeThreadId(String threadId);
 }
 

plugins/org.eclipse.dd.mi/src/org/eclipse/dd/mi/service/MIProcesses.java

@@ -553,8 +553,10 @@ public class MIProcesses extends AbstractDsfService implements IMIProcesses, ICa
 			// This service version only handles a single process to debug, therefore, we can simply
 			// create the context describing this process ourselves.
 			ICommandControlDMContext controlDmc = DMContexts.getAncestorOfType(dmc, ICommandControlDMContext.class);
-			IProcessDMContext procDmc = createProcessContext(controlDmc, UNIQUE_GROUP_ID);
+			// Get the groupId properly for the case of an attach
-			IMIExecutionGroupDMContext newGroupDmc = createExecutionGroupContext(procDmc, UNIQUE_GROUP_ID);
+			String groupId = getExecutionGroupIdFromThread(null);
+			IProcessDMContext procDmc = createProcessContext(controlDmc, groupId);
+			IMIExecutionGroupDMContext newGroupDmc = createExecutionGroupContext(procDmc, groupId);
 			rm.setData(new IContainerDMContext[] {newGroupDmc});
 			rm.done();
 		}
@@ -647,4 +649,14 @@ public class MIProcesses extends AbstractDsfService implements IMIProcesses, ICa
         fContainerCommandCache.reset(context);
 	}
 
+	public void addThreadId(String threadId, String groupId) {
+		// This version of the service does not support multiple
+		// processes, so there is nothing to do here
+	}
+
+	public void removeThreadId(String threadId) {
+		// This version of the service does not support multiple
+		// processes, so there is nothing to do here
+	}
+
 }

plugins/org.eclipse.dd.mi/src/org/eclipse/dd/mi/service/command/CLIEventProcessor_7_0.java

@@ -106,6 +106,9 @@ public class CLIEventProcessor_7_0
         if (rr != null) {
             // Check if the state changed.
             String state = rr.getResultClass();
+            
+            // This is not handled properly yet
+            // see bug 247161
             if (fInferior != null && "error".equals(state)) { //$NON-NLS-1$
                 if (fInferior.getState() == MIInferiorProcess.State.RUNNING) {
                     fInferior.setState(MIInferiorProcess.State.STOPPED);

plugins/org.eclipse.dd.mi/src/org/eclipse/dd/mi/service/command/MIRunControlEventProcessor_7_0.java

@@ -180,9 +180,16 @@ public class MIRunControlEventProcessor_7_0
 
     		    	IMIProcesses procService = fServicesTracker.getService(IMIProcesses.class);
     		    	
+    		    	if ("thread-created".equals(miEvent)) { //$NON-NLS-1$
+    		    		// Update the thread to groupId map with the new groupId
+    		    		procService.addThreadId(threadId, groupId);
+    		    	} else {
+    		    		// It was not clear if MI would specify the groupId in the event
     		    		if (groupId == null) {
     		    			groupId = procService.getExecutionGroupIdFromThread(threadId);
     		    		}
+    		    		procService.removeThreadId(threadId);
+    		    	}
     		    	IProcessDMContext procDmc = procService.createProcessContext(fControlDmc, groupId);
     		    	IContainerDMContext processContainerDmc = procService.createExecutionGroupContext(procDmc, groupId);
 
@@ -193,9 +200,7 @@ public class MIRunControlEventProcessor_7_0
         				event = new MIThreadExitEvent(processContainerDmc, exec.getToken(), threadId);
     				}
     				
-    				if (event != null) {
    					fCommandControl.getSession().dispatchEvent(event, fCommandControl.getProperties());
-    				}
     			} else if ("thread-group-created".equals(miEvent) || "thread-group-exited".equals(miEvent)) { //$NON-NLS-1$ //$NON-NLS-2$
     				
     				String groupId = null;
@@ -222,14 +227,12 @@ public class MIRunControlEventProcessor_7_0
     						event = new MIThreadGroupExitedEvent(procDmc, exec.getToken(), groupId);
     					}
 
-    					if (event != null) {
    						fCommandControl.getSession().dispatchEvent(event, fCommandControl.getProperties());
     				}
     			}
     		}
     	}
     }
-    }
     
     protected MIEvent<?> createEvent(String reason, MIExecAsyncOutput exec) {
     	String threadId = null; 
@@ -253,6 +256,7 @@ public class MIRunControlEventProcessor_7_0
 
     	IMIProcesses procService = fServicesTracker.getService(IMIProcesses.class);
 
+    	// MI does not currently provide the group-id in these events
     	if (groupId == null) {
     		groupId = procService.getExecutionGroupIdFromThread(threadId);
     	}
@@ -319,7 +323,6 @@ public class MIRunControlEventProcessor_7_0
     	MIOutput output =  cmdResult.getMIOutput();
     	MIResultRecord rr = output.getMIResultRecord();
         if (rr != null) {
-            int id = rr.getToken();
             // Check if the state changed.
             String state = rr.getResultClass();
             if ("running".equals(state)) { //$NON-NLS-1$

plugins/org.eclipse.dd.tests.gdb/src/org/eclipse/dd/tests/gdb/GDBProcessesTest.java

@@ -108,17 +108,12 @@ public class GDBProcessesTest extends BaseTestCase {
          */
         fSession.getExecutor().submit(new Runnable() {
             public void run() {
-            	fGdbCtrl.getInferiorProcessId(
+            	String pid = fProcService.getExecutionGroupIdFromThread(null);
-            			new DataRequestMonitor<String>(fSession.getExecutor(), rm) {
+
-            				@Override
-            				protected void handleSuccess() {
-            					String pid = getData();
             	IProcessDMContext procDmc = fProcService.createProcessContext(fGdbCtrl.getContext(), pid);
             	fProcService.getExecutionData(procDmc, rm);            					
             }
         });
-            }
-        });
         /*
          * Wait for the operation to get over
          */

plugins/org.eclipse.dd.tests.gdb/src/org/eclipse/dd/tests/gdb/MIRunControlTest.java

@@ -115,18 +115,12 @@ public class MIRunControlTest extends BaseTestCase {
          */
         fRunCtrl.getExecutor().submit(new Runnable() {
             public void run() {
-            	fGDBCtrl.getInferiorProcessId(
+            	String pid = fProcService.getExecutionGroupIdFromThread(null);
-            			new DataRequestMonitor<String>(fRunCtrl.getExecutor(), rm) {
-            				@Override
-            				protected void handleSuccess() {
-            					String pid = getData();
             	IProcessDMContext procDmc = fProcService.createProcessContext(fGDBCtrl.getContext(), pid);
             	IContainerDMContext groupDmc = fProcService.createExecutionGroupContext(procDmc, pid);
             	fRunCtrl.getExecutionContexts(groupDmc, rm);
             }
         });
-            }
-        });
         wait.waitUntilDone(AsyncCompletionWaitor.WAIT_FOREVER);
         Assert.assertTrue(wait.getMessage(), wait.isOK());
 
@@ -207,18 +201,12 @@ public class MIRunControlTest extends BaseTestCase {
          */
          fRunCtrl.getExecutor().submit(new Runnable() {
             public void run() {
-            	fGDBCtrl.getInferiorProcessId(
+            	String pid = fProcService.getExecutionGroupIdFromThread(null);
-            			new DataRequestMonitor<String>(fRunCtrl.getExecutor(), rmExecutionCtxts) {
-            				@Override
-            				protected void handleSuccess() {
-            					String pid = getData();
             	IProcessDMContext procDmc = fProcService.createProcessContext(fGDBCtrl.getContext(), pid);
             	IContainerDMContext groupDmc = fProcService.createExecutionGroupContext(procDmc, pid);
             	fRunCtrl.getExecutionContexts(groupDmc, rmExecutionCtxts);
             }
         });
-            }
-        });
         wait.waitUntilDone(AsyncCompletionWaitor.WAIT_FOREVER);
         Assert.assertTrue(wait.getMessage(), wait.isOK());
         wait.waitReset();
@@ -265,18 +253,12 @@ public class MIRunControlTest extends BaseTestCase {
          */
         fRunCtrl.getExecutor().submit(new Runnable() {
             public void run() {
-            	fGDBCtrl.getInferiorProcessId(
+            	String pid = fProcService.getExecutionGroupIdFromThread(null);
-            			new DataRequestMonitor<String>(fRunCtrl.getExecutor(), rm) {
-            				@Override
-            				protected void handleSuccess() {
-            					String pid = getData();
             	IProcessDMContext procDmc = fProcService.createProcessContext(fGDBCtrl.getContext(), pid);
             	IContainerDMContext groupDmc = fProcService.createExecutionGroupContext(procDmc, pid);
             	fRunCtrl.getExecutionData(fRunCtrl.createMIExecutionContext(groupDmc, 1), rm);
             }
         });
-            }
-        });
         wait.waitUntilDone(AsyncCompletionWaitor.WAIT_FOREVER);
         Assert.assertTrue(wait.getMessage(), wait.isOK());
         
@@ -490,18 +472,12 @@ public class MIRunControlTest extends BaseTestCase {
         
          fRunCtrl.getExecutor().submit(new Runnable() {
             public void run() {
-               	fGDBCtrl.getInferiorProcessId(
+            	String pid = fProcService.getExecutionGroupIdFromThread(null);
-            			new DataRequestMonitor<String>(fRunCtrl.getExecutor(), rm) {
-            				@Override
-            				protected void handleSuccess() {
-            					String pid = getData();
             	IProcessDMContext procDmc = fProcService.createProcessContext(fGDBCtrl.getContext(), pid);
             	IContainerDMContext groupDmc = fProcService.createExecutionGroupContext(procDmc, pid);
             	fRunCtrl.resume(groupDmc, rm);
             }
         });
-            }
-        });
         wait.waitUntilDone(AsyncCompletionWaitor.WAIT_FOREVER);
 
         try {
@@ -516,11 +492,7 @@ public class MIRunControlTest extends BaseTestCase {
 		wait.waitReset();
 		fRunCtrl.getExecutor().submit(new Runnable() {
 			public void run() {
-				fGDBCtrl.getInferiorProcessId(
+				String pid = fProcService.getExecutionGroupIdFromThread(null);
-						new DataRequestMonitor<String>(fRunCtrl.getExecutor(), null) {
-							@Override
-							protected void handleCompleted() {
-            					String pid = getData();
 				IProcessDMContext procDmc = fProcService.createProcessContext(fGDBCtrl.getContext(), pid);
 				IContainerDMContext groupDmc = fProcService.createExecutionGroupContext(procDmc, pid);
 
@@ -528,8 +500,6 @@ public class MIRunControlTest extends BaseTestCase {
 				wait.waitFinished();
 			}
 		});
-			}
-		});
 
         wait.waitUntilDone(AsyncCompletionWaitor.WAIT_FOREVER);
         Assert.assertFalse("Target is suspended. It should have been running", (Boolean)wait.getReturnInfo());
@@ -577,11 +547,7 @@ public class MIRunControlTest extends BaseTestCase {
 		wait.waitReset();
         fRunCtrl.getExecutor().submit(new Runnable() {
             public void run() {
-               	fGDBCtrl.getInferiorProcessId(
+            	String pid = fProcService.getExecutionGroupIdFromThread(null);
-            			new DataRequestMonitor<String>(fRunCtrl.getExecutor(), null) {
-            				@Override
-            				protected void handleCompleted() {
-            					String pid = getData();
             	IProcessDMContext procDmc = fProcService.createProcessContext(fGDBCtrl.getContext(), pid);
             	IContainerDMContext groupDmc = fProcService.createExecutionGroupContext(procDmc, pid);
 
@@ -589,8 +555,6 @@ public class MIRunControlTest extends BaseTestCase {
             	wait.waitFinished();
             }
         });
-            }
-        });
 
         wait.waitUntilDone(AsyncCompletionWaitor.WAIT_FOREVER);
         Assert.assertFalse("Target is suspended. It should have been running", (Boolean)wait.getReturnInfo());