Comment 1 Chuck Bridgham 2008-10-08 18:50:20 EDT

Created attachment 114610 [details]
Possible patch

Comment 2 Chuck Bridgham 2008-10-08 18:55:33 EDT

Added a possible patch to prevent the situation of component core metadata being used before existing on disk.  

Changed the "isFlexProject" api to also check the exitence of the metadata file.

The introduction of multithreading operations(different threads creating project metadata) introduced resource change events in non-deterministic order - and does cause the FileAdapter code to be less reliable.  It may need to be redesigned... this patch should solve this particular timing issue, but not the problems with the FileAdapter.

Comment 3 Carl Anderson 2008-10-09 23:34:06 EDT

Created attachment 114744 [details]
Updated patch to handle WTP 0.7

This tweak makes it so that existing projects created by WTP 0.7 (can you believe we still support those?) still work.

Also, it looks like the ModuleStructuralModel changes aren't necessary- WTPModulesResourceFactory has all of the necessary values already defined as static public variables- perhaps we should modify ModuleStructuralModel to reuse those?

Now, what still doesn't work is project creation.  The various *FacetInstallDelegates for Java EE modules all have code like:

// Setup the flexible project structure.
			final IVirtualComponent c = ComponentCore.createComponent(project);
			c.create(0, null);

The call to c.create() would create the .settings/org.eclipse.wst.common.component but because that file doesn't exist yet, the call to ComponentCore.createComponent() returns null, resulting in a NPE on the c.create() line.  The root cause of that is ComponentImplManager.createComponent() which checks to see if ModuleCoreNature.isFlexibleProject(), which is what this patch changes.

Comment 4 Carl Anderson 2008-10-22 12:51:34 EDT

Created attachment 115833 [details]
Updated patch to handle module creation

This version of the patch handles the only case where we don't want to check for the existance of the .settings stuff ... module creation.

Comment 5 Chuck Bridgham 2008-10-28 11:44:37 EDT

I have reviewed this patch...  and approve.

This ended up touching a few more classes because we needed to expand the usefulness of the "isFlexibleProject()" api.  while creating the project, we didn't need to check the existence of all the metadata while is was being added, but other clients of this api needed to ensure the full consistency of all required metadata that makes a project "flexible" or using the component core api.

Comment 6 Carl Anderson 2008-11-04 16:29:34 EST

This is stop ship because the inconsistency here is premature access of the contents of the .settings folder, causing empty content creation, and thus component corruption.

There is no workaround.

This has been extensively tested by running various JUnit buckets as well as testing by hand by both the Java EE team and adopter products.


This patch now makes it so that only creation scenarios will create the .settings content for WTP.  If the .settings content does not exist, and this is not a creation scenario, ModuleCoreNature.isFlexibleProject() will return false, which will prevent access attempts to the .settings content (and thus prevent the creation of empty stubs and corruption of classpath containers and such.)
Chuck Bridgham has reviewed this fix.

This is a low risk change

Comment 7 David Williams 2008-11-04 18:48:46 EST

This sounds important (even major?) and conceptually a big improvement. 
But the code changes are too complex for me to really review, so will need to trust your reviewers, and hope for maybe even a second look, just to make sure. 

For example, (an easy one), I think the following code looks sort of "dangerous" or at least sloppy ... 

try {
       IComponentImplFactory factory = findFactoryForProject(project);
       if(null != factory){
               return factory.createComponent(project);
       }
} catch (Exception e) {
       // Just return a default component
}


It seems to me just to catch any 'ol exception and to still create a default component could be improved to catch only meaningful exceptions that would imply a default component would be the right thing to return. I do not know what exceptions can be thrown here, so I'm just making a general coding comment. When I think of "catch (Exception e) I think of disk failures, networks going down, who knows what other runtime exceptions occurring ... where as I'd think you'd be expecting something more specific ... emf-resource-not-found (or something, similar to this fictional example). 

Its up to you (and, I do trust your reviewers :) ... just wanted to comment. 

Comment 8 Carl Anderson 2008-11-04 22:15:35 EST

David, in response to comment #7 (and to assuage the "fears" of any other PMC member that that comment might incur), if you look at the current ComponentImplManager.createComponent(IProject project), you will see the exact same try/catch (other than the fact that it calls factory.createComponent(Project, boolean)).  My goal here was to provide an alternate (yet consistent) path for project creation, with minimal code change.
Now, it does make sense to open a bug against WTP 3.1 for investigation into a lessening of the restrictions on that try/catch.

Comment 9 Carl Anderson 2008-11-06 01:01:14 EST

Committed to R3_0_maintenance for 3.0.3

Comment 10 Randall Theobald 2010-08-03 11:24:53 EDT

We still see this ridiculous pop-up very frequently. It seems like either this bug was used to get in a related fix (but one that did not directly prevent the situation I described) or perhaps the same issue has re-surfaced along another code path. This pop-up is very embarrassing for our product since it occurs when all the user has done is import a project interchange. No editors are open, yet this pop-up occurs that says an editor is inconsistent. It is an intermittent issue, but very annoying.

My org.eclipse.wst.common.modulecore version is 1.1.209.v20100205.

How can I help get this resolved?

Comment 11 Randall Theobald 2010-08-09 10:09:46 EDT

After some root-cause analysis, I found a fundamental problem in how our product was interacting with the WST model, which resulted in this intermittent problem. Fixing on our end.

Comment 12 Bernhard Pieber 2010-08-25 08:49:37 EDT

(In reply to comment #11)
> After some root-cause analysis, I found a fundamental problem in how our
> product was interacting with the WST model, which resulted in this intermittent
> problem. Fixing on our end.

As we have the exact same issue I would be highly interested in how you fixed it.

Comment 13 Randall Theobald 2010-08-25 09:13:20 EDT

Sure. In our case, we do some dynamic J2EE "shadow" project creation to back our logical projects. The inconsistent files pop-up would happen during WST-level modification of such a newly-created project. After a few days of deep debugging, I found that our problem was in the way we were creating our projects, and a side-effect of problems reported in bug 249951 (https://bugs.eclipse.org/bugs/show_bug.cgi?id=249951). Our initial flow was as follows:

(1) Create new empty project
(2) Lay down files in project from JET templates, including .project, .classpath, org.eclipse.wst.common.component, etc. (we used Eclipse APIs here)
(3) As an added measure, do refreshLocal infinite depth on the project
(4) Alter the WST-level project dependencies using WST APIs

The inconsistent files pop-up would happen during step 4. I found that the project natures were not getting updated immediately in step 2 or 3. Since we were modifying the .project file directly, and not using the Eclipse project nature APIs, a call to get the natures relied on the timing of when the resource change notification was processed for the .project file. Sometimes, after (3), a call to get all the natures of the project would return an empty set. This was the main problem since the WST APIs often make sure that a project has the proper nature before doing anything. I couldn't find a way to ensure that the project nature in the model would be consistent with the disk using this flow of steps. Without too many more details, in step (4) we were sometimes ending up in the inconsistent state reported by the inconsistent files pop-up (with the initial state being set by WST during a resource change notification sometime during or shortly after step (2) ).

To fix, once the problem was understood, I simply laid the files down on disk first and then created the project. This would use the correct .project file from the start and not leave a chance for the inconsistent state. This meant, however, that I had to use plain java.io calls to lay down the files instead of Eclipse APIs.

Let me know if you have any more questions.