Skip to content

Commit 8aa4a91

Browse files
committed
Don't run into NPE while hiding Breakpoints view
`VariablesView.becomesHidden()` is setting `ViewerInputService.NULL_INPUT` which sets the `IViewerInputUpdate` "input element" to null and so the viewer input for `Breakpoints` view to null. This is expected case, however the code in `ViewerUpdateMonitor` and in `ChildrenUpdate` was not prepared to that valid use case and runs into NPE or reports errors in case where no error happens. To fix (and add more error context for possible remaining input issues), let the `VariablesView` provide last used `IViewerInputUpdate` object so it can be used by `ViewerUpdateMonitor`, and let the `ChildrenUpdate` code be aware about viewer input element being null (which is documented as possible value at `ViewerUpdateMonitor.getViewerInput()`). Additionally implemented `toString()` in all related objects that can be set in `ViewerInputUpdate`, so the data can be provided in the error reported by `ViewerUpdateMonitor` for all remaining use cases with "null" input. Fixes #2150
1 parent 54a6a86 commit 8aa4a91

File tree

6 files changed

+105
-11
lines changed

6 files changed

+105
-11
lines changed

debug/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/elements/adapters/DefaultBreakpointsViewInput.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,4 +69,16 @@ public boolean equals(Object arg0) {
6969
return super.equals(arg0);
7070
}
7171

72+
@Override
73+
public String toString() {
74+
StringBuilder builder = new StringBuilder();
75+
builder.append("DefaultBreakpointsViewInput ["); //$NON-NLS-1$
76+
if (fContext != null) {
77+
builder.append("context="); //$NON-NLS-1$
78+
builder.append(fContext);
79+
}
80+
builder.append("]"); //$NON-NLS-1$
81+
return builder.toString();
82+
}
83+
7284
}

debug/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/ChildrenUpdate.java

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
*******************************************************************************/
1616
package org.eclipse.debug.internal.ui.viewers.model;
1717

18+
import java.util.Objects;
19+
1820
import org.eclipse.debug.internal.ui.DebugUIPlugin;
1921
import org.eclipse.debug.internal.ui.viewers.model.provisional.IChildrenUpdate;
2022
import org.eclipse.debug.internal.ui.viewers.model.provisional.IElementContentProvider;
@@ -205,19 +207,27 @@ Object[] getElements() {
205207

206208
@Override
207209
protected boolean doEquals(ViewerUpdateMonitor update) {
208-
return
209-
update instanceof ChildrenUpdate &&
210-
((ChildrenUpdate)update).getOffset() == getOffset() &&
211-
((ChildrenUpdate)update).getLength() == getLength() &&
212-
getViewerInput().equals(update.getViewerInput()) &&
213-
getElementPath().equals(update.getElementPath());
210+
if (!(update instanceof ChildrenUpdate other)) {
211+
return false;
212+
}
213+
boolean offsetAndLengthOK = other.getOffset() == getOffset() && other.getLength() == getLength();
214+
if (!offsetAndLengthOK) {
215+
return false;
216+
}
217+
Object viewerInput = getViewerInput();
218+
if (!Objects.equals(viewerInput, other.getViewerInput())) {
219+
return false;
220+
}
221+
return getElementPath().equals(other.getElementPath());
214222
}
215223

216224
@Override
217225
protected int doHashCode() {
218-
return (int)Math.pow(
219-
(getClass().hashCode() + getViewerInput().hashCode() + getElementPath().hashCode()) * (getOffset() + 2),
220-
getLength() + 2);
226+
Object viewerInput = getViewerInput();
227+
int inputBits = viewerInput != null ? viewerInput.hashCode() : 0;
228+
int result = (int) Math.pow(
229+
(getClass().hashCode() + inputBits + getElementPath().hashCode()) * (getOffset() + 2), getLength() + 2);
230+
return result;
221231
}
222232

223233
}

debug/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/ViewerInputUpdate.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,35 @@ public Object getViewerInput() {
143143
return fViewerInput;
144144
}
145145

146+
@Override
147+
public String toString() {
148+
StringBuilder builder = new StringBuilder();
149+
builder.append("ViewerInputUpdate ["); //$NON-NLS-1$
150+
builder.append("inputElement="); //$NON-NLS-1$
151+
builder.append(fInputElement);
152+
builder.append(", "); //$NON-NLS-1$
153+
if (fContext != null) {
154+
builder.append("context="); //$NON-NLS-1$
155+
builder.append(fContext);
156+
builder.append(", "); //$NON-NLS-1$
157+
}
158+
if (fSource != null) {
159+
builder.append("source="); //$NON-NLS-1$
160+
builder.append(fSource);
161+
builder.append(", "); //$NON-NLS-1$
162+
}
163+
if (fViewerInput != null) {
164+
builder.append("viewerInput="); //$NON-NLS-1$
165+
builder.append(fViewerInput);
166+
builder.append(", "); //$NON-NLS-1$
167+
}
168+
if (fRequestor != null) {
169+
builder.append("requestor="); //$NON-NLS-1$
170+
builder.append(fRequestor);
171+
}
172+
builder.append("]"); //$NON-NLS-1$
173+
return builder.toString();
174+
}
146175

