Attachment 206328 Details for Bug 361934 – Implement fix according to Mikhail's suggestion

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

[patch] Implement fix according to Mikhail's suggestion

timeout_4.patch (text/plain), 17.14 KB, created by Mohamed Hussein

on 2011-11-02 09:49:55 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: Mohamed Hussein

Created: 2011-11-02 09:49:55 EDT

Size: 17.14 KB

patch

obsolete

>From 8463c75eba749893cba362a25d0a549fd71af712 Wed, 2 Nov 2011 15:43:38 +0200
>From: mhussein <mohamed_hussein@mentor.com>
>Date: Wed, 2 Nov 2011 15:43:21 +0200
>Subject: [PATCH] Fix bug 361934 - Added timeout for all gdb commands - Added user preference to enable timeout and specify timeout value.
>
>Updates:
>- Follow Mikhail's suggestion of:
>  -- Moving TimeoutCommandListener to separate file.
>  -- Call it using a step in the FinalLaunchSequence instead of directly
>- Change constant name to indicate that timeout is in sec
>
>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..c4f787a 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_S,
>+				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..3d4ef29 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 in seconds.
>+	 * @since 4.1
>+	 */
>+	public static final String PREF_DEFAULT_COMMAND_TIMEOUT_S = 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..be393f4 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_S, 30);
> 	}
> }
>diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/launching/FinalLaunchSequence.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/launching/FinalLaunchSequence.java
>index 54b7447..d162f46 100644
>--- a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/launching/FinalLaunchSequence.java
>+++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/launching/FinalLaunchSequence.java
>@@ -11,6 +11,7 @@
>  *     IBM Corporation 
>  *     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.launching;
> 
>@@ -28,6 +29,7 @@
> import org.eclipse.cdt.dsf.datamodel.DataModelInitializedEvent;
> import org.eclipse.cdt.dsf.datamodel.IDMContext;
> import org.eclipse.cdt.dsf.debug.service.ISourceLookup.ISourceLookupDMContext;
>+import org.eclipse.cdt.dsf.debug.service.command.ICommandListener;
> import org.eclipse.cdt.dsf.gdb.IGDBLaunchConfigurationConstants;
> import org.eclipse.cdt.dsf.gdb.IGdbDebugPreferenceConstants;
> import org.eclipse.cdt.dsf.gdb.actions.IConnect;
>@@ -38,6 +40,7 @@
> import org.eclipse.cdt.dsf.mi.service.CSourceLookup;
> import org.eclipse.cdt.dsf.mi.service.IMIProcesses;
> import org.eclipse.cdt.dsf.mi.service.command.CommandFactory;
>+import org.eclipse.cdt.dsf.mi.service.command.TimeOutCommandListener;
> import org.eclipse.cdt.dsf.mi.service.command.output.MIInfo;
> import org.eclipse.cdt.dsf.service.DsfServicesTracker;
> import org.eclipse.cdt.dsf.service.DsfSession;
>@@ -59,6 +62,7 @@
> 
> 	private DsfServicesTracker fTracker;
> 	private DsfSession fSession;
>+	private ICommandListener fCommandListener;
> 
> 	/**
> 	 * @since 4.0
>@@ -75,6 +79,7 @@
> 			return new String[] {
> 					"stepInitializeFinalLaunchSequence",   //$NON-NLS-1$
> 					// Global GDB settings
>+					"stepSetCommandTimeout",   //$NON-NLS-1$
> 					"stepSetEnvironmentDirectory",   //$NON-NLS-1$
> 					"stepSetBreakpointPending",    //$NON-NLS-1$
> 					"stepEnablePrettyPrinting",    //$NON-NLS-1$
>@@ -151,6 +156,33 @@
> 	}
> 
> 	/**
>+	 * Sets gdb command timeout if the {@link IGdbDebugPreferenceConstants#PREF_COMMAND_TIMEOUT_ENABLE} is enabled.
>+	 * @since 4.1
>+	 */
>+	@Execute
>+	public void stepSetCommandTimeout(RequestMonitor requestMonitor) {
>+		if (Platform.getPreferencesService().getBoolean(GdbPlugin.PLUGIN_ID,
>+				IGdbDebugPreferenceConstants.PREF_COMMAND_TIMEOUT_ENABLE, true, null)) {
>+			fCommandListener = new TimeOutCommandListener(fCommandControl, requestMonitor);
>+			fCommandControl.addCommandListener(fCommandListener);
>+		}
>+		requestMonitor.done();
>+	}
>+
>+	/**
>+	 * Removes the timeout listener.
>+	 * @since 4.1
>+	 */
>+	@RollBack("stepSetCommandTimeout")
>+	public void rollBackSetCommandTimeout(RequestMonitor requestMonitor) {
>+		if (fCommandListener != null) {
>+			fCommandControl.removeCommandListener(fCommandListener);
>+			fCommandListener = null;
>+		}
>+		requestMonitor.done();
>+	}
>+
>+	/**
> 	 * Specify GDB's working directory. 
> 	 * @since 4.0 
> 	 */
>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..12a8f63 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;
> 
>@@ -72,6 +73,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$
> 	
>     /*
>@@ -319,7 +326,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() {
>diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/TimeOutCommandListener.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/TimeOutCommandListener.java
>new file mode 100644
>index 0000000..d31b007
>--- /dev/null
>+++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/TimeOutCommandListener.java
>@@ -0,0 +1,77 @@
>+/*******************************************************************************
>+ * Copyright (c) 2011 Mentor Graphics and others.
>+ * All rights reserved. This program and the accompanying materials
>+ * are made available under the terms of the Eclipse Public License v1.0
>+ * which accompanies this distribution, and is available at
>+ * http://www.eclipse.org/legal/epl-v10.html
>+ *
>+ * Contributors:
>+ * Mentor Graphics - Initial API and implementation (Bug 361934)
>+ *******************************************************************************/
>+package org.eclipse.cdt.dsf.mi.service.command;
>+
>+import java.util.HashMap;
>+import java.util.Map;
>+import java.util.concurrent.Callable;
>+import java.util.concurrent.Executors;
>+import java.util.concurrent.ScheduledExecutorService;
>+import java.util.concurrent.ScheduledFuture;
>+import java.util.concurrent.TimeUnit;
>+
>+import org.eclipse.cdt.dsf.concurrent.RequestMonitor;
>+import org.eclipse.cdt.dsf.debug.service.command.ICommandListener;
>+import org.eclipse.cdt.dsf.debug.service.command.ICommandResult;
>+import org.eclipse.cdt.dsf.debug.service.command.ICommandToken;
>+import org.eclipse.cdt.dsf.gdb.IGdbDebugPreferenceConstants;
>+import org.eclipse.cdt.dsf.gdb.internal.GdbPlugin;
>+import org.eclipse.cdt.dsf.gdb.service.command.IGDBControl;
>+import org.eclipse.core.runtime.Platform;
>+
>+/**
>+ * A Timeout command listener that will shutdown the gdb connection if any of the gdb commands timed out.
>+ * @see IGdbDebugPreferenceConstants#PREF_DEFAULT_COMMAND_TIMEOUT_S 
>+ * @since 4.1
>+ */
>+public class TimeOutCommandListener implements ICommandListener {
>+	private final IGDBControl connectionControl;
>+	private final ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(AbstractMIControl.NUM_OF_PARALLEL_COMMANDS); 
>+	private final Map<ICommandToken, ScheduledFuture<?>> commands = new HashMap<ICommandToken, ScheduledFuture<?>>(AbstractMIControl.NUM_OF_PARALLEL_COMMANDS);
>+	private final int timeout;
>+	private final RequestMonitor rm;
>+
>+	public TimeOutCommandListener(IGDBControl igdbControl, RequestMonitor rm) {
>+		this.connectionControl = igdbControl;
>+		this.rm = rm;
>+		timeout  = Platform.getPreferencesService().getInt(GdbPlugin.PLUGIN_ID, IGdbDebugPreferenceConstants.PREF_DEFAULT_COMMAND_TIMEOUT_S, 30, null);
>+	}
>+	@Override
>+	public void commandQueued(ICommandToken token) {}
>+	@Override
>+	public void commandRemoved(ICommandToken token) {}
>+
>+	/**
>+	 * Start timeout check for the sent command.
>+	 */
>+	@Override
>+	public void commandSent(final ICommandToken token) {
>+		ScheduledFuture<?> task = scheduler.schedule(new Callable<Object>() {
>+			@Override
>+			public Object call() {
>+				connectionControl.shutdown(rm);
>+				return null;
>+			}
>+		}, timeout, TimeUnit.SECONDS);
>+		commands.put(token, task);
>+	}
>+
>+	/**
>+	 * Cancels timeout check for the command after its result is returned.
>+	 */
>+	@Override
>+	public void commandDone(ICommandToken token, ICommandResult result) {
>+		if (commands.containsKey(token)) {
>+			ScheduledFuture<?> task = commands.remove(token);
>+			task.cancel(true);
>+		}
>+	}
>+}

Actions: View | Diff

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