Bug 335080 - Make CDOView thread-safe
Summary: Make CDOView thread-safe
Status: CLOSED FIXED
Alias: None
Product: EMF
Classification: Modeling
Component: cdo.core (show other bugs)
Version: 4.0   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: Eike Stepper CLA
QA Contact: Eike Stepper CLA
URL:
Whiteboard: Appealing to a Broader Community
Keywords: noteworthy
: 326628 (view as bug list)
Depends on: 340709
Blocks:
  Show dependency tree
 
Reported: 2011-01-22 04:42 EST by Eike Stepper CLA
Modified: 2011-06-23 03:42 EDT (History)
4 users (show)

See Also:


Attachments
Patch v1 (188.34 KB, patch)
2011-01-22 07:32 EST, Eike Stepper CLA
no flags Details | Diff
Patch v2 - attempt to execute XA transactin in parallel (8.20 KB, patch)
2011-01-23 05:03 EST, Eike Stepper CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eike Stepper CLA 2011-01-22 04:42:10 EST
Currently the thread-safety of CDOView (and CDOTransaction!) is focused on proper synchronization between the InvalidationRunner thread in the CDOSession and *one* client thread per view. The current problems are:

- A number of different locks are used, e.g. viewLock, stateLock, objectsLock --> deadlock potential
- Not all places that need protection are properly guarded, e.g. tx.rollback() is not --> local data corruption potential

Goal of this bugzilla:

1) Use only the view monitor
2) Use it in every non-private method of AbstractCDOView, CDOViewImpl, CDOTransactionImpl, CDOSavepointImpl, CDOStoreImpl, CDOStatemachine
3) Clarify whether it should be used in CDOObjectImpl and CDOObjectWrapper (would imply CDOLegacyAdapter, etc.)
Comment 1 Eike Stepper CLA 2011-01-22 05:03:58 EST
This effort will also move the InvalidationRunner from CDOSessionImpl to CDOViewImpl (1 thread -> n threads).

The session will still receive remote commitInfos and bring them in the right order (if needed) before passing them to all views. No additional thread will be involved anymore.

A view will place the commitInfos from the session in a queue and let the InvalidationRunner handle it eventually.

The overall timing will be slightly different:

1. Out-of-order commitInfos are queued, see CDOSessionImpl.invalidate()
2. When the right commitInfo is in the queue, see CDOSessionImpl.invalidateOrdered() :
2.1. the revisionManager is updated with the changeset,
2.2. the sessions's lastUpdateTime is set
2.3. CDOSessionInvalidationEvent is sent
2.4. commitInfo is passed to all views (new: views with unrelated branches are no longer excluded here!)
3. The view places the commitInfo in the invalidation queue, see CDOViewImpl.invalidate()
4. the views's InvalidationRunner processes the commitInfo, see CDOViewImpl.doInvalidate()
4.1. the view invalidates itself with the commitInfo's changeset *if the view's branch requires that*
4.1.1. CDOStateMachine is triggered for the changeset, conflicts detected
4.1.2. sendInvalidationNotifications()
4.1.3. handleConflicts()
4.1.4. sendDeltaNotifications()
4.1.5. fireAdaptersNotifiedEvent()
4.2. the view's lastUpdateTime is set *unconditionally*

Three important consequences:

a) The views of a session are no longer invalidated "atomically" altogether
b) A view's lastUpdateTimeStamps is always updated and waitForUpdate() works more predictably
Comment 2 Eike Stepper CLA 2011-01-22 07:32:12 EST
Created attachment 187359 [details]
Patch v1

A patch until I'm sure I can fork branches in SVN :P
Comment 3 Eike Stepper CLA 2011-01-23 03:32:55 EST
Committed revision 6894 to trunk
Comment 4 Eike Stepper CLA 2011-01-23 05:03:03 EST
Created attachment 187379 [details]
Patch v2 - attempt to execute XA transactin in parallel

Idea: compute all the needed tx state *before* xa threads are forked.

Does not fully work, yet. Keeping sequential execution for now...
Comment 5 Eike Stepper CLA 2011-01-23 05:17:26 EST
Resolving for now.
Comment 6 Eike Stepper CLA 2011-01-25 06:56:41 EST
*** Bug 326628 has been marked as a duplicate of this bug. ***
Comment 7 Eike Stepper CLA 2011-01-30 03:24:56 EST
Committed revision 6966:
- trunk/plugins/org.eclipse.emf.cdo
- trunk/plugins/org.eclipse.emf.cdo.tests
Comment 8 Eike Stepper CLA 2011-01-30 03:26:35 EST
Revision 6966 brought back the AbstractObjectConflictResolver.MergeLocalChangesPerFeature but deprecated it.
Comment 9 Eike Stepper CLA 2011-06-23 03:42:26 EDT
Available in R20110608-1407