Comment 1 Martin Oberhuber 2008-02-08 09:36:01 EST

The reason for this NPE seems to be that in following line, adapter==null is the result:

ISubSystemConfigurationAdapter adapter = (ISubSystemConfigurationAdapter)ssFactory.getAdapter(ISubSystemConfigurationAdapter.class);

Since the exception only happens occasionally, it must be a timing issue. In this case, it looks like the adapter factory has not yet been loaded at the time the call is made.

By resolving bug 180519 we made adapter factories declarative and thus able to load lazily. By resolving bug 203105, we implemented automatic activation of UI plugins in a separate thread being spawned at Core initialization. It looks like if this thread is too slow activating the UI, the observed NPE can happen.

It's well known that the kind of UI activation we currently do is not ideal and needs to be improved for better lazy loading. The problem with this is, that we need to find the right places in the UI on which to call IAdapterManager.loadAdapter() to activate the plugins which host the adapter factories. Ideally, the following should hold:
  1.) Only few places with loadAdapter()
  2.) Still enough places to ensure factories are loaded in all cases
  3.) No places with loadAdapter() in the UI thread, especially not
      when opening menus.

Potentially, such a good solution is related to fixing bug 82973 in the Platform, though a fix will also work without that according to bug 82973 comment 30. Potentially, it's also related to avoiding plugin activations where possible and using information from plugin.xml markup instead. 

In the concrete case, for instance, the code just tries to display an image which should really be available in the SubSystemConfigurationProxy and thus not require activating the (files,shells,processes).ui plugin which holds the SubsystemConfigurationAdapter.

So summing up, fixing plugin activation in RSE with respect to getAdapter() will require thorough code review to either replace getAdpater() my mechisms using Markup only, or put loadAdapter() into the proper places.

Comment 2 Martin Oberhuber 2008-02-15 15:42:01 EST

bug 219166 is likely a dup of this one

Comment 3 Martin Oberhuber 2008-02-28 16:00:45 EST

*** Bug 219166 has been marked as a duplicate of this bug. ***

Comment 4 Martin Oberhuber 2008-04-02 10:38:04 EDT

*** Bug 225169 has been marked as a duplicate of this bug. ***

Comment 5 Martin Oberhuber 2008-04-02 10:44:16 EDT

*** Bug 225184 has been marked as a duplicate of this bug. ***

Comment 6 Martin Oberhuber 2008-04-10 11:30:56 EDT

Created attachment 95543 [details]
Patch fixing the issue

Attached patch fixes the issue by improving adapter loading as follows:

1.) Get rid of the forced deferred adapter loader Thread that was added with
    bug 203105 in core.files and similar Activator.start() methods

2.) Add the same Platform.getAdapterManager().loadAdapter() call into the
    various SubSystem.initializeSubSystem() calls. This leads to forced
    adapter loading when the subsystem is connected. Also, fix known 
    overriders of (un)initializeSubSystem() such that they all call super
    as it was already specified in Javadoc. Update Javadocs.

3.) For Subsystems like Local, which don't support individual connecting,
    add a call to initializSubSystem() into the resolveFilterString() methods
    such that adapters are loaded as soon as actual resources are about to 
    get shown.

4.) In some other places (showing menus, filter property page, wizard pages
    etc) where adapter==null, add a fallback to 
    Platform.getAdapterManager().loadAdapter() for loading adapters on demand.
    Some of these places will need to get reviewed to ensure smooth and fast
    operation in all cases. They are all tagged with
       "Lazy Loading:"
    type comments.


This fix needs thorough testing, it's not unlikely that I missed one or the other adapter==null instance which may lead to a NullPointerException, or may lead to silent non-availability of features, menus, property pages, icons or similar. One helper in investigation is to try and operate RSE with minimal number of plugins loaded: Window > Show View > PDE Runtime > Plug-in Registry is the view to use here, with Filter:Active Plugins Only and name filter "rse".

After this fix, rse.shells.ui / rse.files.ui / rse.processes.ui are no longer loaded by default, until a subsystem is connected or expanded, or the submenu of a subsystem is opened.


Migration Notes for Extenders:
------------------------------
Extenders which depend on their adapters being available BEFORE connect for some reason (e.g. custom images, custom actions, especially custom filter actions or icons) will need to provision themselves to eagerly load their UI plugins which declare the adapters. Also, extenders need to ensure that they properly call super.initializeSubSystem() where needed.

At some places, such eager loading may be possible to get rid of by replacing programmatic contributions with declarative (plugin.xml) contributions, e.g. for menus. One example of such a yet-to-do improvement is the "Launch Shell" action, see bug 226550.

Comment 7 Martin Oberhuber 2008-04-10 11:32:38 EDT

Patch committed:
[218304] Improve deferred adapter loading

Comment 8 Martin Oberhuber 2008-04-10 11:58:20 EDT

*** Bug 225337 has been marked as a duplicate of this bug. ***

Comment 9 Martin Oberhuber 2008-04-10 12:00:03 EDT

*** Bug 225337 has been marked as a duplicate of this bug. ***

Comment 10 Martin Oberhuber 2008-04-10 12:18:31 EDT

Marking this as API since the contract is changed how bundles are loaded, and extenders need to observe that as per the migration notes in comment 6, repeated here:

Migration Notes for Extenders:
------------------------------
Extenders which depend on their adapters being available BEFORE connect for
some reason (e.g. custom images, custom actions, especially custom filter
actions or icons) will need to provision themselves to eagerly load their UI
plugins which declare the adapters. Also, extenders need to ensure that they
properly call super.initializeSubSystem() where needed.

At some places, such eager loading may be possible to get rid of by replacing
programmatic contributions with declarative (plugin.xml) contributions, e.g.
for menus. One example of such a yet-to-do improvement is the "Launch Shell"
action, see bug 226550.