Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 311932 - ClasspathContainerVirtualComponent throws NPE on workspace restart
Summary: ClasspathContainerVirtualComponent throws NPE on workspace restart
Status: RESOLVED FIXED
Alias: None
Product: WTP Common Tools
Classification: WebTools
Component: wst.common (show other bugs)
Version: 3.2   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.2 RC1   Edit
Assignee: Jason Peterson CLA
QA Contact: Carl Anderson CLA
URL:
Whiteboard: PMC_approved
Keywords:
: 312678 (view as bug list)
Depends on:
Blocks: 312678
  Show dependency tree
 
Reported: 2010-05-06 12:58 EDT by Jason Peterson CLA
Modified: 2010-11-08 04:33 EST (History)
3 users (show)

See Also:
jasonpet: pmc_approved? (david_williams)
jasonpet: pmc_approved? (raghunathan.srinivasan)
jasonpet: pmc_approved? (naci.dai)
deboer: pmc_approved+
jasonpet: pmc_approved? (neil.hauge)
jasonpet: pmc_approved? (kaloyan)
ccc: review+


Attachments
patch (1.13 KB, patch)
2010-05-06 12:58 EDT, Jason Peterson CLA
ccc: iplog+
Details | Diff
Allows for re-fetching later (2.14 KB, patch)
2010-05-07 03:02 EDT, Rob Stryker CLA
no flags Details | Diff
Applies cleanly (2.28 KB, patch)
2010-05-11 21:27 EDT, Rob Stryker CLA
no flags Details | Diff
move the check for a null containerEntries into loadContainerEntries() (2.51 KB, patch)
2010-05-12 13:20 EDT, Carl Anderson CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jason Peterson CLA 2010-05-06 12:58:49 EDT
Created attachment 167342 [details]
patch

An adopter ran into a scenario where they had added the EAR library to an EAR project as a Classpath Container reference from the Deployment Assembly editor.  Everything appeared OK until they restarted the workspace and got the following npe thrown to the log (close/reopen will also trigger the npe):


java.lang.NullPointerException at org.eclipse.jst.common.internal.modulecore.ClasspathContainerVirtualComponent.<init>(ClasspathContainerVirtualComponent.java:48) at org.eclipse.jst.common.internal.modulecore.ClasspathContainerReferenceResolver.resolve(ClasspathContainerReferenceResolver.java:43)
.
.

The fix is to simply and safely check for null to avoid the npe from being thrown.
Comment 1 Rob Stryker CLA 2010-05-07 03:02:09 EDT
Created attachment 167434 [details]
Allows for re-fetching later

There is a bit of uncertainty as to exactly how long virtual components stick around, at least for me, and because of this I think it's a bit irresponsible to definitely assign the member variable an empty array. If these objects stick around very long, when the container does exist, this component may still be returning the empty array. 

So with that in mind, I've attached a different patch which can re-check on occasion.
Comment 2 Carl Anderson CLA 2010-05-07 15:30:23 EDT
Rob, your latest patch does not apply cleanly- ClasspathContainerVirtualComponent was updated on 5/5.  Can you please update your patch?
Comment 3 Rob Stryker CLA 2010-05-11 21:27:17 EDT
Created attachment 168069 [details]
Applies cleanly
Comment 4 Carl Anderson CLA 2010-05-12 13:20:08 EDT
Created attachment 168206 [details]
move the check for a null containerEntries into loadContainerEntries()
Comment 5 Rob Stryker CLA 2010-05-12 13:24:11 EDT
looks good, push through for approvals now
Comment 6 Carl Anderson CLA 2010-05-12 14:08:56 EDT
After a lot of discussion about this bug, here is what I, as the component lead, have decided:

We have some great ideas on how to improve our handling of containerEntries within a ClasspathContainerVirtualComponent.  However, we are in RC1, and as such simple, safe fixes are preferable.

I opened bug 312678 to allow us to address a more thorough solution for this long term.  Short term, let's go with the original patch that Jason Peterson attached.
Comment 7 Carl Anderson CLA 2010-05-12 14:09:13 EDT
I approve of the original patch.
Comment 8 Jason Peterson CLA 2010-05-12 14:47:34 EDT
 * Explain why you believe this is a stop-ship defect. Or, if it is a
"hotbug" (requested by an adopter) please document it as such. 

On workspace restart a NPE is thrown to log and user is unable to open the deployment assembly page for the EAR in which the user had added the EAR library.

    * Is there a work-around? If so, why do you believe the work-around is
insufficient? 

The workaround is to manually edit the component file to remove the container. This is very inefficient for the user.

    * How has the fix been tested? Is there a test case attached to the
bugzilla record? Has a JUnit Test been added? 

No unit tests for this bug.

    * Give a brief technical overview. Who has reviewed this fix? 
The fix is to simply and safely check for null to avoid the npe from being
thrown.  This has been reviewed and approved by Carl Anderson.  Jason Sholl and Rob Stryker have reviewed it as well.

    * What is the risk associated with this fix? 

I see little risk with this patch
Comment 9 Jason Sholl CLA 2010-05-12 15:31:01 EDT
code checked into head for WTP 3.2 RC1
Comment 10 Rob Stryker CLA 2010-11-08 04:33:17 EST
*** Bug 312678 has been marked as a duplicate of this bug. ***