147176

148177
}

debug/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/ViewerUpdateMonitor.java

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@
2121
import org.eclipse.debug.internal.ui.viewers.model.provisional.IElementContentProvider;
2222
import org.eclipse.debug.internal.ui.viewers.model.provisional.IPresentationContext;
2323
import org.eclipse.debug.internal.ui.viewers.model.provisional.ITreeModelViewer;
24+
import org.eclipse.debug.internal.ui.viewers.model.provisional.IViewerInputUpdate;
2425
import org.eclipse.debug.internal.ui.viewers.model.provisional.IViewerUpdate;
26+
import org.eclipse.debug.internal.ui.viewers.model.provisional.ViewerInputService;
27+
import org.eclipse.debug.internal.ui.views.variables.VariablesView;
2528
import org.eclipse.jface.viewers.TreePath;
2629
import org.eclipse.swt.widgets.Display;
2730

@@ -87,7 +90,15 @@ public ViewerUpdateMonitor(TreeModelContentProvider contentProvider, Object view
8790
fContext = context;
8891
// Bug 380288: Catch and log a race condition where the viewer input is null.
8992
if (viewerInput == null) {
90-
DebugUIPlugin.log(new NullPointerException("Input to viewer update should not be null")); //$NON-NLS-1$
93+
if (!(context.getPart() instanceof VariablesView view)) {
94+
DebugUIPlugin.log(new NullPointerException("Input to viewer update should not be null")); //$NON-NLS-1$
95+
} else {
96+
IViewerInputUpdate viewerUpdate = view.getLastViewerUpdate();
97+
if (viewerUpdate == null || viewerUpdate.getElement() != ViewerInputService.NULL_INPUT) {
98+
DebugUIPlugin.log(new NullPointerException(
99+
"Input element in viewer update should not be null: " + viewerUpdate)); //$NON-NLS-1$
100+
}
101+
}
91102
}
92103
fViewerInput = viewerInput;
93104
fElementContentProvider = elementContentProvider;

debug/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/views/DebugModelPresentationContext.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,4 +43,16 @@ public IDebugModelPresentation getModelPresentation() {
4343
return fPresentation;
4444
}
4545

46+
@Override
47+
public String toString() {
48+
StringBuilder builder = new StringBuilder();
49+
builder.append("DebugModelPresentationContext ["); //$NON-NLS-1$
50+
if (fPresentation != null) {
51+
builder.append("presentation="); //$NON-NLS-1$
52+
builder.append(fPresentation);
53+
}
54+
builder.append("]"); //$NON-NLS-1$
55+
return builder.toString();
56+
}
57+
4658
}

debug/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/views/variables/VariablesView.java

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,12 @@ public class VariablesView extends AbstractDebugView implements IDebugContextLis
153153
*/
154154
private static final String CSS_VARIABLES_VIEWER_ID = "VariablesViewer"; //$NON-NLS-1$
155155

156+
/**
157+
* Viewer update object used during
158+
* {@link #viewerInputUpdateComplete(IViewerInputUpdate)}
159+
*/
160+
private IViewerInputUpdate lastViewerUpdate;
161+
156162
/**
157163
* Selection provider wrapping an exchangeable active selection provider. Sends
158164
* out a selection changed event when the active selection provider changes.
@@ -294,7 +300,12 @@ public void setSelection(ISelection selection) {
294300
*/
295301
private final IViewerInputRequestor fRequester = update -> {
296302
if (!update.isCanceled()) {
297-
viewerInputUpdateComplete(update);
303+
lastViewerUpdate = update;
304+
try {
305+
viewerInputUpdateComplete(update);
306+
} finally {
307+
lastViewerUpdate = null;
308+
}
298309
}
299310
};
300311

@@ -442,6 +453,15 @@ protected void viewerInputUpdateComplete(IViewerInputUpdate update) {
442453
updateAction(FIND_ACTION);
443454
}
444455

456+
/**
457+
* @return Viewer update object being processed during
458+
* {@link #viewerInputUpdateComplete(IViewerInputUpdate)}, {@code null}
459+
* otherwise
460+
*/
461+
public final IViewerInputUpdate getLastViewerUpdate() {
462+
return lastViewerUpdate;
463+
}
464+
445465
/**
446466
* Sets the input to the viewer
447467
* @param context the object context

0 commit comments

Comments
 (0)