[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
[cdt-dev] Scanner discovery running in main thread


Hi,

we've run into a case where scanner discovery hangs during creating new project. The reason it hangs is our internal
problem, but a bigger problem is that it also hangs entire UI. Basically,

	MBSWizardHandler.setProjectDescription(IProject, boolean, boolean, IProgressMonitor) line: 618	

is called when creating new project and this eventually results in call to

	GCCSpecsRunSIProvider(DefaultRunSIProvider).invokeProvider

while we're still in main thread. So, if gcc hangs, UI hangs. The most suspicious place in the
call chain is


org.eclipse.cdt.ui.wizards.CDTCommonProjectWizard.getRunnable(boolean, boolean)

That code has runnable, that has syncExec that creates WorkspaceModifyDelegatingOperation that is
run via

	getContainer().run(false, true, op);

The combination of syncExec and false as the first parameter means everything is run in main thread.
So, I've tried the enclosed patch 1. Did not work very well, since it results in WorkspaceOperation
in the other thread, and we're already inside WorkspaceOperation in the main thread, so deadlock
happens. I had better luck with the attached patch 2. At least, the progress monitor during project
creation can actually be clicked. Obviously, the patch is not quite ready, but I wanted to ask
whether this approach is reasonable, and worth pursuing further.

The main question is whether calling CWizardHandler.createProject in secondary thread is OK. Given
that it broke MBSWizardHandler it might as well break other's code. OTOH, we can create CWizardHandler2,
whose createProject method is clearly documented as running in secondary thread.

Thoughts?



--
Vladimir Prus
CodeSourcery / Mentor Graphics
http://www.mentor.com/embedded-software/
diff --git a/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/wizards/CDTCommonProjectWizard.java b/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/wizards/CDTCommonProjectWizard.java
index e2bf380..0801661 100644
--- a/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/wizards/CDTCommonProjectWizard.java
+++ b/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/wizards/CDTCommonProjectWizard.java
@@ -259,7 +259,7 @@ implements IExecutableExtension, IWizardWithMemory, ICDTCommonProjectWizard
 							}
 						});
 						try {
-							getContainer().run(false, true, op);
+							getContainer().run(true, true, op);
 						} catch (InvocationTargetException e) {
 							except[0] = e;
 						} catch (InterruptedException e) {
diff --git a/org.eclipse.cdt.managedbuilder.ui/src/org/eclipse/cdt/managedbuilder/ui/wizards/MBSWizardHandler.java b/org.eclipse.cdt.managedbuilder.ui/src/org/eclipse/cdt/managedbuilder/ui/wizards/MBSWizardHandler.java
index c886075..9359eee 100644
--- a/org.eclipse.cdt.managedbuilder.ui/src/org/eclipse/cdt/managedbuilder/ui/wizards/MBSWizardHandler.java
+++ b/org.eclipse.cdt.managedbuilder.ui/src/org/eclipse/cdt/managedbuilder/ui/wizards/MBSWizardHandler.java
@@ -68,6 +68,7 @@ import org.eclipse.swt.accessibility.AccessibleEvent;
 import org.eclipse.swt.events.SelectionAdapter;
 import org.eclipse.swt.events.SelectionEvent;
 import org.eclipse.swt.widgets.Composite;
+import org.eclipse.swt.widgets.Display;
 import org.eclipse.swt.widgets.Table;
 import org.eclipse.swt.widgets.TableItem;
 import org.eclipse.ui.dialogs.WizardNewProjectCreationPage;
@@ -347,15 +348,23 @@ public class MBSWizardHandler extends CWizardHandler {
 	}
 	
 	public Map<String, String> getMainPageData() {
-		WizardNewProjectCreationPage page = (WizardNewProjectCreationPage)getStartingPage();
+		final WizardNewProjectCreationPage page = (WizardNewProjectCreationPage)getStartingPage();
 		Map<String, String> data = new HashMap<String, String>();
-		String projName = page.getProjectName();
+		final String[] projName_a = new String[1];
+		final String[] location_a = new String[1];
+		Display.getDefault().syncExec(new Runnable() {
+			public void run() {
+				projName_a[0] = page.getProjectName();
+				location_a[0] = page.getLocationPath().toOSString();
+			}
+		});
+		String projName = projName_a[0];
 		projName = projName != null ? projName.trim() : EMPTY_STR; 
 		data.put("projectName", projName); //$NON-NLS-1$
 		data.put("baseName", getBaseName(projName)); //$NON-NLS-1$
 		data.put("baseNameUpper", getBaseName(projName).toUpperCase() ); //$NON-NLS-1$
 		data.put("baseNameLower", getBaseName(projName).toLowerCase() ); //$NON-NLS-1$
-		String location = page.getLocationPath().toOSString();
+		String location = location_a[0];
 		if(location == null)
 			location = EMPTY_STR;
 		data.put("location", location); //getProjectLocation().toPortableString()); //$NON-NLS-1$
@@ -549,12 +558,17 @@ public class MBSWizardHandler extends CWizardHandler {
 	}
 		
 	@Override
-	public void createProject(IProject project, boolean defaults, boolean onFinish, IProgressMonitor monitor) throws CoreException {
+	public void createProject(final IProject project, boolean defaults, boolean onFinish, IProgressMonitor monitor) throws CoreException {
 		try {
 			monitor.beginTask("", 100); //$NON-NLS-1$
 			setProjectDescription(project, defaults, onFinish, monitor);
 			doTemplatesPostProcess(project);
-			doCustom(project);
+			Display.getDefault().syncExec(new Runnable() {
+				public void run() {
+					doCustom(project);		
+				}
+			});
+			
 			monitor.worked(30);
 		} finally {
 			monitor.done();
@@ -728,14 +742,19 @@ public class MBSWizardHandler extends CWizardHandler {
 	// Methods specific for MBSWizardHandler
 
 	public IToolChain[] getSelectedToolChains() {
-		TableItem[] tis = table.getSelection();
-		if (tis == null || tis.length == 0)
-			return new IToolChain[0];
-		IToolChain[] ts = new IToolChain[tis.length];
-		for (int i=0; i<tis.length; i++) {
-			ts[i] = (IToolChain)tis[i].getData();
-		}
-		return ts;
+		final List<IToolChain> ts = new ArrayList<IToolChain>();
+		
+		Display.getDefault().syncExec(new Runnable() {
+			public void run() {
+				TableItem[] tis = table.getSelection();
+				if (tis == null || tis.length == 0)
+					return;
+				for (int i=0; i<tis.length; i++) {
+					ts.add((IToolChain)tis[i].getData());
+				}
+			}
+		});			
+		return ts.toArray(new IToolChain[0]);
 	}
 	public int getToolChainsCount() {
 		if (entryInfo == null)
diff --git a/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/wizards/CDTCommonProjectWizard.java b/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/wizards/CDTCommonProjectWizard.java
index e2bf380..de2b549 100644
--- a/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/wizards/CDTCommonProjectWizard.java
+++ b/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/wizards/CDTCommonProjectWizard.java
@@ -232,52 +232,27 @@ implements IExecutableExtension, IWizardWithMemory, ICDTCommonProjectWizard
 
 	private IRunnableWithProgress getRunnable(boolean _defaults, final boolean onFinish) {
 		final boolean defaults = _defaults;
-		return new IRunnableWithProgress() {
-			public void run(IProgressMonitor imonitor) throws InvocationTargetException, InterruptedException {
-				final Exception except[] = new Exception[1];
-				getShell().getDisplay().syncExec(new Runnable() {
-					public void run() {
-						IRunnableWithProgress op= new WorkspaceModifyDelegatingOperation(new IRunnableWithProgress() {
-							public void run(IProgressMonitor monitor) throws InvocationTargetException, InterruptedException {
-								final IProgressMonitor fMonitor;
-								if (monitor == null) {
-									fMonitor= new NullProgressMonitor();
-								} else {
-									fMonitor = monitor;
-								}
-								fMonitor.beginTask(CUIPlugin.getResourceString("CProjectWizard.op_description"), 100); //$NON-NLS-1$
-								fMonitor.worked(10);
-								try {							
-									newProject = createIProject(lastProjectName, lastProjectLocation, new SubProgressMonitor(fMonitor, 40));
-									if (newProject != null) 
-										fMainPage.h_selected.createProject(newProject, defaults, onFinish, new SubProgressMonitor(fMonitor, 40));
-									fMonitor.worked(10);
-								} catch (CoreException e) {	CUIPlugin.log(e); }
-								finally {
-									fMonitor.done();
-								}
-							}
-						});
-						try {
-							getContainer().run(false, true, op);
-						} catch (InvocationTargetException e) {
-							except[0] = e;
-						} catch (InterruptedException e) {
-							except[0] = e;
-						}
-					}
-				});
-				if (except[0] != null) {
-					if (except[0] instanceof InvocationTargetException) {
-						throw (InvocationTargetException)except[0];
-					}
-					if (except[0] instanceof InterruptedException) {
-						throw (InterruptedException)except[0];
-					}
-					throw new InvocationTargetException(except[0]);
-				}					
+		return new WorkspaceModifyDelegatingOperation(new IRunnableWithProgress() {
+			public void run(IProgressMonitor monitor) throws InvocationTargetException, InterruptedException {
+				final IProgressMonitor fMonitor;
+				if (monitor == null) {
+					fMonitor= new NullProgressMonitor();
+				} else {
+					fMonitor = monitor;
+				}
+				fMonitor.beginTask(CUIPlugin.getResourceString("CProjectWizard.op_description"), 100); //$NON-NLS-1$
+				fMonitor.worked(10);
+				try {							
+					newProject = createIProject(lastProjectName, lastProjectLocation, new SubProgressMonitor(fMonitor, 40));
+					if (newProject != null) 
+						fMainPage.h_selected.createProject(newProject, defaults, onFinish, new SubProgressMonitor(fMonitor, 40));
+					fMonitor.worked(10);
+				} catch (CoreException e) {	CUIPlugin.log(e); }
+				finally {
+					fMonitor.done();
+				}
 			}
-		};
+		});
 	}
 	
 	public IProject createIProject(final String name, final URI location) throws CoreException{
diff --git a/org.eclipse.cdt.ui/templateengine/org/eclipse/cdt/ui/templateengine/processes/OpenFiles.java b/org.eclipse.cdt.ui/templateengine/org/eclipse/cdt/ui/templateengine/processes/OpenFiles.java
index 4bd1379..c2685b4 100644
--- a/org.eclipse.cdt.ui/templateengine/org/eclipse/cdt/ui/templateengine/processes/OpenFiles.java
+++ b/org.eclipse.cdt.ui/templateengine/org/eclipse/cdt/ui/templateengine/processes/OpenFiles.java
@@ -44,6 +44,7 @@ public class OpenFiles extends ProcessRunner {
 			IFile iFile = projectHandle.getFile(fileTargetPath);
 			if (iFile.exists()) {
 				try {
+					if (false)
 					IDE.openEditor(PlatformUI.getWorkbench().getActiveWorkbenchWindow().getActivePage(),
 							iFile);
 				} catch (PartInitException e) {