Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 247521 - Framework.installBundle updates BundleRepository in unsafe manner
Summary: Framework.installBundle updates BundleRepository in unsafe manner
Status: RESOLVED FIXED
Alias: None
Product: Equinox
Classification: Eclipse Project
Component: Framework (show other bugs)
Version: 3.5   Edit
Hardware: PC All
: P3 normal (vote)
Target Milestone: 3.5 M4   Edit
Assignee: Thomas Watson CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 247636
  Show dependency tree
 
Reported: 2008-09-16 12:26 EDT by Rob Harrop CLA
Modified: 2008-11-04 12:37 EST (History)
1 user (show)

See Also:

Attachments
patch to address issue (2.87 KB, patch)
2008-09-16 12:26 EDT, Rob Harrop CLA 		no flags Details | Diff
Updated cut of the bundle repository patch. (4.68 KB, patch)
2008-10-28 04:40 EDT, Rob Harrop CLA 		tjwatson: iplog+
Details | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Rob Harrop CLA 2008-09-16 12:26:23 EDT 
Created attachment 112687 [details]
patch to address issue

Build ID: HEAD

Steps To Reproduce:
1. See Framework.installBundle


More information:
Comment 1 Thomas Watson CLA 2008-09-23 13:40:19 EDT 
Rob, correct me if I am wrong.  I think the Framework portion of the patch is not needed if attachment 112883 [details] in bug 247522 is released.
Comment 2 Rob Harrop CLA 2008-09-24 06:08:02 EDT 
Tom you are completely correct. I've clearly screwed up attaching patches to these issues :)

I have newer versions of the patches for this bug and for bug 247522 running in our test suite locally. When I have hammered on them I'll reattach them to these issues, until then I think it is worth waiting.

The getNextBundleId patch in bug 247520 is well tested here now and seems to have fixed all our problems in that area.
Comment 3 Thomas Watson CLA 2008-10-22 17:07:53 EDT 
Hi Rob, any more progress here?  We are getting late for 3.5 M3.  This will likely need to be pushed to 3.5 M4.
Comment 4 Rob Harrop CLA 2008-10-23 04:55:03 EDT 
Tom,

I just got back from a few weeks away. While I've been out we've had a completely successful set of integration test runs against our build so I'm happy that the fixes I have locally work correctly. I'm only in the office for a few hours today, but when I get back on Monday I'll post the latest patches to all the thread safety issues.

Rob
Comment 5 Rob Harrop CLA 2008-10-28 04:40:48 EDT 
Created attachment 116259 [details]
Updated cut of the bundle repository patch.
Comment 6 Thomas Watson CLA 2008-11-04 11:39:46 EST 
Rob, I released the latest patch with an updated copyright to include your name, SpringSource and the bug number.

I did not release the changes you made to StartLevelManager.  You had added a redundant synchronized block to sortByDependencies(AbstractBundle[], int, int)
Comment 7 Rob Harrop CLA 2008-11-04 11:44:36 EST 
Tom,

Thanks for applying this. I'm not sure that the sync block in SLM is redundant - it essentially guarantees atomicity for the sort operation with respect to the bundles state. If this is not required then it can happily be dropped.

Rob
Comment 8 Thomas Watson CLA 2008-11-04 12:37:20 EST 
(In reply to comment #7)
> Tom,
> 
> Thanks for applying this. I'm not sure that the sync block in SLM is redundant
> - it essentially guarantees atomicity for the sort operation with respect to
> the bundles state. If this is not required then it can happily be dropped.
> 

The reason I think it is redundant is because the private method "sortByDependencies(AbstractBundle[], int, int)" is called by "sortByDependency(AbstractBundle[])" while holding the BundleRespository lock.