Bug 537399 - prevent GC on logical type results

Added a bit flag to instruct IAstEvaluationEngine to disable GC on the
result object.

Logical structures use this flag and only reenable GC when the program
resumes.

Change-Id: I0992957be34bad1992f49929c863a252435c4695
Signed-off-by: Julian Honnen <julian.honnen@vector.com>
diff --git a/org.eclipse.jdt.debug.tests/java9/LogicalStructures.java b/org.eclipse.jdt.debug.tests/java9/LogicalStructures.java
index cb268b3..1244ca8 100644
--- a/org.eclipse.jdt.debug.tests/java9/LogicalStructures.java
+++ b/org.eclipse.jdt.debug.tests/java9/LogicalStructures.java
@@ -12,11 +12,7 @@
  *     IBM Corporation - initial API and implementation
  *******************************************************************************/
 
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
+import java.util.*;
 import java.util.Map.Entry;
 
 /**
@@ -25,15 +21,30 @@
 public class LogicalStructures {
 	
 	public static void main(String[] args) {
-		Map map = new HashMap();
-		map.put("one", new Integer(1));
-		map.put("two", new Integer(2));
-		List list = new ArrayList();
+		generateGarbage();
+		
+		Map<String, Integer> map = new HashMap<>();
+		map.put("one", 1);
+		map.put("two", 2);
+		List<String> list = new ArrayList<>();
 		list.add("three");
 		list.add("four");
-		Set set = map.entrySet();
-		Entry entry = (Entry) set.iterator().next();
+		Set<Entry<String, Integer>> set = map.entrySet();
+		Map.Entry<String, Integer> entry = set.iterator().next();
 		entry.getKey();
 	}
+	
+	private static void generateGarbage() {
+		// generate garbage repeatedly to verify that logical values don't get GCed
+		Timer timer = new Timer(true);
+		timer.scheduleAtFixedRate(new TimerTask() {
+			private List<String> garbage;
+			@Override
+			public void run() {
+				System.gc();
+				garbage = Arrays.asList(new String("a"), new String("b"), new String("c"));
+			}
+		}, 50, 20);
+	}
 
 }
diff --git a/org.eclipse.jdt.debug.tests/testprograms/LogicalStructures.java b/org.eclipse.jdt.debug.tests/testprograms/LogicalStructures.java
index cb268b3..1244ca8 100644
--- a/org.eclipse.jdt.debug.tests/testprograms/LogicalStructures.java
+++ b/org.eclipse.jdt.debug.tests/testprograms/LogicalStructures.java
@@ -12,11 +12,7 @@
  *     IBM Corporation - initial API and implementation
  *******************************************************************************/
 
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
+import java.util.*;
 import java.util.Map.Entry;
 
 /**
@@ -25,15 +21,30 @@
 public class LogicalStructures {
 	
 	public static void main(String[] args) {
-		Map map = new HashMap();
-		map.put("one", new Integer(1));
-		map.put("two", new Integer(2));
-		List list = new ArrayList();
+		generateGarbage();
+		
+		Map<String, Integer> map = new HashMap<>();
+		map.put("one", 1);
+		map.put("two", 2);
+		List<String> list = new ArrayList<>();
 		list.add("three");
 		list.add("four");
-		Set set = map.entrySet();
-		Entry entry = (Entry) set.iterator().next();
+		Set<Entry<String, Integer>> set = map.entrySet();
+		Map.Entry<String, Integer> entry = set.iterator().next();
 		entry.getKey();
 	}
+	
+	private static void generateGarbage() {
+		// generate garbage repeatedly to verify that logical values don't get GCed
+		Timer timer = new Timer(true);
+		timer.scheduleAtFixedRate(new TimerTask() {
+			private List<String> garbage;
+			@Override
+			public void run() {
+				System.gc();
+				garbage = Arrays.asList(new String("a"), new String("b"), new String("c"));
+			}
+		}, 50, 20);
+	}
 
 }
diff --git a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/variables/TestLogicalStructures.java b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/variables/TestLogicalStructures.java
index d282a98..1d0f49b 100644
--- a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/variables/TestLogicalStructures.java
+++ b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/variables/TestLogicalStructures.java
@@ -46,7 +46,7 @@
 	 */
 	public void testListLogicalStructure() throws Exception {
 		String typeName = "LogicalStructures";
-		createLineBreakpoint(36, typeName);
+		createLineBreakpoint(34, typeName);
 		IJavaThread thread= null;
 		try {
 			thread= launchToBreakpoint(typeName);
@@ -66,6 +66,7 @@
 			assertEquals("Should be one logical structure type", 1, types.length);
 
 			IJavaObject logicalValue = (IJavaObject) types[0].getLogicalStructure(value);
+			Thread.sleep(200); // run a few GC cycles
 			assertEquals("Logical value should be an array", "java.lang.Object[]", logicalValue.getJavaType().getName());
 
 			IJavaArray array = (IJavaArray) logicalValue;
@@ -84,7 +85,7 @@
 	 */
 	public void testMapLogicalStructure() throws Exception {
 		String typeName = "LogicalStructures";
-		createLineBreakpoint(33, typeName);
+		createLineBreakpoint(34, typeName);
 		IJavaThread thread= null;
 		try {
 			thread= launchToBreakpoint(typeName);
@@ -104,6 +105,7 @@
 			assertEquals("Should be one logical structure type", 1, types.length);
 
 			IJavaObject logicalValue = (IJavaObject) types[0].getLogicalStructure(value);
+			Thread.sleep(200); // run a few GC cycles
 			assertEquals("Logical value should be an array", "java.lang.Object[]", logicalValue.getJavaType().getName());
 
 			IJavaArray array = (IJavaArray) logicalValue;
@@ -122,7 +124,7 @@
 	 */
 	public void testEntryLogicalStructure() throws Exception {
 		String typeName = "LogicalStructures";
-		createLineBreakpoint(36, typeName);
+		createLineBreakpoint(34, typeName);
 		IJavaThread thread= null;
 		try {
 			thread= launchToBreakpoint(typeName);
@@ -142,6 +144,7 @@
 			assertEquals("Should be one logical structure type", 1, types.length);
 
 			IJavaObject logicalValue = (IJavaObject) types[0].getLogicalStructure(value);
+			Thread.sleep(200); // run a few GC cycles
 			IVariable[] children = logicalValue.getVariables();
 			assertEquals("Should be two elements in the structure", 2, children.length);
 			assertEquals("First entry should be key", "key", children[0].getName());
diff --git a/org.eclipse.jdt.debug/eval/org/eclipse/jdt/debug/eval/IAstEvaluationEngine.java b/org.eclipse.jdt.debug/eval/org/eclipse/jdt/debug/eval/IAstEvaluationEngine.java
index c1156bb..394f322 100644
--- a/org.eclipse.jdt.debug/eval/org/eclipse/jdt/debug/eval/IAstEvaluationEngine.java
+++ b/org.eclipse.jdt.debug/eval/org/eclipse/jdt/debug/eval/IAstEvaluationEngine.java
@@ -33,6 +33,14 @@
 public interface IAstEvaluationEngine extends IEvaluationEngine {
 
 	/**
+	 * Instructs the evaluation engine to disable garbage collection on the result object. The caller is then responsible to call
+	 * {@link IJavaObject#enableCollection()} on the evaluated result. Can be passed as a bit flag to the <code>evaluationDetail</code> parameter.
+	 *
+	 * @since 3.17
+	 */
+	int DISABLE_GC_ON_RESULT = 0x0100;
+
+	/**
 	 * Asynchronously evaluates the given expression in the context of the
 	 * specified stack frame, reporting the result back to the given listener.
 	 * The thread is resumed from the location at which it is currently
@@ -50,8 +58,9 @@
 	 *            the listener that will receive notification when/if the
 	 *            evaluation completes
 	 * @param evaluationDetail
-	 *            one of <code>DebugEvent.EVALUATION</code> or
-	 *            <code>DebugEvent.EVALUATION_IMPLICIT</code>
+	 *            bitmask of one of <code>DebugEvent.EVALUATION</code> or
+	 *            <code>DebugEvent.EVALUATION_IMPLICIT</code> and
+	 *            optionally <code>DISABLE_GC_ON_RESULT</code>
 	 * @param hitBreakpoints
 	 *            whether or not breakpoints should be honored in the evaluation
 	 *            thread during the evaluation. If <code>false</code>,
@@ -97,8 +106,9 @@
 	 *            the listener that will receive notification when/if the
 	 *            evaluation completes
 	 * @param evaluationDetail
-	 *            one of <code>DebugEvent.EVALUATION</code> or
-	 *            <code>DebugEvent.EVALUATION_IMPLICIT</code>
+	 *            bitmask of one of <code>DebugEvent.EVALUATION</code> or
+	 *            <code>DebugEvent.EVALUATION_IMPLICIT</code> and
+	 *            optionally <code>DISABLE_GC_ON_RESULT</code>
 	 * @param hitBreakpoints
 	 *            whether or not breakpoints should be honored in the evaluation
 	 *            thread during the evaluation. If <code>false</code>,
diff --git a/org.eclipse.jdt.debug/eval/org/eclipse/jdt/internal/debug/eval/ast/engine/ASTEvaluationEngine.java b/org.eclipse.jdt.debug/eval/org/eclipse/jdt/internal/debug/eval/ast/engine/ASTEvaluationEngine.java
index edb4f8a..b5be5c5 100644
--- a/org.eclipse.jdt.debug/eval/org/eclipse/jdt/internal/debug/eval/ast/engine/ASTEvaluationEngine.java
+++ b/org.eclipse.jdt.debug/eval/org/eclipse/jdt/internal/debug/eval/ast/engine/ASTEvaluationEngine.java
@@ -79,6 +79,7 @@
 
 public class ASTEvaluationEngine implements IAstEvaluationEngine {
 	public static final String ANONYMOUS_VAR_PREFIX = "val$"; //$NON-NLS-1$
+	private static final int EVALUATION_DETAIL_BITMASK = DebugEvent.EVALUATION | DebugEvent.EVALUATION_IMPLICIT;
 	private IJavaProject fProject;
 
 	private IJavaDebugTarget fDebugTarget;
@@ -699,6 +700,8 @@
 
 		private IEvaluationListener fListener;
 
+		private boolean fDisableGcOnResult;
+
 		public EvalRunnable(InstructionSequence expression, IJavaThread thread,
 				IRuntimeContext context, IEvaluationListener listener,
 				int evaluationDetail, boolean hitBreakpoints) {
@@ -706,8 +709,9 @@
 			fThread = thread;
 			fContext = context;
 			fListener = listener;
-			fEvaluationDetail = evaluationDetail;
+			fEvaluationDetail = (evaluationDetail & EVALUATION_DETAIL_BITMASK);
 			fHitBreakpoints = hitBreakpoints;
+			fDisableGcOnResult = (evaluationDetail & IAstEvaluationEngine.DISABLE_GC_ON_RESULT) != 0;
 		}
 
 		@Override
@@ -774,7 +778,7 @@
 					EventFilter filter = new EventFilter();
 					try {
 						DebugPlugin.getDefault().addDebugEventFilter(filter);
-						interpreter.execute();
+						interpreter.execute(fDisableGcOnResult);
 					} catch (CoreException exception) {
 						fException = exception;
 						if (fEvaluationDetail == DebugEvent.EVALUATION
diff --git a/org.eclipse.jdt.debug/eval/org/eclipse/jdt/internal/debug/eval/ast/engine/Interpreter.java b/org.eclipse.jdt.debug/eval/org/eclipse/jdt/internal/debug/eval/ast/engine/Interpreter.java
index 51377b8..9a3a2b5 100644
--- a/org.eclipse.jdt.debug/eval/org/eclipse/jdt/internal/debug/eval/ast/engine/Interpreter.java
+++ b/org.eclipse.jdt.debug/eval/org/eclipse/jdt/internal/debug/eval/ast/engine/Interpreter.java
@@ -60,7 +60,7 @@
 		fInternalVariables = new HashMap<>();
 	}
 
-	public void execute() throws CoreException {
+	public void execute(boolean disableGcOnResult) throws CoreException {
 		try {
 			reset();
 			while (fInstructionCounter < fInstructions.length && !fStopped) {
@@ -75,7 +75,7 @@
 			throw new CoreException(new Status(IStatus.ERROR,
 					JDIDebugModel.getPluginIdentifier(), e.getMessage(), e));
 		} finally {
-			releaseObjects();
+			releaseObjects(disableGcOnResult);
 		}
 	}
 
@@ -132,13 +132,16 @@
 	/**
 	 * Re-enable garbage collection if interim results.
 	 */
-	private void releaseObjects() {
+	private void releaseObjects(boolean disableGcOnResult) {
 		if (fPermStorage != null) {
+			IJavaValue result = getResult();
 			Iterator<IJavaObject> iterator = fPermStorage.iterator();
 			while (iterator.hasNext()) {
 				IJavaObject object = iterator.next();
 				try {
-					object.enableCollection();
+					if (!disableGcOnResult || object != result) {
+						object.enableCollection();
+					}
 				} catch (CoreException e) {
 					// don't worry about GC if the VM has terminated
 					if ((e.getStatus().getException() instanceof VMDisconnectedException)) {
diff --git a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/logicalstructures/JavaLogicalStructure.java b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/logicalstructures/JavaLogicalStructure.java
index 2c180f5..e54c8c8 100644
--- a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/logicalstructures/JavaLogicalStructure.java
+++ b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/logicalstructures/JavaLogicalStructure.java
@@ -30,6 +30,7 @@
 import org.eclipse.debug.core.ILogicalStructureType;
 import org.eclipse.debug.core.IStatusHandler;
 import org.eclipse.debug.core.model.IDebugTarget;
+import org.eclipse.debug.core.model.ILogicalStructureTypeDelegate3;
 import org.eclipse.debug.core.model.IThread;
 import org.eclipse.debug.core.model.IValue;
 import org.eclipse.jdt.core.IJavaProject;
@@ -43,7 +44,6 @@
 import org.eclipse.jdt.debug.core.IJavaThread;
 import org.eclipse.jdt.debug.core.IJavaType;
 import org.eclipse.jdt.debug.core.IJavaValue;
-import org.eclipse.jdt.debug.core.IJavaVariable;
 import org.eclipse.jdt.debug.eval.IAstEvaluationEngine;
 import org.eclipse.jdt.debug.eval.ICompiledExpression;
 import org.eclipse.jdt.debug.eval.IEvaluationListener;
@@ -55,7 +55,7 @@
 
 
 
-public class JavaLogicalStructure implements ILogicalStructureType {
+public class JavaLogicalStructure implements ILogicalStructureType, ILogicalStructureTypeDelegate3 {
 
 	private static IStatusHandler fgStackFrameProvider;
 
@@ -151,7 +151,7 @@
 			fResult = null;
 			fEvaluationEngine.evaluateExpression(compiledExpression,
 					fEvaluationValue, fThread, this,
-					DebugEvent.EVALUATION_IMPLICIT, false);
+					DebugEvent.EVALUATION_IMPLICIT | IAstEvaluationEngine.DISABLE_GC_ON_RESULT, false);
 			synchronized (this) {
 				if (fResult == null) {
 					try {
@@ -310,7 +310,7 @@
 					evaluationEngine);
 			if (fValue == null) {
 				// evaluate each variable
-				IJavaVariable[] variables = new IJavaVariable[fVariables.length];
+				JDIPlaceholderVariable[] variables = new JDIPlaceholderVariable[fVariables.length];
 				for (int i = 0; i < fVariables.length; i++) {
 					variables[i] = new JDIPlaceholderVariable(fVariables[i][0],
 							evaluationBlock.evaluate(fVariables[i][1]),
@@ -334,6 +334,17 @@
 		return value;
 	}
 
+	@Override
+	public void releaseValue(IValue value) {
+		if (value instanceof IJavaObject) {
+			try {
+				((IJavaObject) value).enableCollection();
+			} catch (DebugException e) {
+				JDIDebugPlugin.log(e);
+			}
+		}
+	}
+
 	/**
 	 * Returns the <code>IJavaReferenceType</code> from the specified
 	 * <code>IJavaObject</code>
diff --git a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/logicalstructures/LogicalObjectStructureValue.java b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/logicalstructures/LogicalObjectStructureValue.java
index 8eea396..b2c2d8f 100644
--- a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/logicalstructures/LogicalObjectStructureValue.java
+++ b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/logicalstructures/LogicalObjectStructureValue.java
@@ -16,13 +16,13 @@
 import org.eclipse.debug.core.DebugException;
 import org.eclipse.debug.core.ILaunch;
 import org.eclipse.debug.core.model.IDebugTarget;
+import org.eclipse.debug.core.model.IValue;
 import org.eclipse.debug.core.model.IVariable;
 import org.eclipse.jdt.debug.core.IJavaFieldVariable;
 import org.eclipse.jdt.debug.core.IJavaObject;
 import org.eclipse.jdt.debug.core.IJavaThread;
 import org.eclipse.jdt.debug.core.IJavaType;
 import org.eclipse.jdt.debug.core.IJavaValue;
-import org.eclipse.jdt.debug.core.IJavaVariable;
 
 /**
  * A proxy to an object representing the logical structure of that object.
@@ -30,7 +30,7 @@
 public class LogicalObjectStructureValue implements IJavaObject {
 
 	private IJavaObject fObject;
-	private IJavaVariable[] fVariables;
+	private JDIPlaceholderVariable[] fVariables;
 
 	/**
 	 * Constructs a proxy to the given object, with the given variables as
@@ -42,7 +42,7 @@
 	 *            java variables to add as children to this object
 	 */
 	public LogicalObjectStructureValue(IJavaObject object,
-			IJavaVariable[] variables) {
+			JDIPlaceholderVariable[] variables) {
 		fObject = object;
 		fVariables = variables;
 	}
@@ -269,6 +269,13 @@
 	@Override
 	public void enableCollection() throws DebugException {
 		fObject.enableCollection();
+
+		for (JDIPlaceholderVariable variable : fVariables) {
+			IValue variableValue = variable.getValue();
+			if (variableValue instanceof IJavaObject) {
+				((IJavaObject) variableValue).enableCollection();
+			}
+		}
 	}
 
 	/*