Threading issues of aspect negotiators

[stephan]

In the current implementation (approaching the 1.2.8 release), using the AspectAsker or similar aspect negotiators can cause threading issues:

• for solving #202 I introduced a new thread for instantiating teams while processClass tries to wait for the new thread's completion.
  • this solution is not 100% safe, meaning that individual aspect bindings could fail to adapt some early executed base code, if load-order is unfortunate.
• the AspectAsker needs access to the SWT display, but if triggered from a new thread, this will cause errors like Illegal thread access or Not implemented [multiple displays] from SWT.
• attempts to force initialization of the workspace location as early as possible made some plugins like org.eclipse.jface un-adaptable.

[stephan]

OK, I have checked in a series of patches that should improve on this issue: r19820, r19821, r19822.

The UI part heavily involves various threading-tricks:

EarlyDisplay basically implements two very different strategies, transparently selecting the appropriate strategy, based on the state we're called in:

• if no Display has been created yet:
  • do create a new one
  • access it directly as it belongs to the current thread
  • block any action from the org.eclipse.osgi.service.runnable.StartupMonitor, which is achieved by registering another service instance with no-op implementation and rank it highest so it actually overrides the already registered org.eclipse.core.runtime.internal.adaptor.DefaultStartupMonitor.
  • dispose this display after use so that the main display will be created on the right thread later during initialization.
• if a Display has already been created (see also comments in org.objectteams.otdt.earlyui.EarlyUISynchronizer.safeEarlySyncExec(Runnable)):
  • basically perform a syncExec to run our code in the UI thread, but
  • use a special Synchronizer to keep other requests out while we are operating. This is to avoid deadlocks involving UI threading and class loading:
    • block further syncExec requests
    • re-queue asyncExec requests to be explicitly invoked after we're done
    • schedule one prioritized asyncExec for restoring the original Synchronizer after we have left the critical section (UI thread).

The core part replaces a previous strategy from #202 with a hopefully safer variant:

• #202 was unsafe because in rare situations the SimplePrioritizer still had to let processClass finish, before required team instantiation was complete leading to potentially unadapted early code.
• additionally, NoClassDefFoundErrors have been reported when using the AspectAsker (incomplete analysis of what caused these, but apparently load order/threading played a role here, too).

New strategy:

• Don't wait for Eclipse to bring up the dialog for workspace selection (see #202), but do so actively, using a copy of that dialog hosted by the new bundle org.objectteams.otdt.earlyui (this variant of the dialog heavily relies on EarlyDisplay).
  Note, that this bundle is purely optional: if running in a non-Eclipse/OTDT setting we don't want this Eclipse specific dialog.
  TODO: We might want to further separate the EarlyDisplay as used by the AspectAsker from the workspace selection dialog, such that the AspectAsker can also be used in non-Eclipse/OTDT settings.
• If the AspectPermissionManager is not ready when asked #checkTeamBinding() or #checkForcedExports() (because workspace could not yet be accessed) postpone checking of these obligations until the workspace becomes ready. If at that time we detect that we loaded/activated unconfirmed code, deactivate and stop those teams/bundles.

Leaving this ticket open only to await test results and further field experiments.

[stephan]

It seems the solution as of r19822 is the best we can do.

With this patch in place most launches work just fine. I did encounter one deadlock which I dicuss in Eclipse bug 271488. For the time being I see no way we can avoid that kind of deadly embrace :(

Yet, it seems that the probability of the deadlock is quite low. Actually I had to set the default permission for aspect bindings to UNDEFINED in order to pop up the dialog for each requested aspect binding. One of these dialogs eventually blocked the system.

The potential damage of this deadlock is also low, because it only happens during start-up at what time no work is lost nor state corrupted should the workbench have to be killed.

On the long run the best strategy will be to make OT/Equinox much more lazy, which could be achieved once we are able to perform true runtime weaving. With less activity during start-up likelihood increases that the workbench is ready up and running to some degree when aspect binding requests come in and request the dialog to show.

[stephan]

For some extra safety the aspect asker as per r19835 now alternatively uses Swing for its dialogs, which uses a different dispatch thread :) It looks funny to have Eclipse open Swing dialogs but it works smoothly.

So, the (rare) deadlock mentioned in the previous comment can now be avoided altogether. Should anybody still want to see the SWT dialog just use this property

otequinox.aspectasker.useswing

Setting this to false will revert to SWT, default is now Swing, however. (Set this either in eclipse.ini or pass -vmargs -D.. at the command line, but consider that all vmargs defined in eclipse.ini will be discarded if you use the -vmargs command line switch.)