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

Bug 329143

Summary: IProject#getDescription is not thread safe with #setDescription
Product: [Eclipse Project] Platform Reporter: James Blackburn <jamesblackburn+eclipse>
Component: ResourcesAssignee: James Blackburn <jamesblackburn+eclipse>
Status: CLOSED WONTFIX QA Contact:
Severity: major    
Priority: P3 CC: arthorne.eclipse, Szymon.Brandys
Version: 3.7   
Target Milestone: ---   
Hardware: PC   
OS: All   
Whiteboard: stalebug
Bug Depends on: 329142    
Bug Blocks:    
Attachments:
Description Flags
test 1 none

Description James Blackburn CLA 2010-10-30 13:14:34 EDT
I noticed that Project getDescription & setDescription are not thread-safe.  While the element tree is synchronized, the project description for subsequent tree generations is modified (and read) outside of the element tree lock. Moreover the same instance that's modified during #set is visible to the #get path. 

There is no memory barrier, and no atomicity to Project Description updates.

Thread access looks like this:

Bug_XXXXX [JUnit Plug-in Test]	
	org.eclipse.equinox.launcher.Main at localhost:59099	
		Daemon Thread [ReaderThread] (Running)	
		Daemon Thread [Thread-5] (Suspended (breakpoint at line 89 in Project))	
			Project.basicSetDescription(ProjectDescription, int) line: 89	
			Project.setDescription(IProjectDescription, int, IProgressMonitor) line: 1217	
			Project.setDescription(IProjectDescription, IProgressMonitor) line: 1245	
			Bug_XXXXX$1.run() line: 71	
		Daemon Thread [Thread-6] (Suspended (breakpoint at line 384 in Project))	
			Project.getDescription() line: 384	
			Bug_XXXXX$2.run() line: 87	
		Daemon Thread [[ThreadPool Manager] - Idle Thread] (Running)	
	/System/Library/Frameworks/JavaVM.framework/Versions/1.5/Home/bin/java (Oct 30, 2010 5:06:07 PM)	

In this case the project description being modified by basicSetDescription is exactly the same as the project description being read by the getProjectDescription.

Test to be attached (once I know the bug number) demonstrating that the a thread calling #getProjectDescription can see an inconsistent project description.

Ideally a copy of the project description should be modified, and atomically set into the delta tree.


The same may apply to the ResourceInfo, I haven't checked.
Comment 1 James Blackburn CLA 2010-10-30 13:17:07 EDT
Created attachment 182104 [details]
test 1

Test for the issue.
Comment 2 James Blackburn CLA 2010-10-30 13:20:18 EDT
You'll need to apply the patch on bug 329142 before this test starts failing for the right reasons...
Comment 3 John Arthorne CLA 2010-11-01 12:00:32 EDT
(In reply to comment #0)
> I noticed that Project getDescription & setDescription are not thread-safe. 
> While the element tree is synchronized, the project description for subsequent
> tree generations is modified (and read) outside of the element tree lock.
> Moreover the same instance that's modified during #set is visible to the #get
> path. 

I see the bug - ProjectInfo#getDescription should return a clone, and have a separate method for returning the actual object for read-only purposes (like ResourceInfo#getMarkers).

I don't see where the description is modified outside of the tree lock though... Project#setDescription does hold the workspace lock while making its changes.
Comment 4 James Blackburn CLA 2010-11-01 12:09:52 EDT
(In reply to comment #3)
> I see the bug - ProjectInfo#getDescription should return a clone, and have a
> separate method for returning the actual object for read-only purposes (like
> ResourceInfo#getMarkers).
>
> I don't see where the description is modified outside of the tree lock
> though... Project#setDescription does hold the workspace lock while making its
> changes.

There is a clone() happening (Project#getDescription()). I think that's OK. The problem is that the thing being clone()d is being modified concurrently...

I think the problem is that the resource info returned by #getResourceInfo(false, true /*mutable*/) creates the mutable element data which is immediately accessible to the read path. AFAICS there isn't any barrier.
Comment 5 Eclipse Webmaster CLA 2019-09-06 15:29:57 EDT
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.
Comment 6 Eclipse Genie CLA 2022-02-11 16:57:20 EST
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. As such, we're closing this bug.

If you have further information on the current state of the bug, please add it and reopen this bug. 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.

--
The automated Eclipse Genie.