Community
Participate
Working Groups
In our Eclipse RCP product we use the OSGi directive autostart=true. This revealed an interesting problem. If we have the o.e.ui.presentations.r21 plugin in our list of plugins, the autostart directive causes this plugin to start in a Daemon thread before workbench creation. The R21PresentationPlugin will call Display.getDefault() in its R21Colors class [1]-line-178. All other Eclipse agnostic SWT calls will also use Display.getDefault() (e.g. the Nebula widgets) instead of the preferred Workbench.getDisplay() method. The display that is returned, is the one that is created by the R21 initial call to Display. When the workbench starts AFTER the first call of Display.getDefault() then Display.getDefault() will return a different Display than the one created in the Workbench.createDisplay() method. Obviously, the various createPartControl methods will be called with the Workbench Display but the Nebula, or other DOIY widgets will be using the Display.getDefault() causing InvalidThreadAccess to be thrown. [1] http://dev.eclipse.org/viewcvs/viewvc.cgi/org.eclipse.ui.internal.r21presentation/src/org/eclipse/ui/internal/r21presentation/R21Colors.java?revision=1.1&view=markup&sortby=rev
Remy what would you expect nebula to call instead of Display.getDefault()? Should we call Display.getCurrent() it's still very strange that something we are having 2 displays. IIRC the assumption that Display.getDefault() is correct in Nebula because we only expect people to create/use our code from the UI-Thread
(In reply to comment #1) > Remy what would you expect nebula to call instead of Display.getDefault()? I guess the safest option is what SWT does which is to have constructors that take a widget or display. Because then you would just reuse that one instead of having to "reach out" using static methods. Of course, the above involves introducing new APIs so that's not exactly a viable solution per se. > Should we call Display.getCurrent() it's still very strange that something we > are having 2 displays. IIRC the assumption that Display.getDefault() is correct > in Nebula because we only expect people to create/use our code from the > UI-Thread I agree with your statements here. Perhaps it might be possible to delay the initialization of these workbench colours but I haven't looked at the code yet.
oh i just saw that you move this from swt to ui - I was under the impression that you moved it to nebula ;-)
#3 - This is not a Nebula problem. This is the preferred way of getting the correct display in SWT. #2 Remy, this is not only R21 plugin but also debug.ui plugin and there could be many more. I think the workbench code should accept the already created display.
just a thought: If I call Display.getDefault() in some thread and I am the first one to call it then this Thread will become the UI thread. What happens if the Thread ends because it was just some worker? The default variable in Display is still initialized.
(In reply to comment #5) > just a thought: If I call Display.getDefault() in some thread and I am the > first one to call it then this Thread will become the UI thread. What happens > if the Thread ends because it was just some worker? The default variable in > Display is still initialized. I think that this is the problem because with your autostart its some arbitary OSGi-Thread probably something those things are initialized in a static block
(In reply to comment #4) > #3 - This is not a Nebula problem. This is the preferred way of getting the > correct display in SWT. > #2 Remy, this is not only R21 plugin but also debug.ui plugin and there could > be many more. I think the workbench code should accept the already created > display. In the workbench, IWorkbench.getDisplay() is the only guaranteed method of having the workbench work correctly. It's been that way since before 3.0. No application is allowed to make display calls before the workbench creation has started ... unless they call PlatformUI.createDisplay(). You can do that at the beginning of your RCP application if you wish. That also means you should not do something that causes plugins (like the r21 presentation) to start before the workbench is up, or at least the workbench display has been created. There might be something we could do kill the r21 plugin startup if the workbench creation is not in progress. PW
(In reply to comment #4) > #2 Remy, this is not only R21 plugin but also debug.ui plugin and there could > be many more. Debug changed that code, see bug 250048. > I think the workbench code should accept the already created > display. This feels rather risky to me.
Is it save to say that display.getDefault() is broken when you use it in Eclipse? > This feels rather risky to me. Okay but if not all parties play along with this "unknown rule" then there should be some guarding code in Workbench.createDisplay() It could signal that there is already a Display thread initialized and throw a warning. ** WARNING - DISPLAY CREATED BEFORE WORKBENCH INIT COMPLETED Tools like Nebula should avoid calling Display.getDefault() but rather use their parent widget to get the Display. This is really an important pattern for SWT widget developers that want to use their widgets inside the Workbench.
(In reply to comment #9) Here is a stacktrace (from bug 250048) demonstrating that calling PreferenceConverter calls Display.getDefault(). The PreferenceConverter is called from the debug.ui plugin. Daemon Thread [Start Level Event Dispatcher] (Suspended (entry into method getDefault in Display)) Display.getDefault() line: 1632 PreferenceConverter.<clinit>() line: 81 DebugUIPreferenceInitializer.setDefault(IPreferenceStore, String, RGB, boolean) line: 186 DebugUIPreferenceInitializer.setThemeBasedPreferences(IPreferenceStore, boolean) line: 204 DebugUIPreferenceInitializer.initializeDefaultPreferences() line: 79 PreferenceServiceRegistryHelper$1.run() line: 281 SafeRunner.run(ISafeRunnable) line: 42 PreferenceServiceRegistryHelper.runInitializer(IConfigurationElement) line: 284 PreferenceServiceRegistryHelper.applyRuntimeDefaults(String, WeakReference) line: 130 PreferencesService.applyRuntimeDefaults(String, WeakReference) line: 369 DefaultPreferences.applyRuntimeDefaults() line: 166 DefaultPreferences.load() line: 237 DefaultPreferences(EclipsePreferences).create(EclipsePreferences, String, Object) line: 307 DefaultPreferences(EclipsePreferences).internalNode(String, boolean, Object) line: 543 DefaultPreferences(EclipsePreferences).node(String) line: 669 DefaultScope(AbstractScope).getNode(String) line: 38 DefaultScope.getNode(String) line: 67 ScopedPreferenceStore.getDefaultPreferences() line: 250 ScopedPreferenceStore.getPreferenceNodes(boolean) line: 285 ScopedPreferenceStore.internalGet(String) line: 475 ScopedPreferenceStore.getString(String) line: 535 PerspectiveManager.initPerspectives() line: 990 PerspectiveManager.startup() line: 253 DebugUIPlugin.start(BundleContext) line: 501 BundleContextImpl$1.run() line: 783 AccessController.doPrivileged(PrivilegedExceptionAction<T>) line: not available [native method] BundleContextImpl.startActivator(BundleActivator) line: 774 BundleContextImpl.start() line: 755 BundleHost.startWorker(int) line: 370 BundleHost(AbstractBundle).resume() line: 374 Framework.resumeBundle(AbstractBundle) line: 1067 StartLevelManager.resumeBundles(AbstractBundle[], boolean, int) line: 561 StartLevelManager.resumeBundles(AbstractBundle[], int) line: 546 StartLevelManager.incFWSL(int, AbstractBundle[]) line: 459 StartLevelManager.doSetStartLevel(int) line: 243 StartLevelManager.dispatchEvent(Object, Object, int, Object) line: 440 EventManager.dispatchEvent(Set, EventDispatcher, int, Object) line: 227 EventManager$EventThread.run() line: 337
(In reply to comment #10) > (In reply to comment #9) > > Here is a stacktrace (from bug 250048) demonstrating that calling > PreferenceConverter calls Display.getDefault(). The PreferenceConverter is > called from the debug.ui plugin. > Sorry, what is causing this bundle to be started before the workbench? If it is not the workbench, we need to fix it or understand it. Most eclipse plugins are not OSGi bundles, they cannot be started by the OSGi framework at an arbitrary time
(In reply to comment #11) > > Sorry, what is causing this bundle to be started before the workbench? Yes, there is a case of old meets new here. We rely on OSGi services in our product. They should be started. Because there are so many of them we experimented with autostarting all bundles. This was when this "bug" revealed itself. Now we need to go back to maintaining a list with autostarted bundles which is quite the anti-pattern. > is not the workbench, we need to fix it or understand it. Most eclipse plugins > are not OSGi bundles, they cannot be started by the OSGi framework at an > arbitrary time Yes. I see and agree. Is it not possible to defer all OSGi startup activities until after the Application has started that initializes the Workbench? It is just hard to prevent calling Display.getDefault(). One should know that calling it could interfere with the Workbench. Should I file a bug for this? By the way: the debug.ui "Activator" is subclassing AbstractUIPlugin. One could imagine that plugins of this class should work with the workbench to avoid stuff like this.
This is not something that can be dealt with internally. The application starts the workbench, and workbench dependent plugins cannot start before the workbench. That's the architecture of the workbench application. It sounds like you are using the shotgun approach of starting all the bundles so the ones that provide OSGi services can register programmatically. A better solution in equinox is to use Declarative Services to make the all of the services available to the OSGi framework. As they are consumed, their contributing bundles will be started, and bundles like debug.ui will be back in their lazy-loading state. PW
> It sounds like you are using the shotgun approach of starting all the bundles > so the ones that provide OSGi services can register programmatically. A better > solution in equinox is to use Declarative Services to make the all of the > services available to the OSGi framework. As they are consumed, their > contributing bundles will be started, and bundles like debug.ui will be back in > their lazy-loading state. We do use that but the service bundles need to be started before DS can register the services they contain. So this brings us back to maintaining a list.
(In reply to comment #14) > We do use that but the service bundles need to be started before DS can > register the services they contain. So this brings us back to maintaining a > list. I don't understand, don't they just need to be installed to get DS to work? They don't need to be started (in the OSGi meaning of the word) as that would defeat the purpose of DS. In a pure OSGi world they do have to maintain a list of bundles that need to be started, they don't just start all they can find. PW
(In reply to comment #15) > (In reply to comment #14) > > We do use that but the service bundles need to be started before DS can > > register the services they contain. So this brings us back to maintaining a > > list. > > I don't understand, don't they just need to be installed to get DS to work? > They don't need to be started (in the OSGi meaning of the word) as that would > defeat the purpose of DS. I'm pretty sure bundles need to be started to let DS activate a service and bind that service into other services. Stopping a bundle brings the service down and DS then unbinds the references it has previously bound. > What do we do with this? I would like to see some notion in the error log when this occurs. I am happy to write a patch but only if others (i.e. the people in charge of accepting the patch) see the value of such a message.
we are using DS heavily in our applications and don't autostart anything. At the moement we request the service DS does the right thing.
(In reply to comment #17) > we are using DS heavily in our applications and don't autostart anything. At > the moement we request the service DS does the right thing. This is because you are using bundles that have a lazy activation policy (Bundle-ActivationPolicy: lazy). The platform (Equinox) has a default policy (see bug 177641 which shows that p2 is depending on this) which will persistently activate such bundles to get them into the "lazy" STARTING state. DS runtime understands that the bundle is in this "lazy" STARTING state and will process DS components for such bundles even though they have not reached the ACTIVE state. This can be one because bundles in the STARTING state have a valid BundleContext which DS can use to register services with. So technically you don't have an "eager" started bundles, but you do have many lazy activated bundles that have been marked for activation. We are just delaying their activation until the first class load. It sounds like Wim has other bundles with DS components which do not use the lazy activation policy (this is very common in the traditional OSGi crowd). Such bundles will not be processed by DS until they are eagerly activated.
sorry for repeating myself but this should really be a separate comment: What do we do with this bug? I would like to see some notion in the error log when this occurs. I am happy to write a patch but only if others (i.e. the people in charge of accepting the patch) see the value of such a message. What do y'all think?
(In reply to comment #9) > Okay but if not all parties play along with this "unknown rule" then there > should be some guarding code in Workbench.createDisplay() > > It could signal that there is already a Display thread initialized and throw a > warning. > > ** WARNING - DISPLAY CREATED BEFORE WORKBENCH INIT COMPLETED > (In reply to comment #19) > What do we do with this bug? I would like to see some notion in the error log > when > this occurs. I am happy to write a patch but only if others (i.e. the people in > charge of accepting the patch) see the value of such a message. It is probably a good idea for the IDEApplication. I would not put such condition in the workbench code itself. A typical UI application calls PlatformUI#createDisplay() API and is responsible for the display lifecycle. An application could get creative and, say, display a custom login screen with its own display before starting workbench, then have workbench re-use that display (hopefully, it will be in the same thread). As Paul and Tom stated, in your case the confusion was caused by altering startup order. The startup order is a very sensitive areas and really should not be casually changed. It is fine to provide hints for the bundles you develop, but changing startup order of other bundles without deep understanding of their connections is an exercise that is bound to fail. Just leave it to the OSGi to figure out :-).
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. If the bug is still relevant, please remove the "stalebug" whiteboard tag.
