diff --git a/org.eclipse.jdt.debug.tests/test plugin/org/eclipse/jdt/debug/testplugin/GlobalBreakpointListener.java b/org.eclipse.jdt.debug.tests/test plugin/org/eclipse/jdt/debug/testplugin/GlobalBreakpointListener.java index 2740a50b80..c88e68ed51 100644 --- a/org.eclipse.jdt.debug.tests/test plugin/org/eclipse/jdt/debug/testplugin/GlobalBreakpointListener.java +++ b/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 ADDED = new LinkedHashSet<>(); public static Set HIT = new LinkedHashSet<>(); public static Set INSTALLED = new LinkedHashSet<>(); - public static Set REMOVED = new LinkedHashSet<>(); + // Removed breakpoints may no longer have a marker backing equals/hashCode. + public static Set REMOVED = Collections.synchronizedSet(Collections.newSetFromMap(new IdentityHashMap<>())); public static Set INSTALLING = new LinkedHashSet<>(); public static void clear() { diff --git a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/AutomatedSuite.java b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/AutomatedSuite.java index 017d6a4242..8b9df362c8 100644 --- a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/AutomatedSuite.java +++ b/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)); diff --git a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/breakpoints/BreakpointRemovalTests.java b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/breakpoints/BreakpointRemovalTests.java new file mode 100644 index 0000000000..e1c848efdd --- /dev/null +++ b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/breakpoints/BreakpointRemovalTests.java @@ -0,0 +1,415 @@ +/******************************************************************************* + * Copyright (c) 2026 Carsten Hammer and others. + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Carsten Hammer - initial API and implementation + *******************************************************************************/ +package org.eclipse.jdt.debug.tests.breakpoints; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.concurrent.TimeUnit; + +import org.eclipse.core.resources.IMarker; +import org.eclipse.core.resources.IResource; +import org.eclipse.core.resources.IWorkspaceRunnable; +import org.eclipse.core.resources.IncrementalProjectBuilder; +import org.eclipse.core.resources.ResourcesPlugin; +import org.eclipse.core.runtime.CoreException; +import org.eclipse.core.runtime.ILogListener; +import org.eclipse.core.runtime.IStatus; +import org.eclipse.core.runtime.Platform; +import org.eclipse.core.runtime.Status; +import org.eclipse.debug.core.DebugException; +import org.eclipse.debug.core.model.IBreakpoint; +import org.eclipse.jdt.debug.core.IJavaBreakpoint; +import org.eclipse.jdt.debug.core.IJavaBreakpointListener; +import org.eclipse.jdt.debug.core.IJavaClassPrepareBreakpoint; +import org.eclipse.jdt.debug.core.IJavaLineBreakpoint; +import org.eclipse.jdt.debug.core.IJavaObject; +import org.eclipse.jdt.debug.core.IJavaThread; +import org.eclipse.jdt.debug.core.JDIDebugModel; +import org.eclipse.jdt.debug.testplugin.EvalualtionBreakpointListener; +import org.eclipse.jdt.debug.testplugin.GlobalBreakpointListener; +import org.eclipse.jdt.debug.tests.AbstractDebugTest; +import org.eclipse.jdt.internal.debug.core.breakpoints.JavaLineBreakpoint; +import org.eclipse.jdt.internal.debug.core.model.JDIDebugTarget; + +import com.sun.jdi.request.BreakpointRequest; +import com.sun.jdi.request.EventRequest; +import com.sun.jdi.request.EventRequestManager; + +/** + * Tests marker-independent breakpoint removal and cleanup after a request + * deregistration failure, including both sides of the installation, target + * filters, listener notifications and real VM requests. + */ +public class BreakpointRemovalTests extends AbstractDebugTest { + + private static final String TEST_LISTENER = "org.eclipse.jdt.debug.tests.evalListener"; //$NON-NLS-1$ + + public BreakpointRemovalTests(String name) { + super(name); + } + + /** + * Tests the complete marker-deletion lifecycle while a debug target is + * suspended at the deleted breakpoint. + */ + public void testMarkerDeletionCleansTargetAndAllowsReplacementBreakpoint() throws Exception { + String typeName = "HitCountLooper"; //$NON-NLS-1$ + int loopLine = 19; + IJavaLineBreakpoint breakpoint = createLineBreakpoint(loopLine, typeName); + breakpoint.addBreakpointListener(TEST_LISTENER); + EvalualtionBreakpointListener.reset(); + EvalualtionBreakpointListener.VOTE = IJavaBreakpointListener.SUSPEND; + + IJavaThread thread = null; + try { + thread = launchToLineBreakpoint(typeName, breakpoint); + assertNotNull("Breakpoint was not hit", thread); //$NON-NLS-1$ + JDIDebugTarget target = (JDIDebugTarget) thread.getDebugTarget(); + assertTrue("Breakpoint should be tracked by the debug target", containsByIdentity(target.getBreakpoints(), breakpoint)); //$NON-NLS-1$ + assertTrue("Breakpoint should be current in the suspended thread", containsByIdentity(thread.getBreakpoints(), breakpoint)); //$NON-NLS-1$ + + try (RemovalLog log = new RemovalLog()) { + IWorkspaceRunnable runnable = monitor -> { + breakpoint.getMarker().delete(); + get14Project().getProject().build(IncrementalProjectBuilder.INCREMENTAL_BUILD, monitor); + }; + ResourcesPlugin.getWorkspace().run(runnable, null); + + waitForRemovalNotification(); + assertTrue("Breakpoint removal listener was not notified", EvalualtionBreakpointListener.REMOVED); //$NON-NLS-1$ + waitForCleanup(target, thread, breakpoint); + assertFalse("Deleted breakpoint remained in the debug target", containsByIdentity(target.getBreakpoints(), breakpoint)); //$NON-NLS-1$ + assertFalse("Deleted breakpoint remained current in the suspended thread", containsByIdentity(thread.getBreakpoints(), breakpoint)); //$NON-NLS-1$ + log.assertNoErrors(); + + IJavaLineBreakpoint replacement = createLineBreakpoint(loopLine, typeName); + assertTrue("Replacement breakpoint was not installed in the debug target", containsByIdentity(target.getBreakpoints(), replacement)); //$NON-NLS-1$ + thread = resumeToLineBreakpoint(thread, replacement); + assertNotNull("Replacement breakpoint was not hit", thread); //$NON-NLS-1$ + assertTrue("Replacement breakpoint should be current in the suspended thread", containsByIdentity(thread.getBreakpoints(), replacement)); //$NON-NLS-1$ + assertFalse("Deleted breakpoint became current again", containsByIdentity(thread.getBreakpoints(), breakpoint)); //$NON-NLS-1$ + log.assertNoErrors(); + } + } finally { + terminateAndRemove(thread); + removeAllBreakpoints(); + } + } + + /** + * Tests target-side cleanup when a removal notification carries a line + * breakpoint whose marker association has already been cleared. + */ + public void testLineBreakpointRemovalAfterMarkerIsDetached() throws Exception { + String typeName = "HitCountLooper"; //$NON-NLS-1$ + IJavaLineBreakpoint breakpoint = createLineBreakpoint(17, typeName); + breakpoint.addBreakpointListener(TEST_LISTENER); + EvalualtionBreakpointListener.reset(); + + IJavaThread thread = null; + IMarker marker = null; + try { + thread = launchToLineBreakpoint(typeName, breakpoint); + assertNotNull("Breakpoint was not hit", thread); //$NON-NLS-1$ + JDIDebugTarget target = (JDIDebugTarget) thread.getDebugTarget(); + assertTrue("Breakpoint should be tracked by the debug target", containsByIdentity(target.getBreakpoints(), breakpoint)); //$NON-NLS-1$ + assertTrue("Breakpoint should be current in the suspended thread", containsByIdentity(thread.getBreakpoints(), breakpoint)); //$NON-NLS-1$ + + marker = detachMarker(breakpoint); + try (RemovalLog log = new RemovalLog()) { + target.breakpointRemoved(breakpoint, null); + + assertTrue("Breakpoint removal listener was not notified", EvalualtionBreakpointListener.REMOVED); //$NON-NLS-1$ + assertFalse("Breakpoint without marker remained in the debug target", containsByIdentity(target.getBreakpoints(), breakpoint)); //$NON-NLS-1$ + assertFalse("Breakpoint without marker remained current in the suspended thread", containsByIdentity(thread.getBreakpoints(), breakpoint)); //$NON-NLS-1$ + log.assertNoErrors(); + assertTrue("Global listener did not record removal", GlobalBreakpointListener.REMOVED.contains(breakpoint)); //$NON-NLS-1$ + } + } finally { + restoreMarker(breakpoint, marker); + terminateAndRemove(thread); + removeAllBreakpoints(); + } + } + + /** + * Tests the class-prepare-specific request cleanup when the breakpoint marker + * association has already been cleared. + */ + public void testClassPrepareBreakpointRemovalAfterMarkerIsDetached() throws Exception { + String typeName = "HitCountLooper"; //$NON-NLS-1$ + IJavaClassPrepareBreakpoint breakpoint = createClassPrepareBreakpoint("DropTests"); //$NON-NLS-1$ + breakpoint.addBreakpointListener(TEST_LISTENER); + IJavaLineBreakpoint launchBreakpoint = createLineBreakpoint(17, typeName); + EvalualtionBreakpointListener.reset(); + + IJavaThread thread = null; + IMarker marker = null; + try { + thread = launchToLineBreakpoint(typeName, launchBreakpoint); + assertNotNull("Launch breakpoint was not hit", thread); //$NON-NLS-1$ + JDIDebugTarget target = (JDIDebugTarget) thread.getDebugTarget(); + assertTrue("Class prepare breakpoint should be tracked by the debug target", containsByIdentity(target.getBreakpoints(), breakpoint)); //$NON-NLS-1$ + + marker = detachMarker(breakpoint); + try (RemovalLog log = new RemovalLog()) { + target.breakpointRemoved(breakpoint, null); + + assertTrue("Breakpoint removal listener was not notified", EvalualtionBreakpointListener.REMOVED); //$NON-NLS-1$ + assertFalse("Class prepare breakpoint without marker remained in the debug target", containsByIdentity(target.getBreakpoints(), breakpoint)); //$NON-NLS-1$ + log.assertNoErrors(); + assertTrue("Global listener did not record removal", GlobalBreakpointListener.REMOVED.contains(breakpoint)); //$NON-NLS-1$ + } + } finally { + restoreMarker(breakpoint, marker); + terminateAndRemove(thread); + removeAllBreakpoints(); + } + } + + /** + * A marker update failure while deregistering one request must not leave + * subsequent requests active in the VM. Both the target and the breakpoint + * must release their per-target state and notify removal, so the same + * breakpoint can be installed and hit again. + */ + public void testCleanupContinuesAfterRequestDeregistrationFailure() throws Exception { + assertCleanupContinuesAfterRequestDeregistrationFailure(false); + } + + /** A later notification failure must not hide the original removal failure. */ + public void testRemovalPreservesFailureWhenChangeNotificationFails() throws Exception { + assertCleanupContinuesAfterRequestDeregistrationFailure(true); + } + + private void assertCleanupContinuesAfterRequestDeregistrationFailure(boolean failChangeNotification) throws Exception { + String typeName = "HitCountLooper"; //$NON-NLS-1$ + FailingLineBreakpoint breakpoint = new FailingLineBreakpoint(getType(typeName).getResource(), typeName); + getBreakpointManager().addBreakpoint(breakpoint); + IJavaThread thread = null; + try { + thread = launchToLineBreakpoint(typeName, breakpoint); + JDIDebugTarget target = (JDIDebugTarget) thread.getDebugTarget(); + EventRequestManager manager = target.getEventRequestManager(); + // Use the public filter API and real objects from the suspended VM. + // Two adjacent instance filters catch incomplete list removal as well. + breakpoint.setThreadFilter(thread); + breakpoint.addInstanceFilter(thread.getThreadObject()); + breakpoint.addInstanceFilter((IJavaObject) target.newValue("cleanup filter")); //$NON-NLS-1$ + assertSame(thread, breakpoint.getThreadFilter(target)); + assertEquals(2, breakpoint.getInstanceFilters().length); + assertTrue("Breakpoint should remember its installed target", breakpoint.installedIn(target)); //$NON-NLS-1$ + assertTrue("Initial install notification was not delivered", GlobalBreakpointListener.INSTALLED.contains(breakpoint)); //$NON-NLS-1$ + GlobalBreakpointListener.REMOVED.remove(breakpoint); + breakpoint.addDuplicateRequest(target); + List installedRequests = breakpoint.requestsIn(target); + assertTrue("Test needs multiple real JDI requests", installedRequests.size() >= 2); //$NON-NLS-1$ + assertTrue("Breakpoint must be current before removal", containsByIdentity(thread.getBreakpoints(), breakpoint)); //$NON-NLS-1$ + + breakpoint.failuresRemaining = 2; + RuntimeException changeFailure = failChangeNotification ? new IllegalStateException("Injected change notification failure") : null; //$NON-NLS-1$ + breakpoint.changeFailure = changeFailure; + try (RemovalLog log = new RemovalLog()) { + target.breakpointRemoved(breakpoint, null); + assertFalse("Target reference survived failed removal", containsByIdentity(target.getBreakpoints(), breakpoint)); //$NON-NLS-1$ + assertFalse("Thread reference survived failed removal", containsByIdentity(thread.getBreakpoints(), breakpoint)); //$NON-NLS-1$ + for (EventRequest request : installedRequests) { + assertFalse("A breakpoint request survived failed removal", manager.breakpointRequests().contains(request)); //$NON-NLS-1$ + assertFalse("A class-prepare request survived failed removal", manager.classPrepareRequests().contains(request)); //$NON-NLS-1$ + } + assertTrue("Breakpoint retained removed requests", breakpoint.requestsIn(target).isEmpty()); //$NON-NLS-1$ + assertFalse("Breakpoint retained its installed target", breakpoint.installedIn(target)); //$NON-NLS-1$ + assertNull("Breakpoint retained its thread filter", breakpoint.getThreadFilter(target)); //$NON-NLS-1$ + assertEquals("Breakpoint retained instance filters", 0, breakpoint.getInstanceFilters().length); //$NON-NLS-1$ + assertTrue("Removal notification was not delivered after deregistration failure", GlobalBreakpointListener.REMOVED.contains(breakpoint)); //$NON-NLS-1$ + assertEquals("Cleanup stopped at the first failure", 2, breakpoint.failures.size()); //$NON-NLS-1$ + CoreException firstFailure = breakpoint.failures.get(0); + assertEquals("Additional failure was lost", failChangeNotification ? 2 : 1, firstFailure.getSuppressed().length); //$NON-NLS-1$ + assertSame(breakpoint.failures.get(1), firstFailure.getSuppressed()[0]); + if (failChangeNotification) { + assertSame(changeFailure, firstFailure.getSuppressed()[1]); + } + log.assertOnly(firstFailure); + + // Repeated notifications must be harmless and must not repeat cleanup. + GlobalBreakpointListener.REMOVED.remove(breakpoint); + target.breakpointRemoved(breakpoint, null); + assertFalse("Repeated removal notified the listener again", GlobalBreakpointListener.REMOVED.contains(breakpoint)); //$NON-NLS-1$ + log.assertOnly(firstFailure); + + // Reuse the same object: a fresh breakpoint cannot expose stale + // fInstalledTargets state suppressing the next install notification. + breakpoint.failuresRemaining = 0; + breakpoint.changeFailure = null; + GlobalBreakpointListener.INSTALLED.remove(breakpoint); + target.breakpointAdded(breakpoint); + assertTrue("The same breakpoint was not tracked again", containsByIdentity(target.getBreakpoints(), breakpoint)); //$NON-NLS-1$ + assertTrue("The same breakpoint was not marked installed again", breakpoint.installedIn(target)); //$NON-NLS-1$ + assertTrue("Reinstalling the same breakpoint did not notify the listener", GlobalBreakpointListener.INSTALLED.contains(breakpoint)); //$NON-NLS-1$ + thread = resumeToLineBreakpoint(thread, breakpoint); + assertTrue("The same breakpoint was not hit after reinstallation", containsByIdentity(thread.getBreakpoints(), breakpoint)); //$NON-NLS-1$ + log.assertOnly(firstFailure); + } + } finally { + breakpoint.failuresRemaining = 0; + breakpoint.changeFailure = null; + try { + terminateAndRemove(thread); + } finally { + removeAllBreakpoints(); + } + } + } + + /** Injects a marker-update failure only after real request deregistration. */ + private static final class FailingLineBreakpoint extends JavaLineBreakpoint { + int failuresRemaining; + RuntimeException changeFailure; + final List failures = new ArrayList<>(); + + FailingLineBreakpoint(IResource resource, String typeName) throws DebugException { + super(resource, typeName, 19, -1, -1, 0, false, new HashMap<>()); + } + + boolean installedIn(JDIDebugTarget target) { + return isInstalledIn(target); + } + + @Override + protected void fireChanged() { + if (changeFailure != null) { + throw changeFailure; + } + super.fireChanged(); + } + + List requestsIn(JDIDebugTarget target) { + return new ArrayList<>(getRequests(target)); + } + + void addDuplicateRequest(JDIDebugTarget target) throws CoreException { + for (EventRequest request : requestsIn(target)) { + if (request instanceof BreakpointRequest installed) { + BreakpointRequest additional = target.getEventRequestManager().createBreakpointRequest(installed.location()); + configureRequest(additional, target); + registerRequest(additional, target); + return; + } + } + fail("No real breakpoint request was installed"); //$NON-NLS-1$ + } + + @Override + protected void deregisterRequest(EventRequest request, JDIDebugTarget target) throws CoreException { + super.deregisterRequest(request, target); + if (failuresRemaining > 0) { + failuresRemaining--; + CoreException failure = new CoreException(new Status(IStatus.ERROR, JDIDebugModel.getPluginIdentifier(), + "Injected request deregistration failure")); //$NON-NLS-1$ + failures.add(failure); + throw failure; + } + } + } + + private IMarker detachMarker(IJavaBreakpoint breakpoint) { + IMarker marker = breakpoint.getMarker(); + try { + breakpoint.setMarker(null); + } catch (CoreException e) { + // Configuring the breakpoint cannot complete after the marker was detached. + } + assertNull("Breakpoint marker should be detached", breakpoint.getMarker()); //$NON-NLS-1$ + return marker; + } + + private static void restoreMarker(IJavaBreakpoint breakpoint, IMarker marker) throws CoreException { + if (breakpoint.getMarker() == null && marker != null) { + breakpoint.setMarker(marker); + } + } + + private static void waitForRemovalNotification() throws InterruptedException { + long deadline = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(DEFAULT_TIMEOUT); + synchronized (EvalualtionBreakpointListener.REMOVE_LOCK) { + while (!EvalualtionBreakpointListener.REMOVED) { + long remaining = deadline - System.nanoTime(); + if (remaining <= 0) { + return; + } + TimeUnit.NANOSECONDS.timedWait(EvalualtionBreakpointListener.REMOVE_LOCK, remaining); + } + } + } + + private static void waitForCleanup(JDIDebugTarget target, IJavaThread thread, IBreakpoint breakpoint) throws InterruptedException { + long deadline = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(DEFAULT_TIMEOUT); + while (containsByIdentity(target.getBreakpoints(), breakpoint) || containsByIdentity(thread.getBreakpoints(), breakpoint)) { + if (System.nanoTime() - deadline >= 0) { + return; + } + Thread.sleep(10); + } + } + + private static boolean containsByIdentity(IBreakpoint[] breakpoints, IBreakpoint expected) { + for (IBreakpoint breakpoint : breakpoints) { + if (breakpoint == expected) { + return true; + } + } + return false; + } + + private static boolean containsByIdentity(List breakpoints, IBreakpoint expected) { + synchronized (breakpoints) { + for (IBreakpoint breakpoint : breakpoints) { + if (breakpoint == expected) { + return true; + } + } + } + return false; + } + + /** Collects all ERROR statuses only while the removal under test runs. */ + private static final class RemovalLog implements AutoCloseable { + private final List errors = Collections.synchronizedList(new ArrayList<>()); + private final ILogListener listener = (status, _) -> { + if (status.matches(IStatus.ERROR)) { + errors.add(status); + } + }; + + RemovalLog() { + Platform.addLogListener(listener); + } + + void assertNoErrors() { + assertTrue("Unexpected errors during breakpoint removal: " + errors, errors.isEmpty()); //$NON-NLS-1$ + } + + void assertOnly(CoreException expected) { + assertEquals("Unexpected errors during breakpoint removal: " + errors, 1, errors.size()); //$NON-NLS-1$ + assertSame("The original cleanup failure must be reported", expected, errors.get(0).getException()); //$NON-NLS-1$ + } + + @Override + public void close() { + Platform.removeLogListener(listener); + } + } +} diff --git a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/breakpoints/JavaBreakpoint.java b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/breakpoints/JavaBreakpoint.java index ff8f874d7a..b1aecd0a37 100644 --- a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/breakpoints/JavaBreakpoint.java +++ b/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 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); } } diff --git a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/breakpoints/JavaClassPrepareBreakpoint.java b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/breakpoints/JavaClassPrepareBreakpoint.java index adc58681de..d11542bf09 100644 --- a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/breakpoints/JavaClassPrepareBreakpoint.java +++ b/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(); } } diff --git a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIDebugTarget.java b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIDebugTarget.java index e3cdcf7f02..f242fb1d90 100644 --- a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIDebugTarget.java +++ b/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 threads = getThreadIterator(); while (threads.hasNext()) { - threads.next() - .removeCurrentBreakpoint(breakpoint); + threads.next().removeCurrentBreakpoint(breakpoint); } - } catch (CoreException e) { - logError(e); } } } 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 7513927df3..80df47c3d8 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 @@ -476,7 +476,7 @@ protected void addCurrentBreakpoint(IBreakpoint bp) { */ protected void removeCurrentBreakpoint(IBreakpoint bp) { synchronized (breakpointAcessLock) { - fCurrentBreakpoints.remove(bp); + fCurrentBreakpoints.removeIf(breakpoint -> breakpoint == bp); } }