Bug 214981 - Race condition allowed with SetClasspathOperation
Summary: Race condition allowed with SetClasspathOperation
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.2.2   Edit
Hardware: PC Windows XP
: P3 critical (vote)
Target Milestone: 3.3.2   Edit
Assignee: Jerome Lanneluc CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-01-10 22:01 EST by Chuck Bridgham CLA
Modified: 2008-01-28 05:19 EST (History)
5 users (show)

See Also:
philippe_mulet: pmc_approved+


Attachments
Proposed fix against R3_2_maintenance (6.02 KB, patch)
2008-01-11 05:21 EST, Jerome Lanneluc CLA
no flags Details | Diff
Proposed fix against R3_3_maintenance and HEAD (6.12 KB, patch)
2008-01-11 06:49 EST, Jerome Lanneluc CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chuck Bridgham CLA 2008-01-10 22:01:45 EST
Scenario:

We have project migration jobs running in separate threads modifying the ".project" metadata.  Our jobs use (workspace root) scheduling rules to lock access to the project files.

We noticed that threads were spawned executing the SetClasspathOperation that doesn't have a scheduling rule, and it modifies the .project file - but because no lock is obtained, other threads can modify this file, and cause corruption.

Here is a case where a JREUpdateJob is spawned:

at org.eclipse.core.internal.resources.Project.setDescription(Project.java:949)
        at org.eclipse.core.internal.resources.Project.setDescription(Project.java:1062)
        at org.eclipse.jdt.internal.core.DeltaProcessingState$ProjectUpdateInfo.updateProjectReferencesIfNecessary(DeltaProcessingState.java:160)
        at org.eclipse.jdt.internal.core.DeltaProcessingState.updateProjectReferences(DeltaProcessingState.java:245)
        at org.eclipse.jdt.internal.core.SetClasspathOperation.updateProjectReferencesIfNecessary(SetClasspathOperation.java:804)
        at org.eclipse.jdt.internal.core.SetClasspathOperation.executeOperation(SetClasspathOperation.java:254)
        at org.eclipse.jdt.internal.core.JavaModelOperation.run(JavaModelOperation.java:720)
        at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:1737)
        at org.eclipse.jdt.internal.core.JavaModelOperation.runOperation(JavaModelOperation.java:784)
        at org.eclipse.jdt.internal.core.JavaProject.setRawClasspath(JavaProject.java:3016)
        at org.eclipse.jdt.core.JavaCore$5.run(JavaCore.java:4215)
        at org.eclipse.jdt.internal.core.BatchOperation.executeOperation(BatchOperation.java:39)
        at org.eclipse.jdt.internal.core.JavaModelOperation.run(JavaModelOperation.java:720)
        at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:1737)
        at org.eclipse.jdt.core.JavaCore.run(JavaCore.java:4024)
        at org.eclipse.jdt.core.JavaCore.setClasspathContainer(JavaCore.java:4198)
        at org.eclipse.jdt.internal.launching.JREContainerInitializer.initialize(JREContainerInitializer.java:57)
        at org.eclipse.jdt.internal.launching.LaunchingPlugin$VMChanges.rebind(LaunchingPlugin.java:279)
        at org.eclipse.jdt.internal.launching.LaunchingPlugin$VMChanges.access$0(LaunchingPlugin.java:244)
        at org.eclipse.jdt.internal.launching.LaunchingPlugin$1.run(LaunchingPlugin.java:232)
        at org.eclipse.jdt.internal.core.BatchOperation.executeOperation(BatchOperation.java:39)
        at org.eclipse.jdt.internal.core.JavaModelOperation.run(JavaModelOperation.java:720)
        at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:1737)
        at org.eclipse.jdt.core.JavaCore.run(JavaCore.java:4024)
        at org.eclipse.jdt.internal.launching.LaunchingPlugin$VMChanges.doit(LaunchingPlugin.java:236)
        at org.eclipse.jdt.internal.launching.LaunchingPlugin$JREUpdateJob.run(LaunchingPlugin.java:316)

The same time, our migration job, using a workspace root rule also falls into the same method, but is allowed to finish:

  at org.eclipse.core.internal.resources.Project.setDescription(Project.java:949)
        at org.eclipse.core.internal.resources.Project.setDescription(Project.java:1062)
        at com.ibm.etools.common.internal.migration.TacitMigrationEngine.migrateAllMappings(TacitMigrationEngine.java:228)
        at com.ibm.etools.common.internal.migration.TacitMigrationEngine.migrateAllMappings(TacitMigrationEngine.java:209)
        at com.ibm.etools.common.internal.migration.TacitMigrationOperation.execute(TacitMigrationOperation.java:105)
        at com.ibm.etools.common.frameworks.internal.datamodel.WTPOperation.doRun(WTPOperation.java:338)
        at com.ibm.etools.common.frameworks.internal.datamodel.WTPOperation$1.run(WTPOperation.java:250)
        at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:1737)
        at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:1719)
        at com.ibm.etools.common.frameworks.internal.datamodel.WTPOperation.run(WTPOperation.java:268)
        at com.ibm.etools.common.internal.migration.WorkspaceMigrationListener$1.run(WorkspaceMigrationListener.java:142)
        at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:37)
        at com.ibm.etools.common.internal.migration.WorkspaceMigrationListener$WorkspaceMigrationJob.run(WorkspaceMigrationListener.java:111)

The JDT job then finishes, but with a "stale" IProjectDescription - corrupting the file, and erasing any changes from the migration job.



Can the SetClasspathOperation use a scheduling rule?   at least the IProject?
Comment 1 Jerome Lanneluc CLA 2008-01-11 05:21:07 EST
Created attachment 86655 [details]
Proposed fix against R3_2_maintenance
Comment 2 Jerome Lanneluc CLA 2008-01-11 06:49:55 EST
Created attachment 86662 [details]
Proposed fix against R3_3_maintenance and HEAD
Comment 3 Jerome Lanneluc CLA 2008-01-14 04:52:23 EST
Fix released in R3_2_maintenance stream.
Note that there is no regression test since this is a concurrency problem.
Comment 4 Jerome Lanneluc CLA 2008-01-14 04:59:04 EST
Fix released for 3.4M5 in HEAD.
Comment 5 Jerome Lanneluc CLA 2008-01-14 05:03:55 EST
Requesting PMC approval to port to 3.3.2 since:
- the bug causes a file corruption
- there is no workaround for clients
- the fix is simple 
- the fix is safe - there is no call out to client while we take the lock, so no deadlock can happen
Comment 6 Philipe Mulet CLA 2008-01-15 05:33:21 EST
+1 for 3.3.2
Comment 7 Jerome Lanneluc CLA 2008-01-15 07:43:56 EST
Fix released for 3.3.2 in R3_3_maintenance stream.
Comment 8 Eric Jodet CLA 2008-01-24 04:47:02 EST
Verified in the code for 3.3.2 using M20080123-0800 build