From a66ccee4f0c5356af2aef8dde72c613249d35d57 Mon Sep 17 00:00:00 2001
From: Marc Khouzam <marc.khouzam@gmail.com>
Date: Wed, 23 May 2012 16:02:47 -0400
Subject: [PATCH] Bug 379225: Address FindBugs issues for DSF-GDB

Change-Id: Id0bb91c7aaef0e356d1989e1dc949542813d2309
---
 .../dsf/mi/service/MIBreakpointDMData.java    | 14 ++++++++
 .../mi/service/command/AbstractMIControl.java | 34 ++++++++++---------
 2 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIBreakpointDMData.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIBreakpointDMData.java
index 8c8d6e36305..5f20ce05def 100644
--- a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIBreakpointDMData.java
+++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIBreakpointDMData.java
@@ -242,6 +242,20 @@ public class MIBreakpointDMData implements IBreakpointDMData {
 	public boolean equals(MIBreakpointDMData other) {
 		return (fNature == other.fNature) && (fProperties.equals(other.fProperties));
 	}
+
+	@Override
+	public boolean equals(Object other) {
+		if (this == other) return true;
+		if (!(other instanceof MIBreakpointDMData)) return false;
+		MIBreakpointDMData bpData = (MIBreakpointDMData)other;
+		
+		return (fNature == bpData.fNature) && (fProperties.equals(bpData.fProperties));
+	}
+	
+	@Override
+	public int hashCode() {
+		return fNature.hashCode() ^ fProperties.hashCode();
+	}
 	
 	///////////////////////////////////////////////////////////////////////////
 	// IBreakpointDMData
diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/AbstractMIControl.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/AbstractMIControl.java
index f6a012526c0..f1c34df49e4 100644
--- a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/AbstractMIControl.java
+++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/AbstractMIControl.java
@@ -278,20 +278,13 @@ public abstract class AbstractMIControl extends AbstractDsfService
             commandHandle.getRequestMonitor().setStatus(genStatus("Connection is shut down")); //$NON-NLS-1$
             commandHandle.getRequestMonitor().done();
         }
-    	fCommandQueue.clear();
+    	fCommandQueue.clear();   
     	
     	/*
     	 *  Now go through the commands which are outstanding in that they have been sent to the backend.
     	 */
-        synchronized(fRxCommands) {
-            for (CommandHandle commandHandle : fRxCommands.values()) {
-                if (commandHandle.getRequestMonitor() == null) continue;
-                commandHandle.getRequestMonitor().setStatus(genStatus( "Connection is shut down")); //$NON-NLS-1$
-                commandHandle.getRequestMonitor().done();
-            }
-            fRxCommands.clear();
-        }
-        
+    	cancelRxCommands();
+    	
         /*
          *  Now handle any requests which have not been transmitted, but weconsider them handed off.
          */
@@ -307,6 +300,15 @@ public abstract class AbstractMIControl extends AbstractDsfService
         fTxCommands.add(fTerminatorHandle);
     }
     
+    private synchronized void cancelRxCommands() {
+        for (CommandHandle commandHandle : fRxCommands.values()) {
+            if (commandHandle.getRequestMonitor() == null) continue;
+            commandHandle.getRequestMonitor().setStatus(genStatus( "Connection is shut down")); //$NON-NLS-1$
+            commandHandle.getRequestMonitor().done();
+        }
+        fRxCommands.clear();
+    }
+    
     /**
      * Queues the given MI command to be sent to the debugger back end.  
      * 
@@ -621,10 +623,6 @@ public abstract class AbstractMIControl extends AbstractDsfService
             while (true) {
                 CommandHandle commandHandle = null;
                 
-                /*
-                 *   Note: Acquiring locks for both fRxCommands and fTxCommands collections. 
-                 */
-                synchronized(fTxCommands) {
                     try {
                         commandHandle = fTxCommands.take();
                     } catch (InterruptedException e) {
@@ -632,7 +630,12 @@ public abstract class AbstractMIControl extends AbstractDsfService
                     }
         
                     if (commandHandle == fTerminatorHandle) {
-                        
+                    	// There is a small possibility that a new command was inserted
+                    	// in the fRxCommands map after we cleared that map.
+                    	// Just to be safe, clear it again.
+                    	// We do this to avoid synchronizing the handling of fRxCommands
+                    	// because this is more efficient, as it happens only once at shutdown.
+                    	cancelRxCommands();
                         break; // Null command is an indicator that we're shutting down. 
                     }
                     
@@ -643,7 +646,6 @@ public abstract class AbstractMIControl extends AbstractDsfService
                     	// RawCommands will not get an answer, so we cannot put them in the receive queue.
                     	fRxCommands.put(commandHandle.getTokenId(), commandHandle);
                     }
-                }
                 
                 /*
                  *   Construct the new command and push this command out the pipeline.