Bug 273630 - Errors in breakpoint conditions not handled properly if breakpoint listener votes to resume
diff --git a/org.eclipse.jdt.debug.tests/test plugin/org/eclipse/jdt/debug/testplugin/EvalualtionBreakpointListener.java b/org.eclipse.jdt.debug.tests/test plugin/org/eclipse/jdt/debug/testplugin/EvalualtionBreakpointListener.java
index 9dd75d7..bf33840 100644
--- a/org.eclipse.jdt.debug.tests/test plugin/org/eclipse/jdt/debug/testplugin/EvalualtionBreakpointListener.java
+++ b/org.eclipse.jdt.debug.tests/test plugin/org/eclipse/jdt/debug/testplugin/EvalualtionBreakpointListener.java
@@ -10,6 +10,9 @@
  *******************************************************************************/
 package org.eclipse.jdt.debug.testplugin;
 
+import java.util.ArrayList;
+import java.util.List;
+
 import junit.framework.AssertionFailedError;
 
 import org.eclipse.debug.core.DebugEvent;
@@ -41,6 +44,11 @@
 	public static int VOTE = IJavaBreakpointListener.DONT_CARE;
 	
 	/**
+	 * Whether hit
+	 */
+	public static boolean HIT = false;
+	
+	/**
 	 * Expression to evaluate when hit
 	 */
 	public static String EXPRESSION;
@@ -54,6 +62,26 @@
 	 * Evaluation result
 	 */
 	public static IEvaluationResult RESULT;
