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 <simeon.danailov.andreev@gmail.com>
Reviewed-on: https://git.eclipse.org/r/c/jdt/eclipse.jdt.debug/+/191125
Tested-by: JDT Bot <jdt-bot@eclipse.org>
Reviewed-by: Andrey Loskutov <loskutov@gmx.de>
diff --git a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIDebugModelMessages.java b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIDebugModelMessages.java
index 96ab09a..0b0fcf2 100644
--- a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIDebugModelMessages.java
+++ b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIDebugModelMessages.java
@@ -133,6 +133,7 @@
 	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;
diff --git a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIDebugModelMessages.properties b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIDebugModelMessages.properties
index 6c32a5f..75fce95 100644
--- a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIDebugModelMessages.properties
+++ b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIDebugModelMessages.properties
@@ -100,6 +100,7 @@
 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.
diff --git a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDILocalVariable.java b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDILocalVariable.java
index 8cc533b..532364c 100644
--- a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDILocalVariable.java
+++ b/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 @@
 	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 @@
 	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) {
diff --git a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIStackFrame.java b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIStackFrame.java
index fba88a9..d16573f 100644
--- a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIStackFrame.java
+++ b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIStackFrame.java
@@ -760,13 +760,16 @@
 			}
 		}
 
-		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 @@
 		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 @@
 					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 @@
 						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;
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 ce6c70d..da164ea 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
@@ -2361,7 +2361,11 @@
 					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());