Hopefully I got the deadlocks fixed this time.
diff --git a/plugins/org.eclipse.jem.beaninfo/beaninfo/org/eclipse/jem/internal/beaninfo/adapters/BeaninfoNature.java b/plugins/org.eclipse.jem.beaninfo/beaninfo/org/eclipse/jem/internal/beaninfo/adapters/BeaninfoNature.java index c2f7848..909eaaa 100644 --- a/plugins/org.eclipse.jem.beaninfo/beaninfo/org/eclipse/jem/internal/beaninfo/adapters/BeaninfoNature.java +++ b/plugins/org.eclipse.jem.beaninfo/beaninfo/org/eclipse/jem/internal/beaninfo/adapters/BeaninfoNature.java
@@ -11,7 +11,7 @@ *******************************************************************************/ /* * $RCSfile: BeaninfoNature.java,v $ - * $Revision: 1.19 $ $Date: 2004/06/09 22:46:55 $ + * $Revision: 1.20 $ $Date: 2004/06/11 15:35:03 $ */ import java.io.*; @@ -352,37 +352,31 @@ return fRegistry; } - private Object createSemaphore = new Object(); // Semaphore so that only one create can happen at a time. /* * This is <package-protected> so that only the appropriate create job in this * package can call it. This is because this must be controlled to only be * done when build not in progress and serial access. - * If waitForBuild is passed onto launching the registry. If we suspended the builds, then we must not - * wait, or a deadlock will occur. */ - void createRegistry(IProgressMonitor pm, boolean waitForBuild) { - // synchronized on createsemaphore so that only one can create on this particular project at a time. - synchronized (createSemaphore) { - pm.beginTask(BeanInfoAdapterMessages.getString("UICreateRegistryJobHandler.StartBeaninfoRegistry"), 100); //$NON-NLS-1$ - if (isRegistryCreated()) { - pm.done(); - return; // It had already been created. Could of been because threads were racing to do the creation, and one got there first. - } + void createRegistry(IProgressMonitor pm) { + pm.beginTask(BeanInfoAdapterMessages.getString("UICreateRegistryJobHandler.StartBeaninfoRegistry"), 100); //$NON-NLS-1$ + if (isRegistryCreated()) { + pm.done(); + return; // It had already been created. Could of been because threads were racing to do the creation, and one got there first. + } - try { - ConfigurationContributor configurationContributor = (ConfigurationContributor) getConfigurationContributor(); - configurationContributor.setNature(this); - ProxyFactoryRegistry registry = ProxyLaunchSupport.startImplementation(fProject, "Beaninfo", //$NON-NLS-1$ - new IConfigurationContributor[] { configurationContributor}, waitForBuild, new SubProgressMonitor(pm, 100)); - registry.addRegistryListener(registryListener); - synchronized(this) { - fRegistry = registry; - } - } catch (CoreException e) { - BeaninfoPlugin.getPlugin().getLogger().log(e.getStatus()); - } finally { - pm.done(); + try { + ConfigurationContributor configurationContributor = (ConfigurationContributor) getConfigurationContributor(); + configurationContributor.setNature(this); + ProxyFactoryRegistry registry = ProxyLaunchSupport.startImplementation(fProject, "Beaninfo", //$NON-NLS-1$ + new IConfigurationContributor[] { configurationContributor}, false, new SubProgressMonitor(pm, 100)); + registry.addRegistryListener(registryListener); + synchronized(this) { + fRegistry = registry; } + } catch (CoreException e) { + BeaninfoPlugin.getPlugin().getLogger().log(e.getStatus()); + } finally { + pm.done(); } }
diff --git a/plugins/org.eclipse.jem.beaninfo/beaninfo/org/eclipse/jem/internal/beaninfo/adapters/CreateRegistryJobHandler.java b/plugins/org.eclipse.jem.beaninfo/beaninfo/org/eclipse/jem/internal/beaninfo/adapters/CreateRegistryJobHandler.java index 38fb4d0..35386d7 100644 --- a/plugins/org.eclipse.jem.beaninfo/beaninfo/org/eclipse/jem/internal/beaninfo/adapters/CreateRegistryJobHandler.java +++ b/plugins/org.eclipse.jem.beaninfo/beaninfo/org/eclipse/jem/internal/beaninfo/adapters/CreateRegistryJobHandler.java
@@ -10,19 +10,24 @@ *******************************************************************************/ /* * $RCSfile: CreateRegistryJobHandler.java,v $ - * $Revision: 1.5 $ $Date: 2004/06/09 22:46:55 $ + * $Revision: 1.6 $ $Date: 2004/06/11 15:35:03 $ */ package org.eclipse.jem.internal.beaninfo.adapters; +import java.util.logging.Level; + import org.eclipse.core.resources.ResourcesPlugin; import org.eclipse.core.runtime.*; import org.eclipse.core.runtime.jobs.*; + +import org.eclipse.jem.internal.beaninfo.core.BeaninfoPlugin; /** * This class is used by BeaninfoNature to handle the creation of the registry, - * either as a job queued off to the UI thread, or directly in case either in - * UI or UI not started. This class will be a singleton. + * This class will be a singleton. It is needed to handle if UI active without + * requiring UI plugin. (So headless will work too). The subclass <code>UICreateRegistryJobHandler</code> + * will be instantiated in case of UI active. * @since 1.0.0 */ class CreateRegistryJobHandler { @@ -47,6 +52,24 @@ } } + // See if Autobuild sleeping or waiting. This could be a race condition for us. We can't wait for it + // because we may already have the build rule locked by our thread. No way of testing this if beginRule was used. + // We can test if we are a build job (not an inline build), and if so, just go on. + // Maybe we can figure out in future if we find race condition happens significant amount of time. + IJobManager jobManager = Platform.getJobManager(); + Job currentJob = jobManager.currentJob(); + if (currentJob == null || (!currentJob.belongsTo(ResourcesPlugin.FAMILY_AUTO_BUILD) && !currentJob.belongsTo(ResourcesPlugin.FAMILY_MANUAL_BUILD))) { + // See if autojob is waiting or sleeping. + Job[] autojobs = jobManager.find(ResourcesPlugin.FAMILY_AUTO_BUILD); + for (int i = 0; i < autojobs.length; i++) { + int state = autojobs[i].getState(); + if (state == Job.WAITING || state == Job.SLEEPING) { + BeaninfoPlugin.getPlugin().getLogger().log("Build job waiting when trying to start beaninfo registry. Possible race.", Level.WARNING); // $NON-NLS-1$ + break; + } + } + } + jobHandler.processCreateRegistry(nature); } @@ -59,48 +82,53 @@ * * @since 1.0.0 */ - protected void processCreateRegistry(BeaninfoNature nature) { - doCreateRegistry(nature, new NullProgressMonitor()); - } - - private int suspendedCount = 0; // Number of nested suspends. Resume will occur only when it goes back to zero. - - private boolean[] startSemaphore = new boolean[1]; // Used to tell caller when suspend job has suspended the build ruile. - private boolean[] stopSemaphore = new boolean[1]; // Used to tell when to stop. The [0] value is whether stop requested or not. - // The purpose of this job is to suspend the builds while beaninfo is creating its registries. This is to - // prevent deadlocks between a beaninfo start reqistry request and a start request for beaninfo coming from - // a builder. It is a separate job so that if several overlapping beaninfo registry requests come in, then - // the first one in will suspend the build, and the build will not resume until the last one out. The first - // one in my finish before the next one, so we don't want builds to resume until all have completed. - private Job suspendJob = new Job("Suspend builds for BeanInfo") { //$NON-NLS-1$ - - { - this.setSystem(true); // Don't show these to users. - // The rule for running this is build rule. This is how it stops the builders - this.setRule(ResourcesPlugin.getWorkspace().getRuleFactory().buildRule()); + protected void processCreateRegistry(final BeaninfoNature nature) { + IJobManager jobManager = Platform.getJobManager(); + ISchedulingRule buildRule = ResourcesPlugin.getWorkspace().getRuleFactory().buildRule(); + boolean gotRuleLocally = true; + try { + try { + jobManager.beginRule(buildRule, new NullProgressMonitor()); + } catch (IllegalArgumentException e) { + gotRuleLocally = false; // This thread already had a rule, and it conflicted with the build rule, so we need to spawn off. + } + if (gotRuleLocally) + doCreateRegistry(nature, new NullProgressMonitor()); + } finally { + jobManager.endRule(buildRule); // Whether we got the rule or not, we must do endrule. } - protected IStatus run(IProgressMonitor monitor) { - synchronized (startSemaphore) { - startSemaphore[0] = true; - startSemaphore.notifyAll(); - } - - // Now we just wait until suspend count goes to 0. We will be told when this happens. - synchronized (stopSemaphore) { - while (!stopSemaphore[0]) { - try { - stopSemaphore.wait(); - } catch (InterruptedException e) { - } + if (!gotRuleLocally) { + // Spawn off to a job and wait for it. Hopefully we don't have a deadlock somewhere. + Job doCreateJob = new Job(BeanInfoAdapterMessages.getString("UICreateRegistryJobHandler.StartBeaninfoRegistry")) { + + protected IStatus run(IProgressMonitor monitor) { + doCreateRegistry(nature, monitor); + return Status.OK_STATUS; + } + }; + doCreateJob.schedule(); + while (true) { + try { + doCreateJob.join(); + break; + } catch (InterruptedException e) { } } - return Status.OK_STATUS; } - }; - + } + /* - * Do the creation. + * Do the creation. It is expected that the build rule has already been given to this thread. + * It is important that the build rule be given to this thread. This is so that a build won't + * start trying to create the same registry (which has happened in the past) at the same time + * a different thread was trying to start the registry. You would either have a deadlock, or + * a race and get two different registries started. + * + * The build rule also means that all beaninfo registry creations will be serialized and have + * a race condition. The unfortunate part is that two independent project's registries can't be + * created at same time. But that is the result of the build rule. We can't allow the builds, so + * we need to stop all parallel beaninfo registry creations. * * @param nature * @param pm @@ -108,66 +136,10 @@ * @since 1.0.0 */ protected final void doCreateRegistry(BeaninfoNature nature, IProgressMonitor pm) { - pm.beginTask("", 400); //$NON-NLS-1$ - IJobManager jobManager = Platform.getJobManager(); - Job currentJob = jobManager.currentJob(); + pm.beginTask("", 100); //$NON-NLS-1$ try { - if (currentJob == null || (!currentJob.belongsTo(ResourcesPlugin.FAMILY_AUTO_BUILD) && !currentJob.belongsTo(ResourcesPlugin.FAMILY_MANUAL_BUILD))) { - // We are not in the build, so suspend the rule. But first, wait for the builds to complete. This - // is because we need the builds to be completed (so that we see the changed files), but we can't - // let it happen later. This is because there can be a deadlock between the build and this thread - // if we let the build (through some builder which is trying to do introspection) lock on this - // project after we've locked on this project. Therefor we need to complete the build, and then - // stop it from running again until we've completed on this project. - if (jobManager.find(ResourcesPlugin.FAMILY_AUTO_BUILD).length > 0 || jobManager.find(ResourcesPlugin.FAMILY_MANUAL_BUILD).length >0) { - try { - jobManager.join(ResourcesPlugin.FAMILY_AUTO_BUILD, new SubProgressMonitor(pm, 100)); - jobManager.join(ResourcesPlugin.FAMILY_MANUAL_BUILD, new SubProgressMonitor(pm, 100)); - } catch (InterruptedException e) { - // Canceled, go on. - } - } else - pm.worked(200); - - synchronized(this) { - if (suspendedCount++ == 0) { - // Only start the suspend job on the first suspend request. - // We can not let anyone else get past here until we know the - // build has been halted. - synchronized (stopSemaphore) { - stopSemaphore[0] = false; // Reset it for next start - stopSemaphore.notifyAll(); // Just in case job is still waiting. - } - synchronized (startSemaphore) { - startSemaphore[0] = false; - suspendJob.schedule(); - while (!startSemaphore[0]) { - try { - startSemaphore.wait(); // Wait for suspend job to say it has the build rule suspended. - } catch (InterruptedException e) { - } - } - } - } - } - } else - pm.worked(300); - // Don't wait for the build to finish. Either we suspended the build, or we are in the build. In either case - // we can't wait for the build or we would deadlock. - nature.createRegistry(new SubProgressMonitor(pm, 100), false); + nature.createRegistry(new SubProgressMonitor(pm, 100)); } finally { - synchronized (this) { - // No matter what happens we need to stop the suspend job if we are the last. - if (--suspendedCount <= 0) - suspendedCount = 0; - if (suspendJob.getState() != Job.NONE) { - // The job is running. - synchronized (stopSemaphore) { - stopSemaphore[0] = true; - stopSemaphore.notifyAll(); // Tell suspend job to stop. - } - } - } pm.done(); } }
diff --git a/plugins/org.eclipse.jem.beaninfo/beaninfo/org/eclipse/jem/internal/beaninfo/adapters/UICreateRegistryJobHandler.java b/plugins/org.eclipse.jem.beaninfo/beaninfo/org/eclipse/jem/internal/beaninfo/adapters/UICreateRegistryJobHandler.java index 4927c34..4a7c8f4 100644 --- a/plugins/org.eclipse.jem.beaninfo/beaninfo/org/eclipse/jem/internal/beaninfo/adapters/UICreateRegistryJobHandler.java +++ b/plugins/org.eclipse.jem.beaninfo/beaninfo/org/eclipse/jem/internal/beaninfo/adapters/UICreateRegistryJobHandler.java
@@ -10,17 +10,21 @@ *******************************************************************************/ /* * $RCSfile: UICreateRegistryJobHandler.java,v $ - * $Revision: 1.3 $ $Date: 2004/06/04 15:29:34 $ + * $Revision: 1.4 $ $Date: 2004/06/11 15:35:03 $ */ package org.eclipse.jem.internal.beaninfo.adapters; import java.lang.reflect.InvocationTargetException; import java.util.logging.Level; -import org.eclipse.core.runtime.IProgressMonitor; +import org.eclipse.core.resources.ResourcesPlugin; +import org.eclipse.core.runtime.*; +import org.eclipse.core.runtime.jobs.IJobManager; +import org.eclipse.core.runtime.jobs.ISchedulingRule; import org.eclipse.jface.operation.IRunnableWithProgress; import org.eclipse.swt.widgets.Display; import org.eclipse.ui.PlatformUI; +import org.eclipse.ui.progress.IProgressService; import org.eclipse.jem.internal.beaninfo.core.BeaninfoPlugin; @@ -30,7 +34,7 @@ * loaded except if ui plugin is available. * * It will check to see if UI is running, and if it is not, then let super class handle. - * If it is running, then if this is the UI thread, use progress service, else if not then + * If it is running, then if this is the UI thread, do special but run in ui, else if not then * let super handle it normally. * * @since 1.0.0 @@ -47,22 +51,52 @@ if (Display.getCurrent() == null) super.processCreateRegistry(nature); // We are not in the UI thread. Do normal. else { - // We are in the UI, so use the progress service to farm off to another thread and keep the UI active, though disabled. - try { - PlatformUI.getWorkbench().getProgressService().busyCursorWhile(new IRunnableWithProgress() { + // We are in the UI, so try to run in UI. Really don't want to do this, but we need to + // get the build rule, and no way to know if the UI thread already has the build rule. + // It would be nice to spawn off thread to do it, but if we had the build rule we couldn't + // get it for the spawned thread. So hopefully it won't be long. + IProgressService ps = PlatformUI.getWorkbench().getProgressService(); + final ISchedulingRule buildRule = ResourcesPlugin.getWorkspace().getRuleFactory().buildRule(); - public void run(IProgressMonitor monitor) throws InterruptedException { - doCreateRegistry(nature, monitor); - if (monitor.isCanceled()) - throw new InterruptedException(); +// TODO Note: For now, runInUI is just way too annoying. It always flashes up. It is supposed to only flash up +// when it is blocked by the rule. I've opened 66683 to address this. I've also opened 66690 to be able +// to test if the thread has the rule so that runInUI will only be used if UI thread had the rule. +// For now take the risk that the UI thread won't ever have the rule. This is pretty likely to be true. +// final boolean[] gotRuleLocally = new boolean[] {false}; +// try { +// ps.runInUI(ps, new IRunnableWithProgress() { +// +// public void run(IProgressMonitor monitor) throws InvocationTargetException, InterruptedException { +// gotRuleLocally[0] = true; +// doCreateRegistry(nature, new NullProgressMonitor()); +// } +// }, buildRule); +// } catch (InvocationTargetException e) { +// if (e.getCause() instanceof IllegalArgumentException && !gotRuleLocally[0]) { +// // Error was because rule conflicted with currect rules for this thread. So do in separate thread, should be able to get rule there. + try { + ps.busyCursorWhile(new IRunnableWithProgress() { + + public void run(IProgressMonitor monitor) throws InvocationTargetException, InterruptedException { + IJobManager jm = Platform.getJobManager(); + monitor.beginTask("", 200); + try { + jm.beginRule(buildRule, new SubProgressMonitor(monitor, 100)); + doCreateRegistry(nature, new SubProgressMonitor(monitor, 100)); + } finally { + jm.endRule(buildRule); + monitor.done(); + } + } + }); + } catch (InterruptedException e1) { + } catch (InvocationTargetException e2) { + BeaninfoPlugin.getPlugin().getLogger().log(e2.getCause(), Level.WARNING); } - }); - } catch (InvocationTargetException e) { - BeaninfoPlugin.getPlugin().getLogger().log(e.getCause(), Level.WARNING); - } catch (InterruptedException e) { - // It was cancelled, so we just go on and launch. - } - +// } else +// BeaninfoPlugin.getPlugin().getLogger().log(e.getCause(), Level.WARNING); +// } catch (InterruptedException e) { +// } } } else super.processCreateRegistry(nature); // Workbench not running, do default.
diff --git a/plugins/org.eclipse.jem.workbench/workbench/org/eclipse/jem/internal/adapters/jdom/JavaReflectionSynchronizer.java b/plugins/org.eclipse.jem.workbench/workbench/org/eclipse/jem/internal/adapters/jdom/JavaReflectionSynchronizer.java index b91e1a8..b624af1 100644 --- a/plugins/org.eclipse.jem.workbench/workbench/org/eclipse/jem/internal/adapters/jdom/JavaReflectionSynchronizer.java +++ b/plugins/org.eclipse.jem.workbench/workbench/org/eclipse/jem/internal/adapters/jdom/JavaReflectionSynchronizer.java
@@ -11,7 +11,7 @@ *******************************************************************************/ /* * $RCSfile: JavaReflectionSynchronizer.java,v $ - * $Revision: 1.4 $ $Date: 2004/06/09 22:47:06 $ + * $Revision: 1.5 $ $Date: 2004/06/11 15:35:06 $ */ import java.util.ArrayList; @@ -113,11 +113,9 @@ } break; } - case IJavaElementDelta.REMOVED : { - if (!element.isWorkingCopy()) + case IJavaElementDelta.REMOVED : disAssociateSourcePlusInner(getFullNameFromElement(element)); break; - } } } /**