Attachment 206262 Details for Bug 361934 – Minor update to handle Marc's code comments

Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

[patch] Minor update to handle Marc's code comments

timeout_2.patch (text/plain), 14.00 KB, created by Mohamed Hussein

on 2011-11-01 07:22:40 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: Mohamed Hussein

Created: 2011-11-01 07:22:40 EDT

Size: 14.00 KB

patch

obsolete

>From f0aa6eaab6322d6b2b6694b767be163e303d99ca Tue, 1 Nov 2011 13:19:32 +0200
>From: mhussein <mohamed_hussein@mentor.com>
>Date: Tue, 1 Nov 2011 13:18:41 +0200
>Subject: [PATCH] Fix bug 361934 - Added timeout for all gdb commands - Added user preference to enable timeout and specify timeout value.
>
>diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/preferences/GdbDebugPreferencePage.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/preferences/GdbDebugPreferencePage.java
>index 6993e02..3cc69b3 100644
>--- a/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/preferences/GdbDebugPreferencePage.java
>+++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/preferences/GdbDebugPreferencePage.java
>@@ -9,6 +9,7 @@
>  *     Ericsson - initial API and implementation
>  *     Jens Elmenthaler (Verigy) - Added Full GDB pretty-printing support (bug 302121)
>  *     Sergey Prigogin (Google)
>+ *     Mohamed Hussein (Mentor Graphics) - Add timeout for gdb commands (Bug 361934)
>  *******************************************************************************/
> package org.eclipse.cdt.dsf.gdb.internal.ui.preferences;
> 
>@@ -189,6 +190,43 @@
> 		// Need to set layout again.
> 		group2.setLayout(groupLayout);
> 
>+		final Group groupTimeOut= new Group(parent, SWT.NONE);
>+		groupTimeOut.setText(MessagesForPreferences.GdbDebugPreferencePage_timeout_group_label);
>+		groupLayout= new GridLayout(3, false);
>+		groupTimeOut.setLayout(groupLayout);
>+		groupTimeOut.setLayoutData(new GridData(GridData.FILL_HORIZONTAL));
>+
>+		final ListenableBooleanFieldEditor enableGdbTimeoutField = new ListenableBooleanFieldEditor(
>+				IGdbDebugPreferenceConstants.PREF_COMMAND_TIMEOUT_ENABLE,
>+				MessagesForPreferences.GdbDebugPreferencePage_enableTimeout_label,
>+				SWT.NONE, groupTimeOut);
>+
>+		enableGdbTimeoutField.fillIntoGrid(groupTimeOut, 3);
>+		addField(enableGdbTimeoutField);
>+
>+		final IntegerFieldEditor timeoutField = new IntegerFieldEditor(
>+				IGdbDebugPreferenceConstants.PREF_DEFAULT_COMMAND_TIMEOUT,
>+				MessagesForPreferences.GdbDebugPreferencePage_timeout_label,
>+				groupTimeOut);
>+
>+		// Instead of using Integer.MAX_VALUE which is some obscure number,
>+		// using 100000 sec is nice and readable.
>+		timeoutField.setValidRange(1, 100000);
>+
>+		timeoutField.fillIntoGrid(groupTimeOut, 3);
>+		addField(timeoutField);
>+
>+		enableGdbTimeoutField.getChangeControl(groupTimeOut).addSelectionListener(new SelectionAdapter() {
>+			@Override
>+			public void widgetSelected(SelectionEvent e) {
>+				boolean enabled = enableGdbTimeoutField.getBooleanValue();
>+				timeoutField.setEnabled(enabled, groupTimeOut);
>+			}
>+		});
>+
>+		// Need to set layout again.
>+		groupTimeOut.setLayout(groupLayout);
>+
> 		Group group= new Group(parent, SWT.NONE);
> 		group.setText(MessagesForPreferences.GdbDebugPreferencePage_termination_label);
> 		groupLayout= new GridLayout(3, false);
>diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/preferences/MessagesForPreferences.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/preferences/MessagesForPreferences.java
>index 43f3099..cd12633 100644
>--- a/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/preferences/MessagesForPreferences.java
>+++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/preferences/MessagesForPreferences.java
>@@ -8,6 +8,7 @@
>  * Contributors:
>  *     Ericsson - initial API and implementation
>  *     Jens Elmenthaler (Verigy) - Added Full GDB pretty-printing support (bug 302121)
>+ *     Mohamed Hussein (Mentor Graphics) - Add timeout for gdb commands (Bug 361934)
>  *******************************************************************************/
> package org.eclipse.cdt.dsf.gdb.internal.ui.preferences;
> 
>@@ -36,6 +37,12 @@
> 	public static String GdbDebugPreferencePage_initialChildCountLimitForCollections_label;
> 	/** @since 2.2 */
> 	public static String GdbDebugPreferencePage_defaults_label;
>+	/** @since 2.3 */
>+	public static String GdbDebugPreferencePage_timeout_label;
>+	/** @since 2.3 */
>+	public static String GdbDebugPreferencePage_timeout_group_label;
>+	/** @since 2.3 */
>+	public static String GdbDebugPreferencePage_enableTimeout_label;
> 	static {
> 		// initialize resource bundle
> 		NLS.initializeMessages(MessagesForPreferences.class.getName(), MessagesForPreferences.class);
>diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/preferences/MessagesForPreferences.properties b/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/preferences/MessagesForPreferences.properties
>index 849a567..a6551c3 100644
>--- a/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/preferences/MessagesForPreferences.properties
>+++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/preferences/MessagesForPreferences.properties
>@@ -27,3 +27,7 @@
> GdbDebugPreferencePage_initialChildCountLimitForCollections_label=For collections, initially limit child count to
> 
> GdbDebugPreferencePage_defaults_label=Debug Configurations Defaults
>+
>+GdbDebugPreferencePage_timeout_group_label=Timeout for gdb commands
>+GdbDebugPreferencePage_timeout_label=Timeout (in seconds)
>+GdbDebugPreferencePage_enableTimeout_label=Enable timeout
>diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/IGdbDebugPreferenceConstants.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/IGdbDebugPreferenceConstants.java
>index acc6243..6b1c389 100644
>--- a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/IGdbDebugPreferenceConstants.java
>+++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/IGdbDebugPreferenceConstants.java
>@@ -9,6 +9,7 @@
>  *     Ericsson - initial implementation
>  *     Jens Elmenthaler (Verigy) - Added Full GDB pretty-printing support (bug 302121)
>  *     Sergey Prigogin (Google)
>+ *     Mohamed Hussein (Mentor Graphics) - Add timeout for gdb commands (Bug 361934)
>  *******************************************************************************/
> package org.eclipse.cdt.dsf.gdb;
> 
>@@ -20,6 +21,14 @@
>  */
> public interface IGdbDebugPreferenceConstants {
> 	
>+	/**
>+     * Help prefixes.
>+     * This should be used for the preference names as well, though it wasn't used from the start.
>+     * We can't change existing ones for backward compatibility, 
>+     * but all new preferences should use {@link #PREFIX}
>+     */
>+    public static final String PREFIX = GdbPlugin.PLUGIN_ID + "."; //$NON-NLS-1$
>+
> 	/**
> 	 * Boolean preference whether to enable GDB traces. Default is <code>true</code>. 
> 	 */
>@@ -86,9 +95,16 @@
> 	 */
> 	public static final String PREF_DEFAULT_NON_STOP = "defaultNonStop"; //$NON-NLS-1$
> 
>-	/**
>-     * Help prefixes.
>+    /**
>+     * Enable timeout for gdb commands.
>+     * @since 4.1
>      */
>-    public static final String PREFIX = GdbPlugin.PLUGIN_ID + "."; //$NON-NLS-1$
>+	public static final String PREF_COMMAND_TIMEOUT_ENABLE = PREFIX + "commandTimeoutEnable"; //$NON-NLS-1$
>+
>+	/**
>+	 * Default timeout for gdb commands.
>+	 * @since 4.1
>+	 */
>+	public static final String PREF_DEFAULT_COMMAND_TIMEOUT = PREFIX + "defaultCommandTimeout"; //$NON-NLS-1$
> }
> 
>diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/internal/GdbPreferenceInitializer.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/internal/GdbPreferenceInitializer.java
>index e9403f1..c35cd1a 100644
>--- a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/internal/GdbPreferenceInitializer.java
>+++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/internal/GdbPreferenceInitializer.java
>@@ -10,6 +10,7 @@
>  *     Jens Elmenthaler (Verigy) - Added Full GDB pretty-printing support (bug 302121)
>  *     Sergey Prigogin (Google)
>  *     Marc Khouzam (Ericsson) - Move to org.eclipse.cdt.dsf.gdb from UI plugin (bug 348159)
>+ *     Mohamed Hussein (Mentor Graphics) - Add timeout for gdb commands (Bug 361934)
>  *******************************************************************************/
> package org.eclipse.cdt.dsf.gdb.internal;
> 
>@@ -38,5 +39,7 @@
> 		node.putBoolean(IGdbDebugPreferenceConstants.PREF_DEFAULT_STOP_AT_MAIN, ICDTLaunchConfigurationConstants.DEBUGGER_STOP_AT_MAIN_DEFAULT);
> 		node.put(IGdbDebugPreferenceConstants.PREF_DEFAULT_STOP_AT_MAIN_SYMBOL, ICDTLaunchConfigurationConstants.DEBUGGER_STOP_AT_MAIN_SYMBOL_DEFAULT);
> 		node.putBoolean(IGdbDebugPreferenceConstants.PREF_DEFAULT_NON_STOP, IGDBLaunchConfigurationConstants.DEBUGGER_NON_STOP_DEFAULT);
>+		node.putBoolean(IGdbDebugPreferenceConstants.PREF_COMMAND_TIMEOUT_ENABLE, true);
>+		node.putInt(IGdbDebugPreferenceConstants.PREF_DEFAULT_COMMAND_TIMEOUT, 30);
> 	}
> }
>diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/AbstractMIControl.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/AbstractMIControl.java
>index 33bb3e2..04f374b 100644
>--- a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/AbstractMIControl.java
>+++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/AbstractMIControl.java
>@@ -10,6 +10,7 @@
>  *     Ericsson 		  - Modified for handling of multiple stacks and threads
>  *     Nokia - create and use backend service.
>  *     Onur Akdemir (TUBITAK BILGEM-ITI) - Multi-process debugging (Bug 237306)
>+ *     Mohamed Hussein (Mentor Graphics) - Add timeout for gdb commands (Bug 361934)
>  *******************************************************************************/
> package org.eclipse.cdt.dsf.mi.service.command;
> 
>@@ -25,11 +26,17 @@
> import java.util.List;
> import java.util.Map;
> import java.util.concurrent.BlockingQueue;
>+import java.util.concurrent.Callable;
>+import java.util.concurrent.Executors;
> import java.util.concurrent.LinkedBlockingQueue;
> import java.util.concurrent.RejectedExecutionException;
>+import java.util.concurrent.ScheduledExecutorService;
>+import java.util.concurrent.ScheduledFuture;
>+import java.util.concurrent.TimeUnit;
> 
> import org.eclipse.cdt.dsf.concurrent.DataRequestMonitor;
> import org.eclipse.cdt.dsf.concurrent.DsfRunnable;
>+import org.eclipse.cdt.dsf.concurrent.RequestMonitor;
> import org.eclipse.cdt.dsf.datamodel.DMContexts;
> import org.eclipse.cdt.dsf.datamodel.IDMContext;
> import org.eclipse.cdt.dsf.debug.service.IRunControl;
>@@ -39,6 +46,7 @@
> import org.eclipse.cdt.dsf.debug.service.command.ICommandResult;
> import org.eclipse.cdt.dsf.debug.service.command.ICommandToken;
> import org.eclipse.cdt.dsf.debug.service.command.IEventListener;
>+import org.eclipse.cdt.dsf.gdb.IGdbDebugPreferenceConstants;
> import org.eclipse.cdt.dsf.gdb.internal.GdbPlugin;
> import org.eclipse.cdt.dsf.gdb.service.GDBBackend;
> import org.eclipse.cdt.dsf.mi.service.IMICommandControl;
>@@ -59,6 +67,7 @@
> import org.eclipse.cdt.dsf.service.AbstractDsfService;
> import org.eclipse.cdt.dsf.service.DsfSession;
> import org.eclipse.core.runtime.IStatus;
>+import org.eclipse.core.runtime.Platform;
> import org.eclipse.core.runtime.Status;
> 
> import com.ibm.icu.text.MessageFormat;
>@@ -72,6 +81,12 @@
> public abstract class AbstractMIControl extends AbstractDsfService
>     implements IMICommandControl
> {
>+	/**
>+	 * Maximum number of gdb commands allowed to be on the wire at the same time.
>+	 * @since 4.1
>+	 */
>+	protected static final int NUM_OF_PARALLEL_COMMANDS = 3;
>+
> 	private static final String MI_TRACE_IDENTIFIER = " [MI]  "; //$NON-NLS-1$
> 	
>     /*
>@@ -160,7 +175,67 @@
>         fCommandFactory = factory;
>     }
> 
>-    /**
>+	@Override
>+	public void initialize(RequestMonitor rm) {
>+		super.initialize(rm);
>+        addTimeOutCommandListener(rm);
>+	}
>+
>+	/**
>+	 * Adds a timeout listener for gdb commands if the preference requires it.
>+	 * @see IGdbDebugPreferenceConstants#PREF_COMMAND_TIMEOUT_ENABLE
>+	 * @since 4.1
>+	 */
>+	protected void addTimeOutCommandListener(final RequestMonitor requestMonitor) {
>+		if (Platform.getPreferencesService().getBoolean(GdbPlugin.PLUGIN_ID,
>+				IGdbDebugPreferenceConstants.PREF_COMMAND_TIMEOUT_ENABLE, true, null)) {
>+			addCommandListener(new TimeOutCommandListener(requestMonitor));
>+		}
>+	}
>+
>+	/**
>+	 * A Timeout command listener that will shutdown the gdb connection if any of the gdb commands timed out.
>+	 * @see IGdbDebugPreferenceConstants#PREF_DEFAULT_COMMAND_TIMEOUT 
>+	 * @author mhussein
>+	 */
>+	private class TimeOutCommandListener implements ICommandListener {
>+		private ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(NUM_OF_PARALLEL_COMMANDS); 
>+
>+		private Map<ICommandToken, ScheduledFuture<?>> commands = new HashMap<ICommandToken, ScheduledFuture<?>>(NUM_OF_PARALLEL_COMMANDS);
>+		private int timeout;
>+		private final RequestMonitor rm;
>+
>+		public TimeOutCommandListener(RequestMonitor rm) {
>+			this.rm = rm;
>+			timeout  = Platform.getPreferencesService().getInt(GdbPlugin.PLUGIN_ID, IGdbDebugPreferenceConstants.PREF_DEFAULT_COMMAND_TIMEOUT, 30, null);
>+		}
>+		@Override
>+		public void commandQueued(ICommandToken token) {}
>+		@Override
>+		public void commandRemoved(ICommandToken token) {}
>+
>+		@Override
>+		public void commandSent(final ICommandToken token) {
>+			ScheduledFuture<?> task = scheduler.schedule(new Callable<Object>() {
>+				@Override
>+				public Object call() {
>+					shutdown(rm);
>+					return null;
>+				}
>+			}, timeout, TimeUnit.SECONDS);
>+			commands.put(token, task);
>+		}
>+
>+		@Override
>+		public void commandDone(ICommandToken token, ICommandResult result) {
>+			if (commands.containsKey(token)) {
>+				ScheduledFuture<?> task = commands.remove(token);
>+				task.cancel(true);
>+			}
>+		}
>+	}
>+
>+	/**
>      * Set the tracing stream for the MI communication.  If this method is never
>      * called, tracing will be off, by default.
>      * 
>@@ -319,7 +394,7 @@
>         	fCommandQueue.add(handle);
>             processCommandQueued(handle);
>             
>-            if (fRxCommands.size() < 3) {
>+            if (fRxCommands.size() < NUM_OF_PARALLEL_COMMANDS) {
>                 // In a separate dispatch cycle.  This allows command listeners 
>             	// to respond to the command queued event.  
>                 getExecutor().execute(new DsfRunnable() {

Actions: View | Diff

Attachments on bug 361934: 205924 | 206262 | 206328 | 208645 | 210213 | 210215 | 210604 | 210934 | 213410