Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 468906 - NullPointerException in OpenWithMenu$1.compare (92)
Summary: NullPointerException in OpenWithMenu$1.compare (92)
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.5   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 4.5.1   Edit
Assignee: Andrey Loskutov CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-06-01 00:04 EDT by EPP Error Reports CLA
Modified: 2015-09-01 07:26 EDT (History)
4 users (show)

See Also:
daniel_megert: pmc_approved+
daniel_megert: review+

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description EPP Error Reports CLA 2015-06-01 00:04:38 EDT 
The following incident was reported via the automated error reporting:

The user provided the following details for this incident:
Tried to open an .html file with an external editor. In a first step, I choosed an external editor. I set the preference to use that external editor with all *.html files. The editor opened. When I opend the open context on that html file again, I get this exception.


    code:                   0
    plugin:                 org.eclipse.ui_3.107.0.v20150507-1945
    message:                Unhandled event loop exception
    fingerprint:            b69681d1
    exception class:        java.lang.NullPointerException
    exception message:      -
    number of children:     0
    
    java.lang.NullPointerException: null
    at org.eclipse.ui.actions.OpenWithMenu$1.compare(OpenWithMenu.java:92)
    at org.eclipse.ui.actions.OpenWithMenu$1.compare(OpenWithMenu.java:1)
    at java.util.TimSort.countRunAndMakeAscending(TimSort.java:351)
    at java.util.TimSort.sort(TimSort.java:216)
    at java.util.Arrays.sort(Arrays.java:1438)
    at java.util.Arrays$ArrayList.sort(Arrays.java:3895)
    at java.util.Collections.sort(Collections.java:175)
    at org.eclipse.ui.actions.OpenWithMenu.fill(OpenWithMenu.java:248)
    at org.eclipse.jface.action.MenuManager.doItemFill(MenuManager.java:724)
    at org.eclipse.jface.action.MenuManager.update(MenuManager.java:806)
    at org.eclipse.jface.action.MenuManager.handleAboutToShow(MenuManager.java:468)
    at org.eclipse.jface.action.MenuManager.access$1(MenuManager.java:461)
    at org.eclipse.jface.action.MenuManager$2.menuShown(MenuManager.java:493)
    at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:255)
    at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84)
    at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4230)
    at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1491)
    at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1514)
    at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1495)
    at org.eclipse.swt.widgets.Menu.menuWillOpen(Menu.java:811)
    at org.eclipse.swt.widgets.Display.windowProc(Display.java:5746)
    at org.eclipse.swt.internal.cocoa.OS.objc_msgSend(OS.java:-2)
    at org.eclipse.swt.internal.cocoa.NSMenu.popUpContextMenu(NSMenu.java:77)
    at org.eclipse.swt.widgets.Menu._setVisible(Menu.java:280)
    at org.eclipse.swt.widgets.Display.runPopups(Display.java:4149)
    at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3691)
    at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$4.run(PartRenderingEngine.java:1127)
    at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:337)
    at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1018)
    at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:156)
    at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:654)
    at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:337)
    at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:598)
    at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:150)
    at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:139)
    at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196)
    at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:134)
    at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104)
    at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:380)
    at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:235)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(NativeMethodAccessorImpl.java:-2)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:497)
    at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:669)
    at org.eclipse.equinox.launcher.Main.basicRun(Main.java:608)
    at org.eclipse.equinox.launcher.Main.run(Main.java:1515)
   
  

General Information:

    reported-by:      Marcel Bruch
    anonymous-id:     f8087d7b-9d38-4c73-aeb1-6991603c2a84
    eclipse-build-id: 4.5.0.I20150522-2000
    eclipse-product:  org.eclipse.epp.package.java.product
    operating system: MacOSX 10.10.3 (x86_64) - cocoa
    jre-version:      1.8.0_40-b25

The following plug-ins were present on the execution stack (*):
    1. org.eclipse.core.databinding.observable_1.5.0.v20150422-0725
    2. org.eclipse.core.databinding_1.5.0.v20150422-0725
    3. org.eclipse.core.runtime_3.11.0.v20150405-1723
    4. org.eclipse.e4.ui.workbench_1.3.0.v20150429-1329
    5. org.eclipse.e4.ui.workbench.swt_0.13.0.v20150504-0621
    6. org.eclipse.equinox.app_1.3.300.v20150423-1356
    7. org.eclipse.equinox.launcher_1.3.100.v20150511-1540
    8. org.eclipse.jface_3.11.0.v20150520-1503
    9. org.eclipse.swt_3.104.0.v20150522-1901
    10. org.eclipse.ui_3.107.0.v20150507-1945
    11. org.eclipse.ui.ide.application_1.1.0.v20150422-0725
    12. org.eclipse.ui.ide_3.11.0.v20150510-1749

Please note that:
* Messages, stacktraces, and nested status objects may be shortened.
* Bug fields like status, resolution, and whiteboard are sent
  back to reporters.
