diff options
| author | Pierre-Charles David | 2016-05-27 10:28:33 +0000 |
|---|---|---|
| committer | Pierre-Charles David | 2016-05-30 08:00:22 +0000 |
| commit | 9f8ca427badbaf2892a3e0e51a64deb0fe44828b (patch) | |
| tree | 9fd69d60e9b679007f85c4897573adf055961164 | |
| parent | 6dfbad649980487be4656ccd45c3b3ed6ae3f17d (diff) | |
| download | org.eclipse.sirius-9f8ca427badbaf2892a3e0e51a64deb0fe44828b.tar.gz org.eclipse.sirius-9f8ca427badbaf2892a3e0e51a64deb0fe44828b.tar.xz org.eclipse.sirius-9f8ca427badbaf2892a3e0e51a64deb0fe44828b.zip | |
[494611] Avoid CNFE storms on startup
Avoid static access to Messages classes (which require the bundle to be
activated to initialized properly) from the activator classes
themselves.
This caused a situation where to initialize some Sirius class (for
example one registered on a platform extension point):
1. OSGi instanciates the corresponding Sirius activator class (for
example DslCommonPlugin$Implementation). To do this it needs to
initialize its static fields/execute its static blocks.
2. If such a static initialization accesses the bundle's Messages class,
Messages itself needs to be loaded and initialized first.
3. The (static) initialization of the Messages class ends-up calling
EMF's DelegatingResourceLocator.getString(). getString() calls
getPrimaryResourceLocator(), which calls getPluginResourceLocator(),
which returns the value of the "plugin" static field. It is null at
this point because we are still in the process of creating the
instance of DslCommonPlugin$Implementation (step 1 above) that will
be its value later.
4. Seeing no ResourceLocator, EMF's DelegatingResourceLocator assumes
we're in a non-OSGi context and falls back to a costly strategy of
invoking Java's ResourceResourceBundle.getBundle(packageName +
".plugin"), which will ask all the accessible ClasLoaders for the
named resource.
5. Of course they will all fail, throwing a MissingResourceException.
DelegatingResourceLocator tries a last-chance method of looking for a
"plugin.properties" file at the base URL of the plug-in. As it
happens, this matches the location of our properties, so in the end
we get the correct value. Note that here EMF hard-codes the name and
path of the properties file. If we had put them somewhere else (and
mentioned it in the MANIFEST.MF's Bundle-Localization header), this
heuristic would not find them (but OSGi/Equinox would).
After analyzing all our activators, two actual problematic cases have
been identified, both mentioned in http://eclip.se/494611#c3, and both
treated in this patch:
- SiriusPlugin.INTER_REGISTRY: was a private static final field, is now
non-static and non-final, initialized only in the activator's start()
method.
- DslCommonPlugin.PROFILER: creates an instance of TimeProfiler2 which
ends-up accessing Messages to get some of its strings. This one is
public, so instead of breaking the API this late, we simply inline the
strings's default values for the few messages concerned. This means
they are no longer translatable, but has very little impact in
practice for the names of columns or error messages related to the
Sirius profiler.
Also change AcceleoMTLInterpreterPlugin to create the
AcceleoRegistryListener instance only on startup instead of early in the
instanciation. This is only tangentially related to this particular
issue, but still in the same area of "don't execute code before the
activator is fully initialized if it can be avoided".
Bug: 494611
Change-Id: I68ee2c2a8b6e3daa7c9339aa41df483fd83f2ae5
Signed-off-by: Pierre-Charles David <pierre-charles.david@obeo.fr>
5 files changed, 14 insertions, 16 deletions
diff --git a/plugins/org.eclipse.sirius.common.acceleo.mtl/src/org/eclipse/sirius/common/acceleo/mtl/AcceleoMTLInterpreterPlugin.java b/plugins/org.eclipse.sirius.common.acceleo.mtl/src/org/eclipse/sirius/common/acceleo/mtl/AcceleoMTLInterpreterPlugin.java index 3f5f8b54f0..61f0e425d4 100644 --- a/plugins/org.eclipse.sirius.common.acceleo.mtl/src/org/eclipse/sirius/common/acceleo/mtl/AcceleoMTLInterpreterPlugin.java +++ b/plugins/org.eclipse.sirius.common.acceleo.mtl/src/org/eclipse/sirius/common/acceleo/mtl/AcceleoMTLInterpreterPlugin.java @@ -61,7 +61,7 @@ public class AcceleoMTLInterpreterPlugin extends EMFPlugin { * The registry listener that will be used for extensions to the acceleo * interpreter. */ - private final AcceleoRegistryListener registryListener = new AcceleoRegistryListener(); + private AcceleoRegistryListener registryListener; /** * Creates an instance. @@ -73,6 +73,7 @@ public class AcceleoMTLInterpreterPlugin extends EMFPlugin { @Override public void start(final BundleContext context) throws Exception { super.start(context); + registryListener = new AcceleoRegistryListener(); Platform.getExtensionRegistry().addListener(registryListener, AcceleoRegistryListener.IMPORT_HANDLER_EXTENSION_POINT); registryListener.parseInitialContributions(); } @@ -82,6 +83,7 @@ public class AcceleoMTLInterpreterPlugin extends EMFPlugin { super.stop(context); Platform.getExtensionRegistry().removeListener(registryListener); ImportHandlerRegistry.clearRegistry(); + registryListener = null; } } } diff --git a/plugins/org.eclipse.sirius.common/src/org/eclipse/sirius/common/tools/api/profiler/ProfilerTask.java b/plugins/org.eclipse.sirius.common/src/org/eclipse/sirius/common/tools/api/profiler/ProfilerTask.java index 9a4fd932a5..7ff0c6396f 100644 --- a/plugins/org.eclipse.sirius.common/src/org/eclipse/sirius/common/tools/api/profiler/ProfilerTask.java +++ b/plugins/org.eclipse.sirius.common/src/org/eclipse/sirius/common/tools/api/profiler/ProfilerTask.java @@ -10,8 +10,6 @@ *******************************************************************************/ package org.eclipse.sirius.common.tools.api.profiler; -import org.eclipse.sirius.common.tools.Messages; - /** * Represents a task. * @@ -19,9 +17,9 @@ import org.eclipse.sirius.common.tools.Messages; */ public class ProfilerTask { - private static final String THE_NAME_IS_NULL = Messages.ProfilerTask_nullName; + private static final String THE_NAME_IS_NULL = "the name is null"; //$NON-NLS-1$ - private static final String THE_CATEGORY_IS_NULL = Messages.ProfilerTask_nullCategory; + private static final String THE_CATEGORY_IS_NULL = "the category is null"; //$NON-NLS-1$ /** The category. */ private String category; diff --git a/plugins/org.eclipse.sirius.common/src/org/eclipse/sirius/common/tools/api/profiler/TimeProfiler.java b/plugins/org.eclipse.sirius.common/src/org/eclipse/sirius/common/tools/api/profiler/TimeProfiler.java index 240574fe16..e82d4e2251 100644 --- a/plugins/org.eclipse.sirius.common/src/org/eclipse/sirius/common/tools/api/profiler/TimeProfiler.java +++ b/plugins/org.eclipse.sirius.common/src/org/eclipse/sirius/common/tools/api/profiler/TimeProfiler.java @@ -18,7 +18,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import org.eclipse.sirius.common.tools.Messages; import org.eclipse.sirius.ext.base.Option; /** @@ -320,7 +319,7 @@ public class TimeProfiler { */ public void addProfilerListener(final ProfilerListener listener) throws IllegalArgumentException { if (listener == null) { - throw new IllegalArgumentException(Messages.TimeProfiler_nullListener); + throw new IllegalArgumentException("the listener is null"); //$NON-NLS-1$ } this.listeners.add(listener); } diff --git a/plugins/org.eclipse.sirius.common/src/org/eclipse/sirius/common/tools/api/profiler/TimeProfiler2.java b/plugins/org.eclipse.sirius.common/src/org/eclipse/sirius/common/tools/api/profiler/TimeProfiler2.java index a5ee29da29..3e5617fdae 100644 --- a/plugins/org.eclipse.sirius.common/src/org/eclipse/sirius/common/tools/api/profiler/TimeProfiler2.java +++ b/plugins/org.eclipse.sirius.common/src/org/eclipse/sirius/common/tools/api/profiler/TimeProfiler2.java @@ -23,7 +23,6 @@ import java.util.Map.Entry; import java.util.Set; import org.eclipse.sirius.common.tools.DslCommonPlugin; -import org.eclipse.sirius.common.tools.Messages; import org.eclipse.sirius.ext.base.collect.StackEx; /** @@ -34,7 +33,7 @@ import org.eclipse.sirius.ext.base.collect.StackEx; public class TimeProfiler2 extends TimeProfiler { /** The Other task. */ - public static final ProfilerTask OTHER_TASK = new ProfilerTask(Messages.TimeProfiler2_otherCategory, Messages.TimeProfiler2_otherTaskName); + public static final ProfilerTask OTHER_TASK = new ProfilerTask("Other", "Other"); //$NON-NLS-1$//$NON-NLS-2$ /** The roots tasks. */ private List<CompositeTask> roots; @@ -263,7 +262,7 @@ public class TimeProfiler2 extends TimeProfiler { runningTask.stopTask(); } } catch (final EmptyStackException e) { - DslCommonPlugin.getDefault().error(MessageFormat.format(Messages.TimeProfiler2_emptyStackError, task.toString()), e); + DslCommonPlugin.getDefault().error(MessageFormat.format("Empty stack in stopWork. Stopped task: {0}", task.toString()), e); //$NON-NLS-1$ } } diff --git a/plugins/org.eclipse.sirius/src/org/eclipse/sirius/viewpoint/SiriusPlugin.java b/plugins/org.eclipse.sirius/src/org/eclipse/sirius/viewpoint/SiriusPlugin.java index 528d7604e1..d516c8d901 100644 --- a/plugins/org.eclipse.sirius/src/org/eclipse/sirius/viewpoint/SiriusPlugin.java +++ b/plugins/org.eclipse.sirius/src/org/eclipse/sirius/viewpoint/SiriusPlugin.java @@ -69,11 +69,6 @@ public final class SiriusPlugin extends EMFPlugin { */ private static final ModelAccessorsRegistry REGISTRY = new ModelAccessorsRegistry(SiriusUtil.DESCRIPTION_MODEL_EXTENSION); - /** - * create at the initialization to avoid synchronization cost in - * ExtendedPackageRegistry - */ - private static final InterpreterRegistry INTER_REGISTRY = new InterpreterRegistry(); /** * Creates the instance. @@ -105,6 +100,10 @@ public final class SiriusPlugin extends EMFPlugin { * The actual implementation of the Eclipse <b>Plugin</b>. */ public static class Implementation extends EclipsePlugin { + /** + * Registry of all supported interpreters. + */ + private InterpreterRegistry interRegistry; /** * The registry listener that will be used to listen to sessionFactory @@ -141,6 +140,7 @@ public final class SiriusPlugin extends EMFPlugin { @Override public void start(BundleContext context) throws Exception { super.start(context); + interRegistry = new InterpreterRegistry(); // Sets the validator for these model. EValidator.Registry.INSTANCE.put(ViewpointPackage.eINSTANCE, new EValidatorAdapter()); @@ -192,7 +192,7 @@ public final class SiriusPlugin extends EMFPlugin { * @return the global instance of {@link ExtendedPackageRegistry}. */ public InterpreterRegistry getInterpreterRegistry() { - return INTER_REGISTRY; + return interRegistry; } /** |
