Clean up breakpoints after marker loss

org.eclipse.jdt.debug.tests/test plugin/org/eclipse/jdt/debug/testplugin/GlobalBreakpointListener.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2009, 2015 IBM Corporation and others.
+ * Copyright (c) 2009, 2026 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -13,6 +13,8 @@
  *******************************************************************************/
 package org.eclipse.jdt.debug.testplugin;
 
+import java.util.Collections;
+import java.util.IdentityHashMap;
 import java.util.LinkedHashSet;
 import java.util.Set;
 
@@ -33,7 +35,8 @@ public class GlobalBreakpointListener implements IJavaBreakpointListener {
 	public static Set<IJavaBreakpoint> ADDED = new LinkedHashSet<>();
 	public static Set<IJavaBreakpoint> HIT = new LinkedHashSet<>();
 	public static Set<IJavaBreakpoint> INSTALLED = new LinkedHashSet<>();
-	public static Set<IJavaBreakpoint> REMOVED = new LinkedHashSet<>();
+	// Removed breakpoints may no longer have a marker backing equals/hashCode.
+	public static Set<IJavaBreakpoint> REMOVED = Collections.synchronizedSet(Collections.newSetFromMap(new IdentityHashMap<>()));
 	public static Set<IJavaBreakpoint> INSTALLING = new LinkedHashSet<>();
 
 	public static void clear() {

org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/AutomatedSuite.java

@@ -24,6 +24,7 @@
 import org.eclipse.jdt.debug.testplugin.JavaProjectHelper;
 import org.eclipse.jdt.debug.tests.breakpoints.BreakpointListenerTests;
 import org.eclipse.jdt.debug.tests.breakpoints.BreakpointLocationVerificationTests;
+import org.eclipse.jdt.debug.tests.breakpoints.BreakpointRemovalTests;
 import org.eclipse.jdt.debug.tests.breakpoints.BreakpointWorkingSetTests;
 import org.eclipse.jdt.debug.tests.breakpoints.ConditionalBreakpointsInJava8Tests;
 import org.eclipse.jdt.debug.tests.breakpoints.ConditionalBreakpointsTests;
@@ -417,6 +418,7 @@ public AutomatedSuite() {
 		addTest(new TestSuite(TargetPatternBreakpointTests.class));
 		addTest(new TestSuite(BreakpointListenerTests.class));
 		addTest(new TestSuite(JavaBreakpointListenerTests.class));
+		addTest(new TestSuite(BreakpointRemovalTests.class));
 		addTest(new TestSuite(MiscBreakpointsTests.class));
 		addTest(new TestSuite(ThreadNameChangeTests.class));
 		addTest(new TestSuite(BreakpointLocationVerificationTests.class));

org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/breakpoints/JavaBreakpoint.java

@@ -286,7 +286,7 @@ protected void deregisterRequest(EventRequest request, JDIDebugTarget target)
 		// deletion.
 		// Don't try updating the marker (decrementing the install count) if
 		// it no longer exists.
-		if (!(request instanceof ClassPrepareRequest) && getMarker().exists()) {
+		if (!(request instanceof ClassPrepareRequest) && markerExists()) {
 			decrementInstallCount();
 		}
 	}
@@ -745,32 +745,49 @@ protected boolean hasHitCountChanged(EventRequest request)
 	 */
 	public void removeFromTarget(final JDIDebugTarget target)
 			throws CoreException {
-		removeRequests(target);
-		Object removed = fFilteredThreadsByTarget.remove(target);
-		boolean changed = removed != null;
-		boolean markerExists = markerExists();
-		if (!markerExists || (markerExists && getInstallCount() == 0)) {
-			fInstalledTypeName = null;
-		}
-
-		// remove instance filters
-		if (fInstanceFilters != null && !fInstanceFilters.isEmpty()) {
-			for (int i = 0; i < fInstanceFilters.size(); i++) {
-				IJavaObject object = fInstanceFilters.get(i);
-				if (object.getDebugTarget().equals(target)) {
-					fInstanceFilters.remove(i);
-					changed = true;
+		Throwable failure = null;
+		try {
+			removeRequests(target);
+		} catch (CoreException | RuntimeException | Error e) {
+			failure = e;
+			throw e;
+		} finally {
+			try {
+				// Release the breakpoint's side of the association even when
+				// request deregistration fails. Do this before reading the marker.
+				boolean changed = fFilteredThreadsByTarget.remove(target) != null;
+				if (fInstanceFilters != null) {
+					changed |= fInstanceFilters.removeIf(object -> object.getDebugTarget().equals(target));
+				}
+				if (!markerExists() || getInstallCount() == 0) {
+					fInstalledTypeName = null;
+				}
+				if (changed) {
+					fireChanged();
+				}
+			} catch (CoreException | RuntimeException | Error cleanupFailure) {
+				if (failure == null) {
+					failure = cleanupFailure;
+					throw cleanupFailure;
+				}
+				if (failure != cleanupFailure) {
+					failure.addSuppressed(cleanupFailure);
+				}
+			} finally {
+				// A marker read or change notification must not prevent removal
+				// notification or replace the original deregistration failure.
+				try {
+					fireRemoved(target);
+				} catch (RuntimeException | Error notificationFailure) {
+					if (failure == null) {
+						throw notificationFailure;
+					}
+					if (failure != notificationFailure) {
+						failure.addSuppressed(notificationFailure);
+					}
 				}
 			}
 		}
-
-		// fire change notification if required
-		if (changed) {
-			fireChanged();
-		}
-
-		// notification
-		fireRemoved(target);
 	}
 
 	/**
@@ -787,6 +804,7 @@ protected void removeRequests(final JDIDebugTarget target) throws CoreException
 		// ConcurrentModificationException
 		Iterator<EventRequest> iter = requests.iterator();
 		EventRequest req;
+		CoreException failure = null;
 		while (iter.hasNext()) {
 			req = iter.next();
 			try {
@@ -804,10 +822,22 @@ protected void removeRequests(final JDIDebugTarget target) throws CoreException
 			} catch (RuntimeException e) {
 				target.internalError(e);
 			} finally {
-				deregisterRequest(req, target);
+				try {
+					deregisterRequest(req, target);
+				} catch (CoreException e) {
+					// A marker update failure must not leave the other VM requests active.
+					if (failure == null) {
+						failure = e;
+					} else if (failure != e) {
+						failure.addSuppressed(e);
+					}
+				}
 			}
 		}
 		fRequestsByTarget.remove(target);
+		if (failure != null) {
+			throw failure;
+		}
 	}
 
 	/**
@@ -1068,10 +1098,11 @@ protected void fireAdding(IJavaDebugTarget target) {
 	 *            debug target
 	 */
 	protected void fireRemoved(IJavaDebugTarget target) {
+		// State cleanup must not depend on the plugin or its notification.
+		setInstalledIn(target, false);
 		JDIDebugPlugin plugin = JDIDebugPlugin.getDefault();
 		if (plugin != null) {
 			plugin.fireBreakpointRemoved(target, this);
-			setInstalledIn(target, false);
 		}
 	}
 

org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/breakpoints/JavaClassPrepareBreakpoint.java

@@ -146,7 +146,7 @@ protected void deregisterRequest(EventRequest request, JDIDebugTarget target)
 		// deletion.
 		// Don't try updating the marker (decrementing the install count) if
 		// it no longer exists.
-		if (getMarker().exists()) {
+		if (markerExists()) {
 			decrementInstallCount();
 		}
 	}

org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIDebugTarget.java

@@ -1629,17 +1629,30 @@ public void breakpointRemoved(IBreakpoint breakpoint, IMarkerDelta delta) {
 		if (!isAvailable()) {
 			return;
 		}
-		if (supportsBreakpoint(breakpoint)) {
+		// A removal notification can arrive after the marker is no longer
+		// available. Use the target's installed state instead of marker data.
+		boolean installed = false;
+		synchronized (fBreakpoints) {
+			for (IBreakpoint installedBreakpoint : fBreakpoints) {
+				if (installedBreakpoint == breakpoint) {
+					installed = true;
+					break;
+				}
+			}
+		}
+		if (installed) {
 			try {
 				((JavaBreakpoint) breakpoint).removeFromTarget(this);
-				getBreakpoints().remove(breakpoint);
+			} catch (CoreException e) {
+				logError(e);
+			} finally {
+				synchronized (fBreakpoints) {
+					fBreakpoints.removeIf(installedBreakpoint -> installedBreakpoint == breakpoint);
+				}
 				Iterator<JDIThread> threads = getThreadIterator();
 				while (threads.hasNext()) {
-					threads.next()
-							.removeCurrentBreakpoint(breakpoint);
+					threads.next().removeCurrentBreakpoint(breakpoint);
 				}
-			} catch (CoreException e) {
-				logError(e);
 			}
 		}
 	}

org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIThread.java

@@ -476,7 +476,7 @@ protected void addCurrentBreakpoint(IBreakpoint bp) {
 	 */
 	protected void removeCurrentBreakpoint(IBreakpoint bp) {
 		synchronized (breakpointAcessLock) {
-			fCurrentBreakpoints.remove(bp);
+			fCurrentBreakpoints.removeIf(breakpoint -> breakpoint == bp);
 		}
 	}