* The list of present bundles and their respective versions was
  calculated by package naming heuristics. This may or may not reflect reality.

Other Resources:
* Report: https://dev.eclipse.org/recommenders/committers/confess/#/problems/556bd9b6e4b0d8a020cf1c6f  
* Manual: https://dev.eclipse.org/recommenders/community/confess/#/guide


Thank you for your assistance.
Your friendly error-reports-inbox.
Comment 1 Andrey Loskutov CLA 2015-06-06 04:14:42 EDT 
I cannot reproduce, can be that it depends on the external descriptors returned by Program on Mac (I'm on Linux).

The NPE happens because one of the editor descriptors returned by the code below (OpenWithMenu.java:244) is null:

IEditorDescriptor[] editors = registry.getEditors(file.getName(), contentType);
editors = IDE.overrideEditorAssociations(editorInput, contentType, editors);
Collections.sort(Arrays.asList(editors), comparer);

So either the registry contained null descriptor or one of the EditorOverrides modified the editors and inserted null somewhere.

The fix would be to safeguard IDE, EditorRegistry and FileEditorMapping to prevent appearing null entries in IEditorDescriptor arrays.
Comment 2 Andrey Loskutov CLA 2015-06-06 18:00:45 EDT 
OK, I can reproduce now.

First, in preferences, add and make "Text editor" default for *.html file type.
After that, try to "Open With->External-><whatever editor>" and set to use that external editor with all *.html files in the dialog. 

The editor will be opened but two issues happens: the defaults aren't actually properly persisted and NPE will be logged on selecting the "Open With" menu next time (until restart).

The feature to "remember" defaults was added via bug 378485. The editor mappings were not properly persisted in case of external editors. If the file mapping for given type was already there and if *external* editor was chosen as default, the "null" editor descriptor was "remembered" for the current session and nothing was persisted at all.

There are two things to fix: 

1) EditorRegistry and IDE code must not allow "null" editor descriptors (which are internally allowed in FileEditorMapping !) to escape to outside via EditorRegistry.getEditors(String) and IDE.overrideEditorAssociations(). This will fix the NPE in all similar cases where clients (intentionally or not) re-set default editor to "null".

2) EditorSelectionDialog: should use existing descriptor to set default editor (to avoid setting null as default) and always re-set all editor mappings to properly persist user choice.

