[303828] SyncUtil calls no longer wait indefinitely

dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/framework/BaseTestCase.java

@@ -114,7 +114,9 @@ public class BaseTestCase {
 				new ServiceEventWaitor<MIStoppedEvent>(
 						fLaunch.getSession(),
 						MIStoppedEvent.class);
-			fInitialStoppedEvent = eventWaitor.waitForEvent(10000);
+			
+			// In practice, most launches happen within 100-300 milliseconds
+			fInitialStoppedEvent = eventWaitor.waitForEvent(TestsPlugin.massageTimeout(1000));
 		} catch (Exception e) {}
 
  		// If we started a gdbserver add it to the launch to make sure it is killed at the end

dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/framework/ServiceEventWaitor.java

@@ -114,13 +114,24 @@ public class ServiceEventWaitor<V> {
 			}
 			
 			if (timeout != WAIT_FOREVER) {
-				if (timeout/duration > 7.0) {
+				if (duration == 0) {
+					if (timeout > 1000) {
+						System.out.println("WARNING: Caller specified a timeout over a second but the operation was instantenous. The timeout is probably too loose.");
+					}
+					else if (timeout < 100) {
+						System.out.println("WARNING: Caller specified a timeout less than 100 milliseconds. Even though the operation completed instantaneously, the timeout is probably too tight.");
+					}
+				}
+				else {
+					if (timeout/duration > 7.0 && timeout > 2000) {
+						// don't bother for timeouts less than 2 seconds
 						System.out.println("WARNING: Caller specified a timeout that was more than 7X what was necessary. The timeout is probably too loose.");
 					}
 					else if ((((float)(timeout - duration))/(float)duration) < 0.20) {
 						System.out.println("WARNING: Caller specified a timeout that was less than 20% above actual time. The timeout is probably too tight.");
 					}
 				}
+			}
 			else {
 				System.out.println("WARNING: Caller requested to wait forever. It should probably specify some reasonable value.");
 			}

dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/framework/SyncUtil.java

@@ -12,6 +12,8 @@ package org.eclipse.cdt.tests.dsf.gdb.framework;
 
 import static org.junit.Assert.assertTrue;
 
+import java.util.HashMap;
+import java.util.Map;
 import java.util.concurrent.Callable;
 
 import junit.framework.Assert;
@@ -52,10 +54,14 @@ import org.eclipse.cdt.dsf.mi.service.command.output.MIBreakpoint;
 import org.eclipse.cdt.dsf.mi.service.command.output.MIInfo;
 import org.eclipse.cdt.dsf.service.DsfServicesTracker;
 import org.eclipse.cdt.dsf.service.DsfSession;
+import org.eclipse.cdt.tests.dsf.gdb.framework.SyncUtil.DefaultTimeouts.ETimeout;
 import org.eclipse.cdt.tests.dsf.gdb.launching.TestsPlugin;
 import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.Status;
 
+/**
+ * Timeout wait values are in milliseconds, or WAIT_FOREVER.
+ */
 public class SyncUtil {
     
     private static ICommandControlService fCommandControl;
@@ -67,6 +73,8 @@ public class SyncUtil {
     private static IContainerDMContext fGdbContainerDmc;
     private static IBreakpointsTargetDMContext fBreakpointsDmc;
     
+    public static final int WAIT_FOREVER = ServiceEventWaitor.WAIT_FOREVER;
+    
     // Initialize some common things, once the session has been established
     public static void initialize(DsfSession session) {
     	fSession = session;
@@ -89,19 +97,27 @@ public class SyncUtil {
 		tracker.dispose();
 	}
 
-	public static MIStoppedEvent step(final StepType stepType, int numSteps) throws Throwable {
+	public static MIStoppedEvent step(final StepType stepType, int numSteps, int timeout) throws Throwable {
 	    MIStoppedEvent retVal = null;
 		for (int i=0; i<numSteps; i++) {
-		    retVal = step(stepType);
+		    retVal = step(stepType, timeout);
 		}
 		return retVal;
 	}
 
 	public static MIStoppedEvent step(final StepType stepType) throws Throwable {
-		return step(fGdbContainerDmc, stepType);
+		return step(stepType, DefaultTimeouts.get(ETimeout.step));
+	}
+
+	public static MIStoppedEvent step(final StepType stepType, int timeout) throws Throwable {
+		return step(fGdbContainerDmc, stepType, timeout);
 	}
 	
 	public static MIStoppedEvent step(final IExecutionDMContext dmc, final StepType stepType) throws Throwable {
+		return step(dmc, stepType, DefaultTimeouts.get(ETimeout.step));		
+	}
+	
+	public static MIStoppedEvent step(final IExecutionDMContext dmc, final StepType stepType, int timeout) throws Throwable {
 		
 		final ServiceEventWaitor<MIStoppedEvent> eventWaitor =
 			new ServiceEventWaitor<MIStoppedEvent>(
@@ -129,11 +145,16 @@ public class SyncUtil {
 		});
 
 		// Wait for the execution to suspend after the step
-		return eventWaitor.waitForEvent(ServiceEventWaitor.WAIT_FOREVER);
+		return eventWaitor.waitForEvent(timeout);
 	}
 
 	public static MIStoppedEvent runToLine(final IExecutionDMContext dmc, final String fileName, final String lineNo, 
             final boolean skipBreakpoints) throws Throwable {
+		return runToLine(dmc, fileName, lineNo, skipBreakpoints, DefaultTimeouts.get(ETimeout.runToLine));
+	}
+
+	public static MIStoppedEvent runToLine(final IExecutionDMContext dmc, final String fileName, final String lineNo, 
+			                         final boolean skipBreakpoints, int timeout) throws Throwable {
 		
         final ServiceEventWaitor<MIStoppedEvent> eventWaitor =
             new ServiceEventWaitor<MIStoppedEvent>(
@@ -152,24 +173,40 @@ public class SyncUtil {
 		});
 
 		// Wait for the execution to suspend after the step
-    	return eventWaitor.waitForEvent(ServiceEventWaitor.WAIT_FOREVER);	
+    	return eventWaitor.waitForEvent(timeout);	
 	}
 
 	public static MIStoppedEvent runToLine(final String fileName, final String lineNo, 
             final boolean skipBreakpoints) throws Throwable {
-		return runToLine(fGdbContainerDmc, fileName, lineNo, skipBreakpoints);
+		return runToLine(fileName, lineNo, skipBreakpoints, DefaultTimeouts.get(ETimeout.runToLine));
+	}
+
+	public static MIStoppedEvent runToLine(final String fileName, final String lineNo, 
+            final boolean skipBreakpoints, int timeout) throws Throwable {
+		return runToLine(fGdbContainerDmc, fileName, lineNo, skipBreakpoints, timeout);
 	}
 
 	public static MIStoppedEvent runToLine(final String fileName, final String lineNo) throws Throwable {
-		return runToLine(fGdbContainerDmc, fileName, lineNo, false);
+		return runToLine(fileName, lineNo, DefaultTimeouts.get(ETimeout.runToLine));
 	}
 
+	public static MIStoppedEvent runToLine(final String fileName, final String lineNo, int timeout) throws Throwable {
+		return runToLine(fGdbContainerDmc, fileName, lineNo, false, timeout);
+	}
 
 	public static int addBreakpoint(final String location) throws Throwable {
-		return addBreakpoint(location, true);
+		return addBreakpoint(location, DefaultTimeouts.get(ETimeout.addBreakpoint));
 	}
 
-	public static int addBreakpoint(final String location, boolean temporary)
+	public static int addBreakpoint(final String location, int timeout) throws Throwable {
+		return addBreakpoint(location, true, timeout);
+	}
+
+	public static int addBreakpoint(final String location, boolean temporary) throws Throwable {
+		return addBreakpoint(location, temporary, DefaultTimeouts.get(ETimeout.addBreakpoint));
+	}
+	
+	public static int addBreakpoint(final String location, boolean temporary, int timeout)
 							throws Throwable {
 
         final AsyncCompletionWaitor wait = new AsyncCompletionWaitor();
@@ -190,14 +227,14 @@ public class SyncUtil {
 				new MIBreakInsert(fBreakpointsDmc, temporary, false, null, 0, location, 0),
 			    addBreakDone);
 		
-        wait.waitUntilDone(AsyncCompletionWaitor.WAIT_FOREVER);
+        wait.waitUntilDone(timeout);
         assertTrue(wait.getMessage(), wait.isOK());
         MIBreakInsertInfo info = (MIBreakInsertInfo) wait.getReturnInfo();
         return info.getMIBreakpoints()[0].getNumber();
 	}
 
 	
-	public static int[] getBreakpointList() throws Throwable {
+	public static int[] getBreakpointList(int timeout) throws Throwable {
 
 		final AsyncCompletionWaitor wait = new AsyncCompletionWaitor();
 
@@ -214,7 +251,7 @@ public class SyncUtil {
 
 		fCommandControl.queueCommand(new MIBreakList(fBreakpointsDmc), listDRM);
 		
-        wait.waitUntilDone(AsyncCompletionWaitor.WAIT_FOREVER);
+        wait.waitUntilDone(timeout);
         assertTrue(wait.getMessage(), wait.isOK());
 
 		MIBreakpoint[] breakpoints = listDRM.getData().getMIBreakpoints();
@@ -225,11 +262,11 @@ public class SyncUtil {
 		return result;
 	}
 	
-	public static void deleteBreakpoint(int breakpointIndex) throws Throwable {
+	public static void deleteBreakpoint(int breakpointIndex, int timeout) throws Throwable {
-		deleteBreakpoint(new int[] {breakpointIndex});
+		deleteBreakpoint(new int[] {breakpointIndex}, timeout);
 	}
 	
-	public static void deleteBreakpoint(int[] breakpointIndices) throws Throwable {
+	public static void deleteBreakpoint(int[] breakpointIndices, int timeout) throws Throwable {
 
         final AsyncCompletionWaitor wait = new AsyncCompletionWaitor();
 
@@ -249,12 +286,12 @@ public class SyncUtil {
 				new MIBreakDelete(fBreakpointsDmc, breakpointIndices), //$NON-NLS-1$
 				deleteBreakDone);
 		
-        wait.waitUntilDone(AsyncCompletionWaitor.WAIT_FOREVER);
+        wait.waitUntilDone(timeout);
         assertTrue(wait.getMessage(), wait.isOK());
 	}
 
 	
-	public static MIStoppedEvent resumeUntilStopped(final IExecutionDMContext dmc) throws Throwable {
+	public static MIStoppedEvent resumeUntilStopped(final IExecutionDMContext dmc, int timeout) throws Throwable {
         final ServiceEventWaitor<MIStoppedEvent> eventWaitor =
             new ServiceEventWaitor<MIStoppedEvent>(
                     fSession,
@@ -271,14 +308,18 @@ public class SyncUtil {
 		});
 
 		// Wait for the execution to suspend after the step
-    	return eventWaitor.waitForEvent(ServiceEventWaitor.WAIT_FOREVER);			
+    	return eventWaitor.waitForEvent(timeout);			
 	}
 
 	public static MIStoppedEvent resumeUntilStopped() throws Throwable {
-		return resumeUntilStopped(fGdbContainerDmc);
+		return resumeUntilStopped(DefaultTimeouts.get(ETimeout.resumeUntilStopped));
 	}
 
-	public static MIRunningEvent resume(final IExecutionDMContext dmc) throws Throwable {
+	public static MIStoppedEvent resumeUntilStopped(int timeout) throws Throwable {
+		return resumeUntilStopped(fGdbContainerDmc, timeout);
+	}
+
+	public static MIRunningEvent resume(final IExecutionDMContext dmc, int timeout) throws Throwable {
         final ServiceEventWaitor<MIRunningEvent> eventWaitor =
             new ServiceEventWaitor<MIRunningEvent>(
                     fSession,
@@ -295,29 +336,41 @@ public class SyncUtil {
 		});
 
 		// Wait for the execution to suspend after the step
-    	return eventWaitor.waitForEvent(ServiceEventWaitor.WAIT_FOREVER);			
+    	return eventWaitor.waitForEvent(timeout);			
 	}
 
 	public static MIRunningEvent resume() throws Throwable {
-		return resume(fGdbContainerDmc);
+		return resume(DefaultTimeouts.get(ETimeout.resume));
+	}
+
+	public static MIRunningEvent resume(int timeout) throws Throwable {
+		return resume(fGdbContainerDmc, timeout);
 	}
 
 	public static MIStoppedEvent waitForStop() throws Throwable {
+		return waitForStop(DefaultTimeouts.get(ETimeout.waitForStop));
+	}
+	
+	public static MIStoppedEvent waitForStop(int timeout) throws Throwable {
         final ServiceEventWaitor<MIStoppedEvent> eventWaitor =
             new ServiceEventWaitor<MIStoppedEvent>(
                     fSession,
                     MIStoppedEvent.class);
 
 		// Wait for the execution to suspend
-    	return eventWaitor.waitForEvent(ServiceEventWaitor.WAIT_FOREVER);			
+    	return eventWaitor.waitForEvent(timeout);			
 	}
 	
 	public static MIStoppedEvent runToLocation(final String location) throws Throwable {
+		return runToLocation(location, DefaultTimeouts.get(ETimeout.runToLocation));
+	}
+	
+	public static MIStoppedEvent runToLocation(final String location, int timeout) throws Throwable {
 		// Set a temporary breakpoint and run to it.
 		// Note that if there were other breakpoints set ahead of this one,
 		// they will stop execution earlier than planned
-		addBreakpoint(location, true);
+		addBreakpoint(location, true, timeout);
-		return resumeUntilStopped();
+		return resumeUntilStopped(timeout);
 	}
 	
     public static IFrameDMContext getStackFrame(final IExecutionDMContext execCtx, final int level) throws Throwable {
@@ -364,8 +417,7 @@ public class SyncUtil {
         return fSession.getExecutor().submit(callable).get();
     }
     
-    public static IMIExecutionDMContext createExecutionContext(final IContainerDMContext parentCtx, final int threadId)
+    public static IMIExecutionDMContext createExecutionContext(final IContainerDMContext parentCtx, final int threadId) throws Throwable {
-    throws Throwable {
 	    Callable<IMIExecutionDMContext> callable = new Callable<IMIExecutionDMContext>() {
 	        public IMIExecutionDMContext call() throws Exception {
 	            return fRunControl.createMIExecutionContext(parentCtx, threadId);
@@ -374,4 +426,90 @@ public class SyncUtil {
 	    return fSession.getExecutor().submit(callable).get();
     }
     
+    static class DefaultTimeouts {
+
+		/**
+		 * Overridable default timeout values. An override is specified using a
+		 * system property that is "dsf.gdb.tests.timeout.default." plus the
+		 * name of the enum below.
+		 */
+    	enum ETimeout {
+    		addBreakpoint,
+    		deleteBreakpoint,
+    		getBreakpointList,
+    		createExecutionContext,
+    		createExpression,
+    		getFormattedValue,
+    		getStackFrame,
+    		resume,
+    		resumeUntilStopped,
+    		runToLine,
+    		runToLocation,
+    		step,
+    		waitForStop
+    	}
+
+		/**
+		 * Map of timeout enums to their <b>harcoded</b> default value )in
+		 * milliseconds). These can be individually overridden with a system
+		 * property.
+		 * 
+		 * <p>
+		 * In practice, these operations are either very quick or the amount of
+		 * time is hard to predict (depends on what the test is doing). For ones
+		 * that are quick, we allot 1 second, which is ample. For the unknowns
+		 * we allows 10 seconds, which is probably ample in most cases. Tests
+		 * can provide larger values as needed in specific SyncUtil calls.
+		 */
+    	private static Map<ETimeout,Integer> sTimeouts = new HashMap<ETimeout, Integer>();
+    	static {
+    		sTimeouts.put(ETimeout.addBreakpoint, 1000);
+    		sTimeouts.put(ETimeout.deleteBreakpoint, 1000);
+    		sTimeouts.put(ETimeout.getBreakpointList, 1000);
+    		sTimeouts.put(ETimeout.createExecutionContext, 1000);
+    		sTimeouts.put(ETimeout.createExpression, 1000);
+    		sTimeouts.put(ETimeout.getFormattedValue, 1000);
+    		sTimeouts.put(ETimeout.getStackFrame, 1000);
+    		sTimeouts.put(ETimeout.resume, 1000);
+    		sTimeouts.put(ETimeout.resumeUntilStopped, 10000); // 10 seconds
+    		sTimeouts.put(ETimeout.runToLine, 10000);	// 10 seconds
+    		sTimeouts.put(ETimeout.step, 1000);
+    		sTimeouts.put(ETimeout.waitForStop, 10000);	// 10 seconds
+    	}
+
+		/**
+		 * Get the default timeout to use when the caller of a SyncUtil method
+		 * doesn't specify one. We honor overrides specified via system
+		 * properties, as well as apply the multiplier that can also be
+		 * specified via a system property.
+		 * 
+		 * @param timeout
+		 *            the timeout enum
+		 * @return the default value
+		 */
+    	static int get(ETimeout timeout) {
+    		int value = -1;
+    		final String propname = "dsf.gdb.tests.timeout.default." + timeout.toString();
+    		final String prop = System.getProperty(propname);
+    		if (prop != null) {
+	    		try {
+	    			value = Integer.valueOf(value);
+	    			if (value < 0) {
+	    				TestsPlugin.log(new Status(IStatus.ERROR, TestsPlugin.getUniqueIdentifier(), "\"" + propname + "\" property incorrectly specified. Should be an integer value or not specified at all.")); //$NON-NLS-1$
+	    				value = -1;
+	    			}
+	    		}
+	    		catch (NumberFormatException exc) {
+	    			TestsPlugin.log(new Status(IStatus.ERROR, TestsPlugin.getUniqueIdentifier(), "\"" + propname + "\" property incorrectly specified. Should be an integer value or not specified at all.")); //$NON-NLS-1$
+	    			value = -1;
+	    		}
+    		}
+    		
+    		if (value == -1) {
+    			value = sTimeouts.get(timeout);
+    		}
+    		assert value >= 0;
+    		return TestsPlugin.massageTimeout(value);
+    	}
+    }
 }