Bug 540243 - Bad selection when opening Debug View on breakpoint
When the Debug View is not open and a breakpoint is hit, a preference
dictates whether the Debug View is opened. If this preference is used,
it is possible to observe a bad selection in the Debug View. E.g. a
thread is selected and not a stack frame, or a stack frame and a thread
are selected.
The problem is seen for Java debug targets, when owned monitors are
displayed in the Debug View. When the Debug View debug target element is
created, DebugTargetProxy.doInstalled() is called. This will retrieve
the first suspended thread and expand it, if such a thread is
available. The expand delta contains also the top stack frame at which
the thread is suspended. This stack frame is added to the delta with
index 0. Unfortunately, owned monitors are displayed before stack frames
in the list of thread children. So as soon as the children of the thread
are updated for the tree view, a thread owned monitor will replace the
selected stack frame. This results in an invalid selection handling,
which in turn breaks the selection.
This change ensures JavaDebugTargetProxy.getNextSuspendedThreadDelta()
also handles existing monitors for the thread. The top suspended stack
frame now has index that is greater than 0, if the thread owns monitors.
Change-Id: I27be0857844e8faa98f21714512534cc963d5e0b
Signed-off-by: Simeon Andreev <simeon.danailov.andreev@gmail.com>
diff --git a/org.eclipse.jdt.debug.tests/testprograms/Bug540243.java b/org.eclipse.jdt.debug.tests/testprograms/Bug540243.java
new file mode 100644
index 0000000..ed31664
--- /dev/null
+++ b/org.eclipse.jdt.debug.tests/testprograms/Bug540243.java
@@ -0,0 +1,31 @@
+/*******************************************************************************
+ * Copyright (c) 2018 Simeon Andreev 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:
+ * Simeon Andreev - initial API and implementation
+ *******************************************************************************/
+
+public class Bug540243 {
+
+ public static void main(String[] args) throws InterruptedException {
+ Thread t = new Thread(new Runnable() {
+ // method must be synchronized, so that the thread owns a monitor
+ public synchronized void run() {
+ breakpointMethod();
+ }
+ });
+ t.start();
+ t.join();
+ }
+
+ public static void breakpointMethod() {
+ System.out.println("set a breakpoint here");
+ }
+}
diff --git a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/AbstractDebugTest.java b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/AbstractDebugTest.java
index 2eed4e0..a3b7745 100644
--- a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/AbstractDebugTest.java
+++ b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/AbstractDebugTest.java
@@ -207,7 +207,7 @@
"org.eclipse.debug.tests.targets.HcrClass5", "org.eclipse.debug.tests.targets.HcrClass6", "org.eclipse.debug.tests.targets.HcrClass7", "org.eclipse.debug.tests.targets.HcrClass8",
"org.eclipse.debug.tests.targets.HcrClass9", "TestContributedStepFilterClass", "TerminateAll_01", "TerminateAll_02", "StepResult1",
"StepResult2", "StepResult3", "StepUncaught", "TriggerPoint_01", "BulkThreadCreationTest", "MethodExitAndException",
- "Bug534319earlyStart", "Bug534319lateStart", "Bug534319singleThread", "Bug534319startBetwen", "MethodCall", "Bug538303" };
+ "Bug534319earlyStart", "Bug534319lateStart", "Bug534319singleThread", "Bug534319startBetwen", "MethodCall", "Bug538303", "Bug540243" };
/**
* the default timeout
diff --git a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/ui/AbstractDebugViewTests.java b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/ui/AbstractDebugViewTests.java
index ac6f7ea..843f239 100644
--- a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/ui/AbstractDebugViewTests.java
+++ b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/ui/AbstractDebugViewTests.java
@@ -55,13 +55,15 @@
}
private LaunchView debugView;
- private boolean showMonitorsOriginal;
+ private Boolean showMonitorsOriginal;
+ private Boolean showSystemThreadsOriginal;
@Override
protected void setUp() throws Exception {
super.setUp();
IPreferenceStore jdiUIPreferences = JDIDebugUIPlugin.getDefault().getPreferenceStore();
showMonitorsOriginal = jdiUIPreferences.getBoolean(IJavaDebugUIConstants.PREF_SHOW_MONITOR_THREAD_INFO);
+ showSystemThreadsOriginal = jdiUIPreferences.getBoolean(IJavaDebugUIConstants.PREF_SHOW_SYSTEM_THREADS);
jdiUIPreferences.setValue(IJavaDebugUIConstants.PREF_SHOW_MONITOR_THREAD_INFO, true);
resetPerspective(DebugViewPerspectiveFactory.ID);
debugView = sync(() -> (LaunchView) getActivePage().showView(IDebugUIConstants.ID_DEBUG_VIEW));
@@ -72,6 +74,7 @@
protected void tearDown() throws Exception {
IPreferenceStore jdiUIPreferences = JDIDebugUIPlugin.getDefault().getPreferenceStore();
jdiUIPreferences.setValue(IJavaDebugUIConstants.PREF_SHOW_MONITOR_THREAD_INFO, showMonitorsOriginal);
+ jdiUIPreferences.setValue(IJavaDebugUIConstants.PREF_SHOW_SYSTEM_THREADS, showSystemThreadsOriginal);
sync(() -> getActivePage().closeAllEditors(false));
processUiEvents(100);
super.tearDown();
@@ -118,6 +121,7 @@
try {
thread = launchToBreakpoint(typeName, breakpointMethodName, expectedBreakpointHitsCount);
+ assertDebugViewIsOpen();
assertStackFrameIsSelected(breakpointMethodName);
} catch (AssertionError assertionError) {
failedAssertions.add(assertionError);
@@ -272,8 +276,13 @@
sync(() -> getActivePage().activate(debugView));
}
+ protected void assertDebugViewIsOpen() throws Exception {
+ debugView = sync(() -> (LaunchView) getActivePage().findView(IDebugUIConstants.ID_DEBUG_VIEW));
+ assertNotNull("expected Debug View to be open", debugView);
+ }
+
protected void assertDebugViewIsActive() throws Exception {
- assertEquals("expected Debug View to activate after resuming thread", debugView, sync(() -> getActivePage().getActivePart()));
+ assertEquals("expected Debug View to be activate", debugView, sync(() -> getActivePage().getActivePart()));
}
protected static class BreakpointWaiter extends DebugElementKindEventDetailWaiter {
diff --git a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/ui/DebugViewTests.java b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/ui/DebugViewTests.java
index 779348d..ed315d8 100644
--- a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/ui/DebugViewTests.java
+++ b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/ui/DebugViewTests.java
@@ -17,9 +17,13 @@
import org.eclipse.core.runtime.Platform;
import org.eclipse.debug.core.model.IStackFrame;
+import org.eclipse.debug.ui.IDebugUIConstants;
import org.eclipse.jdt.debug.core.IJavaStackFrame;
import org.eclipse.jdt.debug.core.IJavaThread;
import org.eclipse.jdt.debug.tests.TestAgainException;
+import org.eclipse.jdt.debug.ui.IJavaDebugUIConstants;
+import org.eclipse.jdt.internal.debug.ui.JDIDebugUIPlugin;
+import org.eclipse.jface.preference.IPreferenceStore;
import org.eclipse.swt.widgets.TreeItem;
import org.eclipse.test.OrderedTestSuite;
import org.eclipse.ui.IViewPart;
@@ -194,6 +198,36 @@
}
/**
+ * Test for Bug 540243 - Wrong selection when first opening view due to breakpoint
+ *
+ * When hitting a breakpoint, if the Debug View is open at all, and we show Java thread owned monitors, its possible to see a wrong selection in
+ * the Debug View. To ensure this doesn't occur, this test does the following:
+ *
+ * <ol>
+ * <li>ensures the Debug View is showing owned monitors for threads</li>
+ * <li>close the Debug View</li>
+ * <li>create a Java snippet which starts a thread</li>
+ * <li>set a break point in the code executed by the thread</li>
+ * <li>debug the snippet until the break point is reached</li>
+ * <li>validate that the selection in the Debug View contains is exactly the method with a break point</li>
+ * </ol>
+ */
+ public void testWrongSelectionBug540243() throws Exception {
+ IPreferenceStore jdiUIPreferences = JDIDebugUIPlugin.getDefault().getPreferenceStore();
+ Boolean isShowingMonitorThreadInfo = jdiUIPreferences.getBoolean(IJavaDebugUIConstants.PREF_SHOW_MONITOR_THREAD_INFO);
+ assertNotNull("Preference to show thread owned monitors must be set but is not", isShowingMonitorThreadInfo);
+ assertTrue("Preference to show thread owned monitors must be enabled but is not", isShowingMonitorThreadInfo);
+
+ sync(() -> getActivePage().hideView(getActivePage().findView(IDebugUIConstants.ID_DEBUG_VIEW)));
+
+ int iterations = 1;
+ String typeName = "Bug540243";
+ String breakpointMethodName = "breakpointMethod";
+ int expectedBreakpointHitsCount = 1;
+ doTestWrongSelection(iterations, typeName, breakpointMethodName, expectedBreakpointHitsCount);
+ }
+
+ /**
* Test for Bug 534319 - Debug View shows wrong information due to threads with short lifetime
*
* We observe that e.g. starting new threads from the debugged JVM can cause incorrect selections in the Debug View. To assure this doesn't occur,
diff --git a/org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/threadgroups/JavaDebugTargetProxy.java b/org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/threadgroups/JavaDebugTargetProxy.java
index b013a0a..5fd7379 100644
--- a/org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/threadgroups/JavaDebugTargetProxy.java
+++ b/org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/threadgroups/JavaDebugTargetProxy.java
@@ -21,7 +21,10 @@
import org.eclipse.debug.core.DebugPlugin;
import org.eclipse.debug.core.ILaunch;
import org.eclipse.debug.core.ILaunchManager;
+import org.eclipse.debug.core.model.IDebugElement;
import org.eclipse.debug.core.model.IDebugTarget;
+import org.eclipse.debug.core.model.IStackFrame;
+import org.eclipse.debug.core.model.IThread;
import org.eclipse.debug.internal.ui.viewers.model.provisional.IModelDelta;
import org.eclipse.debug.internal.ui.viewers.model.provisional.ModelDelta;
import org.eclipse.debug.internal.ui.viewers.update.DebugEventHandler;
@@ -29,6 +32,7 @@
import org.eclipse.debug.internal.ui.viewers.update.DebugTargetProxy;
import org.eclipse.debug.internal.ui.viewers.update.StackFrameEventHandler;
import org.eclipse.jdt.debug.core.IJavaDebugTarget;
+import org.eclipse.jdt.debug.ui.JavaDebugUtils;
import org.eclipse.jdt.internal.debug.ui.monitors.JavaElementContentProvider;
import org.eclipse.jdt.internal.debug.ui.snippeteditor.ScrapbookLauncher;
import org.eclipse.jface.viewers.Viewer;
@@ -141,4 +145,18 @@
return 0;
}
+ @Override
+ protected int getStackFrameIndex(IStackFrame stackFrame) {
+ int stackFrameIndex = 0;
+ if (((IJavaDebugTarget) fDebugTarget).supportsMonitorInformation()) {
+ IThread thread = stackFrame.getThread();
+ IDebugElement[] ownedMonitors = JavaDebugUtils.getOwnedMonitors(thread);
+ stackFrameIndex += ownedMonitors.length;
+ IDebugElement contendedMonitor = JavaDebugUtils.getContendedMonitor(thread);
+ if (contendedMonitor != null) {
+ stackFrameIndex++;
+ }
+ }
+ return stackFrameIndex;
+ }
}
