Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 345092

Summary: Despite constant checks, race condition exists to produce unnecessary exception
Product: [WebTools] WTP Java EE Tools Reporter: Rob Stryker <stryker>
Component: jst.j2eeAssignee: Rob Stryker <stryker>
Status: RESOLVED FIXED QA Contact: Chuck Bridgham <cbridgha>
Severity: normal    
Priority: P3 CC: kaloyan
Version: unspecifiedFlags: stryker: pmc_approved? (david_williams)
stryker: pmc_approved? (raghunathan.srinivasan)
stryker: pmc_approved? (naci.dai)
stryker: pmc_approved? (deboer)
stryker: pmc_approved? (neil.hauge)
kaloyan: pmc_approved+
stryker: pmc_approved? (cbridgha)
cbridgha: review+
Target Milestone: 3.3 RC1   
Hardware: PC   
OS: Linux   
Whiteboard: PMC_approved
Attachments:
Description Flags
Catches the IllegalStateException and returns null in the content provider none

Description Rob Stryker CLA 2011-05-09 03:07:08 EDT 
The stack trace is as follows, and exists when deleting a project which is deployed on a server. 


java.lang.IllegalStateException: The project <P/MyDynamicProject> is not accessible.
at org.eclipse.jst.jee.model.internal.JEE5ModelProvider.getModelResource(JEE5ModelProvider.java:148)
at org.eclipse.jst.jee.model.internal.Web25ModelProvider.getModelObject(Web25ModelProvider.java:44)
at org.eclipse.jst.jee.model.internal.JEE5ModelProvider.getModelObject(JEE5ModelProvider.java:217)
at org.eclipse.jst.jee.model.internal.common.AbstractMergedModelProvider.loadProviders(AbstractMergedModelProvider.java:261)
at org.eclipse.jst.jee.model.internal.common.AbstractMergedModelProvider.access$2(AbstractMergedModelProvider.java:255)
at org.eclipse.jst.jee.model.internal.common.AbstractMergedModelProvider$LoadModelsWorkspaceRunnable.run(AbstractMergedModelProvider.java:278)
at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:1975)
at org.eclipse.jst.jee.model.internal.common.AbstractMergedModelProvider.loadModel(AbstractMergedModelProvider.java:249)
at org.eclipse.jst.jee.model.internal.common.AbstractMergedModelProvider.getMergedModel(AbstractMergedModelProvider.java:219)
at org.eclipse.jst.jee.model.internal.common.AbstractMergedModelProvider.getModelObject(AbstractMergedModelProvider.java:139)
at org.eclipse.jst.jee.ui.internal.navigator.JEE5ContentProvider.getCachedContentProvider(JEE5ContentProvider.java:98)
at org.eclipse.jst.jee.ui.internal.navigator.Web25ContentProvider.getChildren(Web25ContentProvider.java:35)
at org.eclipse.ui.internal.navigator.extensions.SafeDelegateTreeContentProvider.getChildren(SafeDelegateTreeContentProvider.java:96)
at org.eclipse.ui.internal.navigator.NavigatorContentServiceContentProvider$1.run(NavigatorContentServiceContentProvider.java:150)
at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42)

Despite constant checks throughout the stack for project.isAccessible(), it is still very common that the execution makes it through to the JEE5ModelProvider and throws the runtime exception, blowing away the stack.
Comment 1 Rob Stryker CLA 2011-05-09 03:10:50 EDT 
Created attachment 195038 [details]
Catches the IllegalStateException and returns null in the content provider

Without changing the behaviour of the underlying model classes, it is most prudent for the navigator UI portion to catch and ignore this somewhat-expected error.
Comment 2 Rob Stryker CLA 2011-05-09 03:11:34 EDT 
Assigning
Comment 3 Rob Stryker CLA 2011-05-10 00:18:24 EDT 
Seeking approval on this one =]
Comment 4 Chuck Bridgham CLA 2011-05-10 09:33:47 EDT 
Fix looks good - thanks

I have also seen this one recently
Comment 5 Rob Stryker CLA 2011-05-10 12:12:57 EDT 
Chuck since we passed m7 do we need any PMC reviews on this?
Comment 6 Chuck Bridgham CLA 2011-05-10 15:14:45 EDT 
Yes just one - 

template here: http://wiki.eclipse.org/WTP_PMC_Defect_Review
Comment 7 Rob Stryker CLA 2011-05-11 00:59:10 EDT 
The answers to the following bullets must be incorporated into the bugzilla entry before a defect will be considered for PMC approval:

    * 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. 

   This is not stop-ship, but it does clutter the error log, and also blows away the stack for the navigator content. This means that that particular navigator extension will contribute / return zero results for that view refresh. 

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

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

    The fix has been manually tested. There has been no junit test. 

    * Give a brief technical overview. Who has reviewed this fix? 

   Chuck Bridgham has reviewed the patch. Technically, all that happens is that this semi-expected runtime exception is caught and ignored. This prevents the stack from being blown away, and prevents this particular navigator extension from being interrupted. 

    * What is the risk associated with this fix? 
   
   The fix is an extremely low-risk affecting no other plugins and no complicated or widely used jeetools frameworks. It is simply a UI fix in a content provider.
Comment 8 Rob Stryker CLA 2011-05-11 01:00:51 EDT 
Up for review
Comment 9 Rob Stryker CLA 2011-05-12 00:05:14 EDT 
This has been committed after Kaloyan's PMC approval...  I'm not sure if I waited long enough, and I look forward to a PMC member telling me if I committed too quickly or not.