Fix for bug228308

plugins/org.eclipse.dd.dsf.debug/src/org/eclipse/dd/dsf/debug/model/DsfMemoryBlockRetrieval.java

@@ -40,7 +40,6 @@ import org.eclipse.dd.dsf.debug.service.IMemory.IMemoryDMContext;
 import org.eclipse.dd.dsf.service.DsfServices;
 import org.eclipse.dd.dsf.service.DsfSession;
 import org.eclipse.dd.dsf.service.IDsfService;
-import org.eclipse.dd.dsf.service.DsfSession.SessionEndedListener;
 import org.eclipse.debug.core.DebugException;
 import org.eclipse.debug.core.DebugPlugin;
 import org.eclipse.debug.core.ILaunch;
@@ -74,7 +73,7 @@ import org.w3c.dom.NodeList;
  * code will still be functional after some trivial adjustments.
  * 
  */
-public class DsfMemoryBlockRetrieval extends PlatformObject implements IMemoryBlockRetrievalExtension, SessionEndedListener
+public class DsfMemoryBlockRetrieval extends PlatformObject implements IMemoryBlockRetrievalExtension
 {
 	private final String           fModelId;
 	private final DsfSession       fSession;
@@ -164,9 +163,6 @@ public class DsfMemoryBlockRetrieval extends PlatformObject implements IMemoryBl
         fWordSize     = 1;    // Get this from the launch configuration
         fSupportsValueModification = true;          // Get this from the launch configuration
         fSupportBaseAddressModification = false;    // Get this from the launch configuration
-
-		// So we are notified on exit and can save the memory blocks
-        DsfSession.addSessionEndedListener(this);
 	}
 
 	///////////////////////////////////////////////////////////////////////////
@@ -266,17 +262,11 @@ public class DsfMemoryBlockRetrieval extends PlatformObject implements IMemoryBl
 	    }
 	}
 
-    /**
-     * On session exit, save the memory blocks in the launch configuration
-     */
-    public void sessionEnded(DsfSession session) {
-        DsfSession.removeSessionEndedListener(this);
-        saveMemoryBlocks();        
-    }
-
     // FIXME: Each retrieval overwrites the previous one :-(
