Bug 578899 - NPE with breakpoint in ClassLoader.getDefinedPackage()

This change adds null checks to callers of
JDIStackFrame.getUnderlyingStackFrame(), to prevent NPEs that are not
handled by JDT. Such NPEs result in an error dialog.

JDIStackFrame.getUnderlyingStackFrame() can return null, if the
JDIStackFrame object is used after JDIThread.disposeStackFrames() is
called on the respective JDIThread.

In particular, JDIStackFrame.fStackFrame is set to null on a thread
resume. When getUnderlyingStackFrame() is called with a null in
fStackFrame, and a suspended thread (e.g. the next breakpoint was hit),
the method relies on JDIThread.computeStackFrames() to set the value of
fStackFrame. This is done in JDIStackFrame.bind(). However, after a
JDIThread.disposeStackFrames() call, JDIThread no longer has a reference
to the original JDIStackFrame object, and so will not set its
fStackFrame. So getUnderlyingStackFrame() returns a null.

JDIThread.disposeStackFrames() is called when a thread start event is
sent by the debuggee JVM, and JDT finds a JDIThread object to match the
underlying JVM thread (the stack frames of that JDIThread are then
cleared).

Change-Id: I075de893c53f436c8c56e6834925c216cbf704fd
Signed-off-by: Simeon Andreev <[email protected]>
Reviewed-on: https://git.eclipse.org/r/c/jdt/eclipse.jdt.debug/+/191125
Tested-by: JDT Bot <[email protected]>
Reviewed-by: Andrey Loskutov <[email protected]>

Author: trancexpress

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

