Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 353303 - RWOLockManager.unlock(CONTEXT) gives ConcurrentModEx
Summary: RWOLockManager.unlock(CONTEXT) gives ConcurrentModEx
Status: CLOSED FIXED
Alias: None
Product: EMF
Classification: Modeling
Component: cdo.core (show other bugs)
Version: 4.1   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Caspar D. CLA
QA Contact: Eike Stepper CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-28 07:12 EDT by Caspar D. CLA
Modified: 2012-09-21 07:19 EDT (History)
1 user (show)

See Also:
stepper: review+

Attachments
Patch v1 (1.69 KB, patch)
2011-07-28 07:14 EDT, Caspar D. CLA 		no flags Details | Diff
Patch v2 (7.68 KB, patch)
2011-07-28 07:22 EDT, Caspar D. CLA 		no flags Details | Diff
Patch v3 (left-over tests removed) (6.90 KB, patch)
2011-07-28 07:54 EDT, Eike Stepper CLA 		no flags Details | Diff
Show Obsolete (2) 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 Caspar D. CLA 2011-07-28 07:12:24 EDT 
Unlocking all locks for a given context, i.e. calling 
RWOLockManager.unlock(CONTEXT context), causes ConcurrendModEx,
due to the removal of lockStates while the set of lockStates
for a given context is being iterated over. (The actual
removal happens in #removeLockFromContext.)

No test case was ever written for this scenario. Not sure if
the old RWLockManager did it right. Doesn't matter now, will
provide a fix + testcase for the RWOLockManager momentarily.
Comment 1 Caspar D. CLA 2011-07-28 07:14:08 EDT 
Created attachment 200519 [details]
Patch v1
Comment 2 Caspar D. CLA 2011-07-28 07:22:00 EDT 
Created attachment 200521 [details]
Patch v2

v2 of the patch renames the maps and a method involved with updating
the contextToLockStates map.

I know you (Eike) renamed the method before, but I really believe the
name 'removeLockFromContext' is deceiving. First, what's being removed
is a LockState object, which is a combination of locks, not a single
lock. And second, it's not being removed from a context; rather, it's
being removed from the set of all lockStates that a given context is
involved in. I added JavaDocs to the method, to this effect.

(Functionally patch v2 is the same as v1.)
Comment 3 Caspar D. CLA 2011-07-28 07:26:45 EDT 
PS. I recommend looking at patch v1 first, because it shows how minimal
the fix is.
Comment 4 Eike Stepper CLA 2011-07-28 07:54:24 EDT 
Created attachment 200524 [details]
Patch v3 (left-over tests removed)
Comment 5 Caspar D. CLA 2011-07-28 08:10:48 EDT 
Committed revision 8839.
Comment 6 Eike Stepper CLA 2012-09-21 07:19:00 EDT 
Closing.