+	
+	/**
+	 * List of breakpoints with compilation errors
+	 */
+	public static List COMPILATION_ERRORS = new ArrayList();
+	
+	/**
+	 * List of breakpoints with runtime errors
+	 */
+	public static List RUNTIME_ERRORS = new ArrayList();
+	
+	public static void reset() {
+		VOTE = IJavaBreakpointListener.DONT_CARE;
+		EXPRESSION = null;
+		PROJECT = null;
+		RESULT = null;
+		HIT = false;
+		COMPILATION_ERRORS.clear();
+		RUNTIME_ERRORS.clear();
+	}
 
 	/**
 	 * Constructs a breakpoint listener to evaluate an expression when a breakpoint is hit.
@@ -71,18 +99,21 @@
 	 * @see org.eclipse.jdt.debug.core.IJavaBreakpointListener#breakpointHasCompilationErrors(org.eclipse.jdt.debug.core.IJavaLineBreakpoint, org.eclipse.jdt.core.dom.Message[])
 	 */
 	public void breakpointHasCompilationErrors(IJavaLineBreakpoint breakpoint, Message[] errors) {
+		COMPILATION_ERRORS.add(breakpoint);
 	}
 
 	/* (non-Javadoc)
 	 * @see org.eclipse.jdt.debug.core.IJavaBreakpointListener#breakpointHasRuntimeException(org.eclipse.jdt.debug.core.IJavaLineBreakpoint, org.eclipse.debug.core.DebugException)
 	 */
 	public void breakpointHasRuntimeException(IJavaLineBreakpoint breakpoint, DebugException exception) {
+		RUNTIME_ERRORS.add(breakpoint);
 	}
 
 	/* (non-Javadoc)
 	 * @see org.eclipse.jdt.debug.core.IJavaBreakpointListener#breakpointHit(org.eclipse.jdt.debug.core.IJavaThread, org.eclipse.jdt.debug.core.IJavaBreakpoint)
 	 */
 	public int breakpointHit(IJavaThread thread, IJavaBreakpoint breakpoint) {
+		HIT = true;
 		final Object lock = new Object();
 		RESULT = null;
 		IAstEvaluationEngine engine = EvaluationManager.newAstEvaluationEngine(PROJECT, (IJavaDebugTarget) thread.getDebugTarget());
diff --git a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/breakpoints/JavaBreakpointListenerTests.java b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/breakpoints/JavaBreakpointListenerTests.java
index 7a82a2b..fb787cc 100755
--- a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/breakpoints/JavaBreakpointListenerTests.java
+++ b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/breakpoints/JavaBreakpointListenerTests.java
@@ -159,14 +159,18 @@
 	class Collector implements IJavaBreakpointListener {
 		
 		public List HIT = new ArrayList();
+		public List COMPILATION_ERRORS = new ArrayList();
+		public List RUNTIME_ERRORS = new ArrayList();
 
 		public void addingBreakpoint(IJavaDebugTarget target, IJavaBreakpoint breakpoint) {
 		}
 
 		public void breakpointHasCompilationErrors( IJavaLineBreakpoint breakpoint, Message[] errors) {
+			COMPILATION_ERRORS.add(breakpoint);
 		}
 
 		public void breakpointHasRuntimeException(IJavaLineBreakpoint breakpoint, DebugException exception) {
+			RUNTIME_ERRORS.add(breakpoint);
 		}
 
 		public int breakpointHit(IJavaThread thread, IJavaBreakpoint breakpoint) {
@@ -746,6 +750,7 @@
 		String typeName = "HitCountLooper";
 		IJavaLineBreakpoint bp = createLineBreakpoint(16, typeName);
 		bp.addBreakpointListener("org.eclipse.jdt.debug.tests.evalListener");
+		EvalualtionBreakpointListener.reset();
 		EvalualtionBreakpointListener.PROJECT = getJavaProject();
 		EvalualtionBreakpointListener.EXPRESSION = "return new Integer(i);";
 		EvalualtionBreakpointListener.VOTE = IJavaBreakpointListener.SUSPEND;
@@ -814,6 +819,7 @@
 		// second breakpoint is where the evaluation is performed with a resume vote
 		IJavaLineBreakpoint second = createLineBreakpoint(29, typeName);
 		second.addBreakpointListener("org.eclipse.jdt.debug.tests.evalListener");
+		EvalualtionBreakpointListener.reset();
 		EvalualtionBreakpointListener.PROJECT = getJavaProject();
 		EvalualtionBreakpointListener.EXPRESSION = "return new Integer(sum);";
 		EvalualtionBreakpointListener.VOTE = IJavaBreakpointListener.DONT_SUSPEND;
@@ -866,6 +872,7 @@
 		// second breakpoint is where the evaluation is performed with a resume vote
 		IJavaLineBreakpoint second = createLineBreakpoint(29, typeName);
 		second.addBreakpointListener("org.eclipse.jdt.debug.tests.evalListener");
+		EvalualtionBreakpointListener.reset();
 		EvalualtionBreakpointListener.PROJECT = getJavaProject();
 		EvalualtionBreakpointListener.EXPRESSION = "return new Integer(sum);";
 		EvalualtionBreakpointListener.VOTE = IJavaBreakpointListener.SUSPEND;
@@ -917,6 +924,7 @@
 		IJavaLineBreakpoint first = createLineBreakpoint(19, typeName);
 		IJavaLineBreakpoint second = createLineBreakpoint(29, typeName);
 		second.addBreakpointListener("org.eclipse.jdt.debug.tests.evalListener");
+		EvalualtionBreakpointListener.reset();
 		EvalualtionBreakpointListener.PROJECT = getJavaProject();
 		EvalualtionBreakpointListener.EXPRESSION = "for (int x = 0; x < 1000; x++) { System.out.println(x);} Thread.sleep(200);";
 		EvalualtionBreakpointListener.VOTE = IJavaBreakpointListener.DONT_SUSPEND;
@@ -1030,6 +1038,56 @@
 			removeAllBreakpoints();
 		}
 	}	
+	
+	/**
+	 * Tests that breakpoint listeners are not notified of "hit" when condition has compilation
+	 * errors. Also they should be notified of the compilation errors.
+	 * 
+	 * @throws Exception
+	 */
+	public void testListenersOnCompilationError() throws Exception {
+		String typeName = "HitCountLooper";
+		IJavaLineBreakpoint bp = createConditionalLineBreakpoint(17, typeName, "x == 1", true);
+		bp.addBreakpointListener("org.eclipse.jdt.debug.tests.evalListener");
+		EvalualtionBreakpointListener.reset();
+			
+		IJavaThread thread= null;
+		try {
+			thread= launchToLineBreakpoint(typeName, bp);
+			assertFalse(EvalualtionBreakpointListener.HIT);
+			assertEquals(1, EvalualtionBreakpointListener.COMPILATION_ERRORS.size());
+			assertEquals(bp, EvalualtionBreakpointListener.COMPILATION_ERRORS.get(0));
+			assertEquals(0, EvalualtionBreakpointListener.RUNTIME_ERRORS.size());
+		} finally {
+			terminateAndRemove(thread);
+			removeAllBreakpoints();
+		}		
+	}	
+	
+	/**
+	 * Tests that breakpoint listeners are not notified of "hit" when condition has compilation
+	 * errors. Also they should be notified of the compilation errors.
+	 * 
+	 * @throws Exception
+	 */
+	public void testListenersOnRuntimeError() throws Exception {
+		String typeName = "HitCountLooper";
+		IJavaLineBreakpoint bp = createConditionalLineBreakpoint(17, typeName, "(new String()).charAt(34) == 'c'", true);
+		bp.addBreakpointListener("org.eclipse.jdt.debug.tests.evalListener");
+		EvalualtionBreakpointListener.reset();
+			
+		IJavaThread thread= null;
+		try {
+			thread= launchToLineBreakpoint(typeName, bp);
+			assertFalse(EvalualtionBreakpointListener.HIT);
+			assertEquals(1, EvalualtionBreakpointListener.RUNTIME_ERRORS.size());
+			assertEquals(bp, EvalualtionBreakpointListener.RUNTIME_ERRORS.get(0));
+			assertEquals(0, EvalualtionBreakpointListener.COMPILATION_ERRORS.size());
+		} finally {
+			terminateAndRemove(thread);
+			removeAllBreakpoints();
+		}		
+	}	
 
 	/**
 	 * Tests addition and removal of breakpoint listeners to a breakpoint.
diff --git a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/breakpoints/ConditionalBreakpointHandler.java b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/breakpoints/ConditionalBreakpointHandler.java
index 206d3b7..013f821 100644
--- a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/breakpoints/ConditionalBreakpointHandler.java
+++ b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/breakpoints/ConditionalBreakpointHandler.java
@@ -46,6 +46,11 @@
 public class ConditionalBreakpointHandler implements IJavaBreakpointListener {
 	
 	/**
+	 * Whether the condition had compile or runtime errors
+	 */
+	private boolean fHasErrors = false;
+	
+	/**
 	 * Listens for evaluation completion for condition evaluation.
 	 * If an evaluation evaluates <code>true</code> or has an error, this breakpoint
 	 * will suspend the thread in which the breakpoint was hit.
@@ -242,6 +247,7 @@
 	}	
 	
 	private void fireConditionHasRuntimeErrors(IJavaLineBreakpoint breakpoint, DebugException exception) {
+		fHasErrors = true;
 		JDIDebugPlugin.getDefault().fireBreakpointHasRuntimeException(breakpoint, exception);
 	}
 
@@ -250,6 +256,7 @@
 	 * compiled that contains errors
 	 */
 	private void fireConditionHasErrors(IJavaLineBreakpoint breakpoint, Message[] messages) {
+		fHasErrors = true;
 		JDIDebugPlugin.getDefault().fireBreakpointHasCompilationErrors(breakpoint, messages);
 	}
 	
@@ -264,5 +271,14 @@
 			messages[i]= new Message(errorMessages[i], -1);
 		}
 		return messages;
-	}	
+	}
+	
+	/**
+	 * Returns whether errors were encountered when evaluating the condition (compilation or runtime).
+	 * 
+	 * @return whether errors were encountered when evaluating the condition
+	 */
+	public boolean hasErrors() {
+		return fHasErrors;
+	}
 }
diff --git a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIThread.java b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIThread.java
index 0482a01..f63cd32 100644
--- a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIThread.java
+++ b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIThread.java
@@ -1109,6 +1109,14 @@
 						return false;
 					}
 				}
+				if (handler.hasErrors()) {
+					// there were errors, suspend and do not notify listeners of hit since
+					// they were already notified of compilation/runtime errors
+					synchronized (this) {
+						fSuspendVoteInProgress = false;
+						return true;
+					}
+				}
 			}
 		}