Bugzilla – Full Text Bug Listing
| Summary: | EMF Registries who use PluginClassDescriptor are potentially broken when you handle your Bundle dynamically | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Modeling] EMF | Reporter: | Xavier Maysonnave <x.maysonnave> | ||||||
| Component: | Core | Assignee: | Ed Merks <Ed.Merks> | ||||||
| Status: | CLOSED FIXED | QA Contact: | |||||||
| Severity: | normal | ||||||||
| Priority: | P3 | ||||||||
| Version: | 2.6.0 | ||||||||
| Target Milestone: | --- | ||||||||
| Hardware: | PC | ||||||||
| OS: | Windows XP | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
This logic to remove it should kick in, doesn't it?
Collection<IChildCreationExtender.Descriptor> collection = childCreationExtenderDescriptorRegistry.get(packageURI);
if (add)
{
if (collection == null)
{
childCreationExtenderDescriptorRegistry.put(packageURI, collection = new ArrayList<IChildCreationExtender.Descriptor>());
}
collection.add(new PluginChildCreationExtenderDescriptor(element, "class"));
}
else if (collection != null)
{
for (IChildCreationExtender.Descriptor descriptor : collection)
{
if (descriptor instanceof PluginChildCreationExtenderDescriptor && ((PluginChildCreationExtenderDescriptor)descriptor).matches(element))
{
collection.remove(descriptor);
break;
}
}
}
I.e., did you really test that the descriptor isn't removed?
Yes, I tested it. The descriptor isn't removed because the matches test is against an invalid IConfigurationElement and crashed... Here is the stack trace I got : rg.eclipse.core.runtime.InvalidRegistryObjectException: Invalid registry object at org.eclipse.core.internal.registry.RegistryObjectManager.basicGetObject(RegistryObjectManager.java:272) at org.eclipse.core.internal.registry.RegistryObjectManager.getObject(RegistryObjectManager.java:262) at org.eclipse.core.internal.registry.ConfigurationElementHandle.getConfigurationElement(ConfigurationElementHandle.java:26) at org.eclipse.core.internal.registry.ConfigurationElementHandle.getContributor(ConfigurationElementHandle.java:134) at org.eclipse.emf.edit.EMFEditPlugin$4$1PluginChildCreationExtenderDescriptor.matches(EMFEditPlugin.java:254) at org.eclipse.emf.edit.EMFEditPlugin$4.readElement(EMFEditPlugin.java:272) at org.eclipse.emf.ecore.plugin.RegistryReader.internalReadElement(RegistryReader.java:123) at org.eclipse.emf.ecore.plugin.RegistryReader.access$0(RegistryReader.java:121) at org.eclipse.emf.ecore.plugin.RegistryReader$1.registryChanged(RegistryReader.java:113) at org.eclipse.core.internal.registry.ExtensionRegistry$2.run(ExtensionRegistry.java:921) at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42) at org.eclipse.core.internal.registry.ExtensionRegistry.processChangeEvent(ExtensionRegistry.java:919) at org.eclipse.core.runtime.spi.RegistryStrategy.processChangeEvent(RegistryStrategy.java:260) at org.eclipse.core.internal.registry.osgi.ExtensionEventDispatcherJob.run(ExtensionEventDispatcherJob.java:50) I see! So we could either cache the contributor, which probably also becomes invalid or the contribute name, which is a string, and then compare to the String instead. See what I mean? Could you try that? Created attachment 180267 [details]
Registry Sample from org.eclipse.egf
Ed, I think a more robust implementation is possible, however it relies on internal code. Each extension point has a unique identifier and this information is not yet a public API. In the attached registry sample you could see how EGF is managing its registries. 1 - Each registry listen for a particular extension-point 2 - If suitable each of them have a public or a private unique identifier org.eclipse.core.internal.Handle (I know it's bad...) I found this code in eclipse itself and it sounds to me a good approach. Cheers Created attachment 180280 [details]
Patches to address the issue.
Given that all we want is to implement matches
public boolean matches(IConfigurationElement element)
{
return element.getContributor().equals(this.element.getContributor());
}
which compare contributors, for which the only method is getName(), I'm not sure that something needs to be more robust than comparing the name more directly by caching the contributor name as in this patch...
What needs to be more robust?
Ed, Thanks for the patch. It works as expected. By more robust I mean other registries who rely on PluginClassDescriptor. I feared other side effects on other registries. The fix has been committed to CVS for 2.7. The fixes are available in a published build. Try again to resolve as fixed (and available in a build). Oh, I guess I have to close them! |
The PluginClassDescriptor holds an IConfigurationElement of a matching extension point. Such element could become invalid. As an example the class PluginChildCreationExtenderDescriptor holds information of its RegistryReader analysed extension point 'childCreationExtenders'. As such it stores the according readed IConfigurationElement. When a bundle who uses this extension point is uninstalled and the osgi package admin is refreshed the relevant RegistryReader is notified and the PluginChildCreationExtenderDescriptor try to clean itself. However the stored IConfigurationElement is no longer valid and as such the match method crash: public boolean matches(IConfigurationElement element) { return element.getContributor().equals(this.element.getContributor()); } This report could be reproduced with an OSGI console. 1 - Start emf edit bundle to have an active ChildCreationExtender registry 2 - Start a bundle who exposes a ChildCreationExtender in its plugin.xml 3 - In the osgi console 'ss' your configuration and checked that both plugins are started (note the bundle id). 4 - use the 'uninstall id' command in the osgi console to uninstall the bundle who exposes the ChildCreationExtender 5 - refresh (at this point the PluginChildCreationExtenderDescriptor should crash as its stored IConfigurationElement is no longer valid) Thanks