Community
Participate
Working Groups
Build ID: 3.4..0 Steps To Reproduce: As described in Bug229633 we see the following concurrent modification exception when we have one project nested in another (with linked resources between them) and the inner project is opened after the .project file has been modified on disk. [ This occurs after a CVS update in the outer project, for example.] Exception attached. As you can see the refresh method call is modifying the aliases set while it's being iterated over in updateAliases. Test workspace attached to bug229633. Reproduction steps in bug229633 comment 28 More information: AliasManager$AddToCollectionDoit.doit(IResource) line: 56 <-- modified AliasManager$LocationMap.overLappingResourcesDo(AliasManager$Doit) line: 237 AliasManager.buildAliasedProjectsSet() line: 380 AliasManager.updateStructureChanges() line: 711 AliasManager.hasNoAliases(IResource) line: 578 AliasManager.computeAliases(IResource, IFileStore) line: 425 RefreshLocalAliasVisitor.createResource(UnifiedTreeNode, Resource) line: 34 RefreshLocalAliasVisitor(RefreshLocalVisitor).synchronizeExistence(UnifiedTreeNode, Resource) line: 216 RefreshLocalAliasVisitor(RefreshLocalVisitor).visit(UnifiedTreeNode) line: 290 UnifiedTree.accept(IUnifiedTreeVisitor, int) line: 99 FileSystemResourceManager.refreshResource(IResource, int, boolean, IProgressMonitor) line: 788 FileSystemResourceManager.refresh(IResource, int, boolean, IProgressMonitor) line: 772 Folder(Resource).refreshLocal(int, IProgressMonitor) line: 1594 Folder(Resource).createLink(URI, int, IProgressMonitor) line: 695 Project.reconcileLinksAndGroups(ProjectDescription) line: 1035 Project.updateDescription() line: 1187 File.updateMetadataFiles() line: 411 RefreshLocalVisitor.visit(UnifiedTreeNode) line: 306 UnifiedTree.accept(IUnifiedTreeVisitor, int) line: 99 FileSystemResourceManager.refreshResource(IResource, int, boolean, IProgressMonitor) line: 788 FileSystemResourceManager.refresh(IResource, int, boolean, IProgressMonitor) line: 772 AliasManager.updateAliases(IResource, IFileStore, int, IProgressMonitor) line: 682 <-- Iteration over aliases Project.open(int, IProgressMonitor) line: 906 Project.open(IProgressMonitor) line: 922 OpenResourceAction.invokeOperation(IResource, IProgressMonitor) line: 153 OpenResourceAction(WorkspaceAction).execute(List, IProgressMonitor) line: 162 WorkspaceAction$2.runInWorkspace(IProgressMonitor) line: 483 WorkspaceAction$2(InternalWorkspaceJob).run(IProgressMonitor) line: 38 Worker.run() line: 55
(In reply to comment #34) > I see what your saying, something strange though is that the variable change > in the following code: Whoops. That was clearly an incorrect backtrace. How about this one: Thread [Worker-12] (Suspended (entry into method clear in HashMap)) owns: HashSet<E> (id=4024) HashMap<K,V>.clear() line: 656 [local variables unavailable] HashSet<E>.clear() line: 211 [local variables unavailable] AliasManager.computeAliases(IResource, IFileStore) line: 429 RefreshLocalAliasVisitor.createResource(UnifiedTreeNode, Resource) line: 34 RefreshLocalAliasVisitor(RefreshLocalVisitor).synchronizeExistence(UnifiedTreeNode, Resource) line: 216 RefreshLocalAliasVisitor(RefreshLocalVisitor).visit(UnifiedTreeNode) line: 290 UnifiedTree.accept(IUnifiedTreeVisitor, int) line: 99 FileSystemResourceManager.refreshResource(IResource, int, boolean, IProgressMonitor) line: 788 FileSystemResourceManager.refresh(IResource, int, boolean, IProgressMonitor) line: 772 Folder(Resource).refreshLocal(int, IProgressMonitor) line: 1594 Folder(Resource).createLink(URI, int, IProgressMonitor) line: 695 Project.reconcileLinksAndGroups(ProjectDescription) line: 1035 Project.updateDescription() line: 1187 File.updateMetadataFiles() line: 411 RefreshLocalVisitor.visit(UnifiedTreeNode) line: 306 UnifiedTree.accept(IUnifiedTreeVisitor, int) line: 99 FileSystemResourceManager.refreshResource(IResource, int, boolean, IProgressMonitor) line: 788 FileSystemResourceManager.refresh(IResource, int, boolean, IProgressMonitor) line: 772 AliasManager.updateAliases(IResource, IFileStore, int, IProgressMonitor) line: 682 Project.open(int, IProgressMonitor) line: 906 Project.open(IProgressMonitor) line: 922
James or Serge, could you attach the test case for the issue? Are you going to investigate and fix it?
Created attachment 115550 [details] Patch to iterate over a clone of the set As per Serge's comments in bug 229633 this patch iterates over a clone of the map. Doesn't add any synchronization as it's not (to me) clear that this is needed.
(In reply to comment #2) > James or Serge, could you attach the test case for the issue? Are you going to > investigate and fix it? Hi Szymon, I'll attempt to attach a test as well.
Created attachment 115557 [details] Patch for LinkedResourceTest attached (In reply to comment #2) > James or Serge, could you attach the test case for the issue? Are you going to > investigate and fix it? Attached is a patch to the LinkedResourceTest which attempts to automate performing the tests listed in the other bug. I basically create two .project files with slightly different linked resource definitions, replacing the .project file of the inner project with each in turn. I do this in a loop 0 -> 100 and on linux the test fails sometimes between 0 & ~20... I've tried to make it fail first time, but have so far failed -- it occurs every time when I perform the steps manually... The supplied patch makes the test pass each time. I can't make the test fail on Windows but then that receives resource deltas from the filesystem, doesn't it? Also as serge described previously there are exceptions in the test cleanup when performing these linked resource tests with nested projects: org.eclipse.core.internal.resources.ResourceException: Problems encountered while deleting resources. at org.eclipse.core.internal.localstore.FileSystemResourceManager.delete(FileSystemResourceManager.java:230) at org.eclipse.core.internal.resources.ResourceTree.internalDeleteFolder(ResourceTree.java:350) at org.eclipse.core.internal.resources.ResourceTree.internalDeleteProject(ResourceTree.java:385) at org.eclipse.core.internal.resources.ResourceTree.standardDeleteProject(ResourceTree.java:835) at org.eclipse.core.internal.resources.Resource.unprotectedDelete(Resource.java:1846) at org.eclipse.core.internal.resources.Resource.delete(Resource.java:782) at org.eclipse.core.tests.resources.ResourceTest$1.run(ResourceTest.java:379) at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:1934) at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:1916) at org.eclipse.core.tests.resources.ResourceTest.cleanup(ResourceTest.java:377) at org.eclipse.core.tests.resources.ResourceTest.tearDown(ResourceTest.java:908) at org.eclipse.core.tests.resources.LinkedResourceTest.tearDown(LinkedResourceTest.java:133) at junit.framework.TestCase.runBare(TestCase.java:136) at junit.framework.TestResult$1.protect(TestResult.java:106) at junit.framework.TestResult.runProtected(TestResult.java:124) at junit.framework.TestResult.run(TestResult.java:109) But I think this is probably a problem with the testsuite rather than a problem with the test itself...
I've just found another problem with AliasManager bug251408. Basically if you override and update inner-project's .project file from outer project while inner-project is open (using CVS) you get an illegal rule push. This is because CVS grabs the scheduling rule for the project in which the update is happening, and AliasManager.updateAliases(...) when it performs localManager.refresh(...) may attempt to update linked resources in the nested project. I've attached a patch to bug 251408 to run project level / .project updates in a separate WorkspaceJob. Running project level refreshes in a separate job probably negates the need to do the clone() as provided by this patch...
I think that this should be fixed at the root cause - by disallowing project overlap as suggested by bug 245412 comment 4 (masking out the folder of the physically nested project from its container).
Created attachment 119602 [details] Test 2 (In reply to comment #5) > I do this in a loop 0 -> 100 and on linux the test fails sometimes > between 0 & ~20... I've tried to make it fail first time, but have so far > failed -- it occurs every time when I perform the steps manually... The > supplied patch makes the test pass each time. So it looks like on Linux File.lastModified() uses the stat system call and thus only has second granularity despite the API stating: "A long value representing the time the file was last modified, measured in milliseconds since the epoch (00:00:00 GMT, January 1, 1970), or 0L if the file does not exist or if an I/O error occurs." Updated the test to ensure that the modified time has changed when overwriting the .project file externally. This removes the need for a loop.
James, thanks for the test. Any volunteers to work on it?
Some file systems have up to two second granularity. We have a helper method ResourceTest.ensureOutOfSync for when we need to make a file out of sync during a test.
(In reply to comment #9) > James, thanks for the test. Any volunteers to work on it? There are a couple of patches which fix this. The first patch here does that, though a better patch is the one on bug251408. bug251408 -- alias manager should refresh project description file resource as a separate job as, in the case of overlaping projects, updating linked resources may violate already held scheduling rules (for example if the .project file of an inner project is cvs updated in the outer project). Another related bug it would be good to have fixed / have the patches reviewed: bug246221 -- patches (test and fix) for alias manager not noticing outer project closed
Created attachment 125950 [details] Patch 2 for this & bug 251408 Instead of clone() the set I schedule IProject or project description changes with the refreshmanager. This is also a simple fix for bug251408.
Created attachment 125951 [details] Test 3 (this & bug251408 Test 3. This is a test for this bug and the incorrect nested scheduling rule bug 251408. NB the test bug 251408 currently has a sleep in it. Without the sleep, the test passes. This makes me think that if you do setContents(...) within a few milliseconds of the last resource change, the resource's alias's don't receive resource change notifications... (In reply to comment #10) > Some file systems have up to two second granularity. We have a helper method > ResourceTest.ensureOutOfSync for when we need to make a file out of sync during > a test. Thanks, I had a look at this, unfortunately it also modifies the file you want changed. In these two edge cases, it's the project description that's being written and aliased in two projects.
*** Bug 251408 has been marked as a duplicate of this bug. ***
(In reply to comment #13) > NB the test bug 251408 currently has a sleep in it. I've filed a separate bug + test for this, bug 265233. I've run the full core.tests.resources testsuite. All the tests that passed before on Mac OS X continue to pass after the small change made in this patch. Szymon / John, can you review?
testChangingLinksWithNestedProjects() passes without the patch too. I'm on Win XP. Is the issue Linux specific?
(In reply to comment #16) > testChangingLinksWithNestedProjects() passes without the patch too. I'm on Win > XP. Is the issue Linux specific? Only so much as the issues are timing related. The resource needs to be out of sync when you open the project. Do you have refresh automatically on / is the windows refresh provider enabled?
BTW a quick note on my chosen fix: I always schedule refresh of a project level / project description resource by the refresh manager. This prevents both the concurrent modification exception, and the invalid scheduling rule error. An alternative fix would be to address the two issues separately, i.e. - clone() the hashset, as serge has done in e4, to prevent the first issue - check the current thread's scheduling rule (if it has one) contains the resource in the second instance, if not, schedule a separate refresh. The fix I chose fixes both issues by delegating away to refreshManager...
(In reply to comment #17) > Only so much as the issues are timing related. The resource needs to be out of > sync when you open the project. Do you have refresh automatically on / is the > windows refresh provider enabled? It is green because of the fix for bug 265810.
(In reply to comment #19) > It is green because of the fix for bug 265810. That'll teach me for hypothesizing... Surely this is odd because the two links have different Raw Locations, I would have thought that the link would therefore be recreated during the reconcileLinks()...
I don't think I understand how/why the AliasManager is allowed to refresh, update the sync state on aliased resources when the caller may not hold a scheduling rule which contains all the aliased resources...
(In reply to comment #21) > I don't think I understand how/why the AliasManager is allowed to refresh, > update the sync state on aliased resources when the caller may not hold a > scheduling rule which contains all the aliased resources... John or Szymon, can you comment on this? How is the alias manager allowed update aliased IResources irrespective of currently held scheduling rules? Should the alias manager be allowed to create/remove/update all aliases irrespective of the current scheduling rule (such as when a .project has been modified)?
Moving the target milestone to 3.6.
Moving to 3.7 for further investigation.
(In reply to comment #22) > John or Szymon, can you comment on this? How is the alias manager allowed > update aliased IResources irrespective of currently held scheduling rules? > Should the alias manager be allowed to create/remove/update all aliases > irrespective of the current scheduling rule (such as when a .project has been > modified)? The AliasManager performs a refresh without using any rules. This is technically "illegal", but the rationale is that the resources on disk have already changed and all we are doing is aligning the workspace tree state in memory with reality. See RefreshLocalAliasVisitor for how this is done.
We most likely will not address these bugs during this dev cycle.
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. If the bug is still relevant, please remove the stalebug whiteboard tag.