I have patches for both issues.
Comment 3 Eclipse Genie CLA 2015-07-05 13:46:51 EDT 
New Gerrit change created: https://git.eclipse.org/r/51388
Comment 4 Eclipse Genie CLA 2015-07-05 13:46:56 EDT 
New Gerrit change created: https://git.eclipse.org/r/51387
Comment 5 Eclipse Genie CLA 2015-07-05 13:46:58 EDT 
New Gerrit change created: https://git.eclipse.org/r/51389
Comment 6 Eclipse Genie CLA 2015-07-05 13:47:09 EDT 
New Gerrit change created: https://git.eclipse.org/r/51390
Comment 11 Dani Megert CLA 2015-07-15 05:00:01 EDT 
The total fix is quite big for a backport. Andrey, is there a simpler "fix" to avoid the NPE in 4.5.1?
Comment 12 Andrey Loskutov CLA 2015-07-15 05:33:43 EDT 
(In reply to Dani Megert from comment #11)
> The total fix is quite big for a backport. Andrey, is there a simpler "fix"
> to avoid the NPE in 4.5.1?

Those two changes must be *manually* re-applied on the 4.5.1:
https://git.eclipse.org/r/51389
https://git.eclipse.org/r/51389

But the rest is almost only code cleanup, which should not harm, so I think it is OK to apply all four patches on 4.5.1.
Comment 13 Dani Megert CLA 2015-07-15 08:36:35 EDT 
(In reply to Andrey Loskutov from comment #12)
> (In reply to Dani Megert from comment #11)
> > The total fix is quite big for a backport. Andrey, is there a simpler "fix"
> > to avoid the NPE in 4.5.1?
> 
> Those two changes must be *manually* re-applied on the 4.5.1:
> https://git.eclipse.org/r/51389
> https://git.eclipse.org/r/51389

Those point to the same change.


> But the rest is almost only code cleanup, which should not harm, so I think
> it is OK to apply all four patches on 4.5.1.

We tried to avoid such changes in the maintenance branches.
Comment 14 Andrey Loskutov CLA 2015-07-15 08:52:59 EDT 
(In reply to Dani Megert from comment #13)
> Those point to the same change.

oops, I meant 
https://git.eclipse.org/r/51389
https://git.eclipse.org/r/51390

I have no smaller fix to offer :-(
Comment 15 Marcel Bruch CLA 2015-07-20 06:09:23 EDT 
Is this change available on any update site so far?

And somewhat related:
Is there a way to update an existing EPP installation with the latest integration or maintenance build?


Eclipse IDE for Eclipse Committers
Version: Mars Release Candidate 1 (4.5.0RC1)
Build id: 20150521-1252
Comment 16 Andrey Loskutov CLA 2015-07-20 09:10:19 EDT 
(In reply to Marcel Bruch from comment #15)
> Is this change available on any update site so far?

AFAIK the fix is only visible on nightly platform builds on HEAD of 4.6.
Comment 17 Dani Megert CLA 2015-07-21 08:40:31 EDT 
(In reply to Andrey Loskutov from comment #16)
> (In reply to Marcel Bruch from comment #15)
> > Is this change available on any update site so far?
> 
> AFAIK the fix is only visible on nightly platform builds on HEAD of 4.6.

And I-builds.
Comment 18 Andrey Loskutov CLA 2015-07-29 09:27:29 EDT 
@Dani: I saw your mail regarding 4.5.1 plans (http://www.eclipse.org/eclipse/development/plans/freeze_plan_4_5_1.php) so what are our plans regarding this bug? 

Do we still plan to backport it? If no, we should resolve/fix it and set target to 4.6 M1.
Comment 19 Lars Vogel CLA 2015-08-13 04:03:54 EDT 
(In reply to Andrey Loskutov from comment #18)
> Do we still plan to backport it? If no, we should resolve/fix it and set
> target to 4.6 M1.
From https://www.eclipse.org/eclipse/development/plans/freeze_plan_4_5_1.php:

All fixes submitted to maintenance stream builds must have a component lead or PMC vote on the bug report, and the fix must be reviewed by an additional committer (any committer other than the one who made the fix).

+1 for this fix to downport from component lead.
Comment 20 Andrey Loskutov CLA 2015-08-13 04:28:38 EDT 
(In reply to Lars Vogel from comment #19)
> (In reply to Andrey Loskutov from comment #18)
> > Do we still plan to backport it? If no, we should resolve/fix it and set
> > target to 4.6 M1.
> From https://www.eclipse.org/eclipse/development/plans/freeze_plan_4_5_1.php:
> 
> All fixes submitted to maintenance stream builds must have a component lead
> or PMC vote on the bug report, and the fix must be reviewed by an additional
> committer (any committer other than the one who made the fix).
> 
> +1 for this fix to downport from component lead.

So what is the procedure for backport now: 
1) which patches do we plan to backport (see comment 11 and comment 14), 
2) who is going to review, 
3) and who id going to apply patches?
Comment 21 Lars Vogel CLA 2015-08-13 04:45:42 EDT 
(In reply to Andrey Loskutov from comment #20)
> So what is the procedure for backport now: 
> 1) which patches do we plan to backport (see comment 11 and comment 14),

I suggest both Gerrit reviews from comment 14, (Dani might have a different opinion on that)
 
> 2) who is going to review, 

In case you feel an additional review is needed I suggest Dani.

> 3) and who is going to apply patches?

You can do this. I typically cherry-pick patches onto maintenance and push them either directly to the remove or in case you want an additional review, push it to Gerrit.
Comment 22 Dani Megert CLA 2015-08-14 08:28:10 EDT 
(In reply to Lars Vogel from comment #21)
> (In reply to Andrey Loskutov from comment #20)
> > So what is the procedure for backport now: 
> > 1) which patches do we plan to backport (see comment 11 and comment 14),
> 
> I suggest both Gerrit reviews from comment 14, 

+1, but they need to be resubmitted against 'R4_5_maintenance' branch.

 
> > 2) who is going to review, 
> 
> In case you feel an additional review is needed I suggest Dani.

A review is needed for any bug that goes into 4.5.1. Andrey please add me to the newly submitted changes as reviewer.
Comment 23 Dani Megert CLA 2015-08-25 05:20:24 EDT 
(In reply to Dani Megert from comment #22)
> A review is needed for any bug that goes into 4.5.1. Andrey please add me to
> the newly submitted changes as reviewer.

Andrey, can you please provide the Gerrit change? 4.5.1 is closing down soon.
Comment 24 Eclipse Genie CLA 2015-08-25 18:11:07 EDT 
New Gerrit change created: https://git.eclipse.org/r/54540
Comment 25 Eclipse Genie CLA 2015-08-25 18:11:09 EDT 
New Gerrit change created: https://git.eclipse.org/r/54539
Comment 26 Andrey Loskutov CLA 2015-08-25 18:12:45 EDT 
(In reply to Dani Megert from comment #23)
> (In reply to Dani Megert from comment #22)
> > A review is needed for any bug that goes into 4.5.1. Andrey please add me to
> > the newly submitted changes as reviewer.
> 
> Andrey, can you please provide the Gerrit change? 4.5.1 is closing down soon.

https://git.eclipse.org/r/54539
https://git.eclipse.org/r/54540

I must confess I haven't tested this backport, only had time to merge it.
Comment 29 Dani Megert CLA 2015-09-01 07:26:25 EDT 
Thanks Andrey!