@@ -133,6 +133,7 @@ public class JDIDebugModelMessages extends NLS {
 	public static String JDIStackFrame_ThrowingException;
 	public static String JDIStackFrame_NoMethodReturnValue;
 	public static String JDIStackFrame_NotObservedBecauseOfTimeout;
+	public static String JDIStackFrame_NoLongerAvailable;
 
 	public static String JDIThisVariable_exception_while_retrieving_type_this;
 	public static String JDIThisVariableexception_retrieving_reference_type_name;

org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIDebugModelMessages.properties

@@ -100,6 +100,7 @@ JDIStackFrame_ExceptionThrown={0}() threw
 JDIStackFrame_ThrowingException={0}() is throwing
 JDIStackFrame_NoMethodReturnValue=no method return value
 JDIStackFrame_NotObservedBecauseOfTimeout=(Not observed to speed up the long running step operation)
+JDIStackFrame_NoLongerAvailable=Stack frame is no longer available
 
 JDIThisVariable_exception_while_retrieving_type_this={0} occurred while retrieving type ''this''.
 JDIThisVariableexception_retrieving_reference_type_name={0} occurred retrieving reference type name.

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

@@ -24,6 +24,7 @@
 import com.sun.jdi.InvalidTypeException;
 import com.sun.jdi.LocalVariable;
 import com.sun.jdi.ReferenceType;
+import com.sun.jdi.StackFrame;
 import com.sun.jdi.Type;
 import com.sun.jdi.Value;
 
@@ -58,8 +59,10 @@ public JDILocalVariable(JDIStackFrame frame, LocalVariable local) {
 	protected Value retrieveValue() throws DebugException {
 		synchronized (fStackFrame.getThread()) {
 			if (getStackFrame().isSuspended()) {
-				return getStackFrame().getUnderlyingStackFrame().getValue(
-						fLocal);
+				StackFrame frame = getStackFrame().getUnderlyingStackFrame();
+				if (frame != null) {
+					return frame.getValue(fLocal);
+				}
 			}
 		}
 		// bug 6518
@@ -91,8 +94,16 @@ public String getName() throws DebugException {
 	protected void setJDIValue(Value value) throws DebugException {
 		try {
 			synchronized (getStackFrame().getThread()) {
-				getStackFrame().getUnderlyingStackFrame().setValue(getLocal(),
-						value);
+				StackFrame frame = getStackFrame().getUnderlyingStackFrame();
+				if (frame != null) {
+					frame.setValue(getLocal(), value);
+				} else {
+					String errorMessage = JDIDebugModelMessages.JDIStackFrame_NoLongerAvailable;
+					targetRequestFailed(
+							MessageFormat.format(
+									JDIDebugModelMessages.JDILocalVariable_exception_modifying_local_variable_value,
+									errorMessage), new Throwable(errorMessage)); // use Throwable, as RuntimeException is re-thrown
+				}
 			}
 			fireChangeEvent(DebugEvent.CONTENT);
 		} catch (ClassNotLoadedException e) {

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

@@ -760,13 +760,16 @@ protected void updateVariables() throws DebugException {
 			}
 		}
 
-		List<LocalVariable> locals = null;
+		List<LocalVariable> locals = Collections.EMPTY_LIST;
 		try {
-			locals = getUnderlyingStackFrame().visibleVariables();
+			StackFrame frame = getUnderlyingStackFrame();
+			if (frame != null) {
+				locals = frame.visibleVariables();
+			}
 		} catch (AbsentInformationException e) {
-			locals = Collections.EMPTY_LIST;
+			// continue with empty list of variables
 		} catch (NativeMethodException e) {
-			locals = Collections.EMPTY_LIST;
+			// continue with empty list of variables
 		} catch (RuntimeException e) {
 			targetRequestFailed(
 					MessageFormat.format(
@@ -980,7 +983,12 @@ protected List<LocalVariable> getUnderlyingVisibleVariables() throws DebugExcept
 		synchronized (fThread) {
 			List<LocalVariable> variables = Collections.EMPTY_LIST;
 			try {
-				variables = getUnderlyingStackFrame().visibleVariables();
+				StackFrame frame = getUnderlyingStackFrame();
+				if (frame != null) {
+					variables = frame.visibleVariables();
+				} else {
+					setLocalsAvailable(false);
+				}
 			} catch (AbsentInformationException e) {
 				setLocalsAvailable(false);
 			} catch (NativeMethodException e) {
@@ -1354,7 +1362,7 @@ protected StackFrame getUnderlyingStackFrame() throws DebugException {
 					throw new DebugException(new Status(IStatus.ERROR,
 							JDIDebugPlugin.getUniqueIdentifier(),
 							IJavaStackFrame.ERR_INVALID_STACK_FRAME,
-							JDIDebugModelMessages.JDIStackFrame_25, null));
+							JDIDebugModelMessages.JDIStackFrame_25, new IllegalStateException()));
 				}
 				if (fThread.isSuspended()) {
 					// re-index stack frames - See Bug 47198
@@ -1364,14 +1372,14 @@ protected StackFrame getUnderlyingStackFrame() throws DebugException {
 						fThread.computeStackFrames();
 						if (fDepth == -1) {
 						// If depth is -1, then this is an invalid frame
-							throw new DebugException(new Status(IStatus.ERROR, JDIDebugPlugin.getUniqueIdentifier(), IJavaStackFrame.ERR_INVALID_STACK_FRAME, JDIDebugModelMessages.JDIStackFrame_25, null));
+							throw new DebugException(new Status(IStatus.ERROR, JDIDebugPlugin.getUniqueIdentifier(), IJavaStackFrame.ERR_INVALID_STACK_FRAME, JDIDebugModelMessages.JDIStackFrame_25, new IllegalStateException()));
 						}
 					}
 				} else {
 					throw new DebugException(new Status(IStatus.ERROR,
 							JDIDebugPlugin.getUniqueIdentifier(),
 							IJavaThread.ERR_THREAD_NOT_SUSPENDED,
-							JDIDebugModelMessages.JDIStackFrame_25, null));
+							JDIDebugModelMessages.JDIStackFrame_25, new IllegalStateException()));
 				}
 			}
 			return fStackFrame;

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

@@ -2361,7 +2361,11 @@ protected void step() throws DebugException {
 					return;
 				}
 				setOriginalStepKind(getStepKind());
-				Location location = top.getUnderlyingStackFrame().location();
+				StackFrame frame = top.getUnderlyingStackFrame();
+				if (frame == null) {
+					return;
+				}
+				Location location = frame.location();
 				setOriginalStepLocation(location);
 				setOriginalStepStackDepth(computeStackFrames().size());
 				setStepRequest(createStepRequest());