Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 311332 - [eclipse] Uninstalling the launcher can lead to an NPE
Summary: [eclipse] Uninstalling the launcher can lead to an NPE
Status: RESOLVED FIXED
Alias: None
Product: Equinox
Classification: Eclipse Project
Component: p2 (show other bugs)
Version: 3.6   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: 3.6 RC1   Edit
Assignee: Ian Bull CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-02 23:56 EDT by Ian Bull CLA
Modified: 2010-05-11 13:28 EDT (History)
3 users (show)

See Also:
aniefer: review+
simon_kaegi: review+

Attachments
fix v.1 (1.40 KB, patch)
2010-05-07 18:41 EDT, Ian Bull CLA 		no flags Details | Diff
v2 (2.35 KB, patch)
2010-05-11 12:00 EDT, Andrew Niefer CLA 		no flags Details | Diff
View All 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 Ian Bull CLA 2010-05-02 23:56:10 EDT 
When we uninstall the launcher, we set the launcher name property (on the profile) to null.  If you then perform another profile change request, the profile properties get copied to an another map. Since it has a value of null, it leads to the following NPE:

java.lang.NullPointerException
        at java.util.Hashtable.put(Hashtable.java:394)
        at java.util.Hashtable.putAll(Hashtable.java:466)
        at java.util.Hashtable.<init>(Hashtable.java:197)

at org.eclipse.equinox.internal.p2.director.SimplePlanner.createSelectio
nContext(SimplePlanner.java:203)
at org.eclipse.equinox.internal.p2.director.SimplePlanner.getSolutionFor
(SimplePlanner.java:284)
at org.eclipse.equinox.internal.p2.director.SimplePlanner.getProvisionin
gPlan(SimplePlanner.java:351)

I wonder if we should remove the property if there is no launcher installed?
Comment 1 Pascal Rapicault CLA 2010-05-03 07:31:00 EDT 
You are right, we should simply remove the property.
The issue is likely located in the eclipse touchpoint where we unset the launcher.
Comment 2 Ian Bull CLA 2010-05-03 10:17:48 EDT 
(In reply to comment #1)
> You are right, we should simply remove the property.
> The issue is likely located in the eclipse touchpoint where we unset the
> launcher.

Yep, that's exactly where it is (the setLauncherName method, or something like that).  I'll grab this and get you or Simon to review.
Comment 3 Ian Bull CLA 2010-05-07 18:41:22 EDT 
Created attachment 167589 [details]
fix v.1

If the launcher name is being set to null, then we remove the property.  From the javadocs, if we call remvoe and the property doesn't exist, we're ok.
Comment 4 Ian Bull CLA 2010-05-07 18:47:29 EDT 
Simon, can you review this patch?
Comment 5 Simon Kaegi CLA 2010-05-11 09:47:49 EDT 
Make sense and I think this is fine but adding Andrew to also take a quick look.
Comment 6 Andrew Niefer CLA 2010-05-11 12:00:42 EDT 
Created attachment 167953 [details]
v2

I don't have any particular problem with the patch, except that it doesn't feel like the full fix to me.

Why are we using a Hashtable here in the SimplePlanner?  The profileProperties we are working from will be a HashMap, if it ever happens that there are other keys that have null values then we have the same problem again.

None of the uses of the selectionContext map require a dictionary, I suggest we change it to a HashMap.
Comment 7 Andrew Niefer CLA 2010-05-11 13:28:37 EDT 
Ian and I talked about this and we like the new patch.