-    // FIXME: Racing condition :-( - synchronize on launch config enough?
+    // In theory, we should make this a Job since we are writing to the file system.
-    // FIXME: Make it a Job?
+	// However, this would cause the same racing condition as Bug228308. Finally, we
+	// don't care too much about the UI responsiveness since we are in the process of
+	// shutting down :-)
 	public void saveMemoryBlocks() {
 		try {
 			ILaunchConfigurationWorkingCopy wc = fLaunchConfig.getWorkingCopy();

plugins/org.eclipse.dd.gdb/src/org/eclipse/dd/gdb/internal/provisional/launching/GdbLaunch.java

@@ -29,6 +29,8 @@ import org.eclipse.dd.dsf.concurrent.RequestMonitor;
 import org.eclipse.dd.dsf.concurrent.Sequence;
 import org.eclipse.dd.dsf.concurrent.ThreadSafe;
 import org.eclipse.dd.dsf.concurrent.ThreadSafeAndProhibitedFromDsfExecutor;
+import org.eclipse.dd.dsf.debug.model.DsfMemoryBlockRetrieval;
+import org.eclipse.dd.dsf.debug.service.IMemory.IMemoryDMContext;
 import org.eclipse.dd.dsf.service.DsfServiceEventHandler;
 import org.eclipse.dd.dsf.service.DsfServicesTracker;
 import org.eclipse.dd.dsf.service.DsfSession;
@@ -40,6 +42,7 @@ import org.eclipse.debug.core.DebugException;
 import org.eclipse.debug.core.DebugPlugin;
 import org.eclipse.debug.core.ILaunchConfiguration;
 import org.eclipse.debug.core.Launch;
+import org.eclipse.debug.core.model.IMemoryBlockRetrieval;
 import org.eclipse.debug.core.model.ISourceLocator;
 import org.eclipse.debug.core.model.ITerminate;
 
@@ -56,6 +59,7 @@ public class GdbLaunch extends Launch
     private boolean fInitialized = false;
     private boolean fShutDown = false;
 
+    private DsfMemoryBlockRetrieval fMemRetrieval;
     
     public GdbLaunch(ILaunchConfiguration launchConfiguration, String mode, ISourceLocator locator) {
         super(launchConfiguration, mode, locator);
@@ -74,7 +78,6 @@ public class GdbLaunch extends Launch
     public void initializeControl()
         throws CoreException
     {
-        
         Runnable initRunnable = new DsfRunnable() { 
             public void run() {
                 fTracker = new DsfServicesTracker(GdbPlugin.getBundleContext(), fSession.getId());
@@ -93,6 +96,28 @@ public class GdbLaunch extends Launch
         } catch (ExecutionException e) {
             new Status(IStatus.ERROR, GdbPlugin.PLUGIN_ID, IDsfStatusConstants.INTERNAL_ERROR, "Error initializing launch", e); //$NON-NLS-1$
         }
+
+        // Create a memory retrieval and register it with the session 
+        try {
+            fExecutor.submit( new Callable<Object>() {
+                public Object call() throws CoreException {
+                    GDBControl gdbControl = fTracker.getService(GDBControl.class);
+                    if (gdbControl != null) {
+                        fMemRetrieval = new DsfMemoryBlockRetrieval(
+                                GdbLaunchDelegate.GDB_DEBUG_MODEL_ID, getLaunchConfiguration(), fSession);
+                        fSession.registerModelAdapter(IMemoryBlockRetrieval.class, fMemRetrieval);
+                        fMemRetrieval.initialize((IMemoryDMContext) gdbControl.getControlDMContext());
+                    }
+                    return null;
+                }
+            }).get();
+        } catch (InterruptedException e) {
+            throw new CoreException(new Status(IStatus.ERROR, GdbPlugin.PLUGIN_ID, 0, "Interrupted while waiting for get process callable.", e)); //$NON-NLS-1$
+        } catch (ExecutionException e) {
+            throw (CoreException)e.getCause();
+        } catch (RejectedExecutionException e) {
+            throw new CoreException(new Status(IStatus.ERROR, GdbPlugin.PLUGIN_ID, 0, "Debugger shut down before launch was completed.", e)); //$NON-NLS-1$
+        }
     }
 
     public DsfSession getSession() { return fSession; }
@@ -208,6 +233,10 @@ public class GdbLaunch extends Launch
                     fTracker.dispose();
                     fTracker = null;
                     DsfSession.endSession(fSession);
+                    
+                    // DsfMemoryBlockRetrieval.saveMemoryBlocks();
+                    fMemRetrieval.saveMemoryBlocks();
+                    
                     // endSession takes a full dispatch to distribute the 
                     // session-ended event, finish step only after the dispatch.
                     fExecutor.shutdown();

plugins/org.eclipse.dd.gdb/src/org/eclipse/dd/gdb/internal/provisional/launching/GdbLaunchDelegate.java

@@ -12,9 +12,7 @@ package org.eclipse.dd.gdb.internal.provisional.launching;
 
 import java.io.File;
 import java.io.IOException;
-import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
-import java.util.concurrent.RejectedExecutionException;
 
 import org.eclipse.cdt.core.model.ICProject;
 import org.eclipse.cdt.debug.core.ICDTLaunchConfigurationConstants;
@@ -30,12 +28,8 @@ import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.NullProgressMonitor;
 import org.eclipse.core.runtime.Status;
 import org.eclipse.dd.dsf.concurrent.ThreadSafe;
-import org.eclipse.dd.dsf.debug.model.DsfMemoryBlockRetrieval;
-import org.eclipse.dd.dsf.debug.service.IMemory.IMemoryDMContext;
-import org.eclipse.dd.dsf.service.DsfServicesTracker;
 import org.eclipse.dd.gdb.internal.GdbPlugin;
 import org.eclipse.dd.gdb.internal.provisional.IGDBLaunchConfigurationConstants;
-import org.eclipse.dd.gdb.internal.provisional.service.command.GDBControl;
 import org.eclipse.dd.gdb.internal.provisional.service.command.GDBControl.SessionType;
 import org.eclipse.debug.core.DebugException;
 import org.eclipse.debug.core.DebugPlugin;
@@ -44,7 +38,6 @@ import org.eclipse.debug.core.ILaunchConfiguration;
 import org.eclipse.debug.core.ILaunchManager;
 import org.eclipse.debug.core.IStatusHandler;
 import org.eclipse.debug.core.model.ILaunchConfigurationDelegate2;
-import org.eclipse.debug.core.model.IMemoryBlockRetrieval;
 import org.eclipse.debug.core.model.IPersistableSourceLocator;
 import org.eclipse.debug.core.model.ISourceLocator;
 import org.eclipse.debug.core.sourcelookup.IPersistableSourceLocator2;
@@ -176,29 +169,6 @@ public class GdbLaunchDelegate extends AbstractCLaunchDelegate
         } catch (ExecutionException e1) {
             throw new DebugException(new Status(IStatus.ERROR, GdbPlugin.PLUGIN_ID, DebugException.REQUEST_FAILED, "Error in final launch sequence", e1.getCause())); //$NON-NLS-1$
         }
-        // Create a memory retrieval and register it with session 
-        try {
-            launch.getDsfExecutor().submit( new Callable<Object>() {
-                public Object call() throws CoreException {
-                    DsfServicesTracker tracker = new DsfServicesTracker(GdbPlugin.getBundleContext(), launch.getSession().getId());
-                    GDBControl gdbControl = tracker.getService(GDBControl.class);
-                    if (gdbControl != null) {
-                        IMemoryBlockRetrieval memRetrieval = new DsfMemoryBlockRetrieval(
-                            GDB_DEBUG_MODEL_ID, config, launch.getSession());
-                        launch.getSession().registerModelAdapter(IMemoryBlockRetrieval.class, memRetrieval);
-                        ((DsfMemoryBlockRetrieval) memRetrieval).initialize((IMemoryDMContext)gdbControl.getControlDMContext());
-                    }
-                    tracker.dispose();
-                    return null;
-                }
-            }).get();
-        } catch (InterruptedException e) {
-            throw new CoreException(new Status(IStatus.ERROR, GdbPlugin.PLUGIN_ID, 0, "Interrupted while waiting for get process callable.", e)); //$NON-NLS-1$
-        } catch (ExecutionException e) {
-            throw (CoreException)e.getCause();
-        } catch (RejectedExecutionException e) {
-            throw new CoreException(new Status(IStatus.ERROR, GdbPlugin.PLUGIN_ID, 0, "Debugger shut down before launch was completed.", e)); //$NON-NLS-1$
-        }
 	}
 
 	// Copied from the CDT

plugins/org.eclipse.dd.gdb/src/org/eclipse/dd/gdb/internal/provisional/launching/ShutdownSequence.java

@@ -75,35 +75,6 @@ public class ShutdownSequence extends Sequence {
         public void execute(RequestMonitor requestMonitor) {
             shutdownService(MIRegisters.class, requestMonitor);
         }
-// TODO: As Pawel about the necessity of this step
-// Not clear on the purpose of this step since the next one does it also
-// (stopTrackingBreakpoints() is called as part of the shutdown method)
-// Besides, the run control is already gone so removing breakpoints from
-// the back-end is bound to fail...
-//    }, new Step() {
-//        @Override
-//        public void execute(final RequestMonitor requestMonitor) {
-//            MIBreakpointsManager bpm = fTracker.getService(MIBreakpointsManager.class);
-//            GDBControl commandControl = fTracker.getService(GDBControl.class);
-//            if (bpm != null && commandControl != null) {
-//            	bpm.stopTrackingBreakpoints(
-//                    commandControl.getGDBDMContext(), 
-//                    new RequestMonitor(getExecutor(), requestMonitor) {
-//                        @Override
-//                        protected void handleCompleted() {
-//                            // If un-installing breakpoints fails, log the error but continue shutting down.
-//                            if (!isSuccess) {
-//                                DsfGdbPlugin.getDefault().getLog().log(getStatus());
-//                            }
-//                            requestMonitor.done();
-//                        }
-//                    });
-//            } else {
-//                requestMonitor.setStatus(new Status(IStatus.ERROR, DsfGdbPlugin.PLUGIN_ID, IDsfStatusConstants.INTERNAL_ERROR,
-//                    "Needed services not found.", null)); //$NON-NLS-1$
-//                requestMonitor.done();
-//            }
-//        }
     }, new Step() {
         @Override
         public void execute(RequestMonitor requestMonitor) {