Comment 1 Miles Parker 2011-02-16 19:03:44 EST

I *may* be modifying some eObjects directly, i.e. through object API and not via commands, though all such modifications would occur within some command and (to product this case anyway) Possible that this might muck things up / not be supported? I'm only executing one top-level command at a time. 

I realize that EMF is not thread safe, in which case the answer to that one would probably be yes. I'll comb through the code for such instances. But if I use a single EditingDomain and Command stack and all deltas happen through a command, I shouldn't expect to see a CME in any case, correct?

Comment 2 Ed Merks 2011-02-16 19:10:28 EST

This could only happen if the list being iterated over by
ItemProviderAdapter.java:340 is modified during the iteration.   Certainly the item provider code won't cause such modification.  If you're making changes on another thread, that's very likely to cause such a problem.  But that's something your application needs to handle.

Comment 3 Miles Parker 2011-02-16 19:37:47 EST

(In reply to comment #2)
> This could only happen if the list being iterated over by
> ItemProviderAdapter.java:340 is modified during the iteration.   Certainly the
> item provider code won't cause such modification.  If you're making changes on
> another thread, that's very likely to cause such a problem.  But that's
> something your application needs to handle.

I think this concurrency issue might have come up before, if so forgive me. I do know that there are no changes outside of a command that is being triggered from UI thread, and then executed as a separate job outside of the UI thread. There is a lot of latency in the command, that is it is possible for a modification to be made, and then another modification to the same list to come along at an arbitrary time later, perhaps while an existing notification is being handled. Could I expect Edit to handle this case?

Looking at the code below it appears that there is a modification to an EList from an asynchronous UI thread. If a notification is triggered as a consequence of a list modification within a non-UI thread, what prevents those from colliding?

Comment 4 Ed Merks 2011-02-16 20:17:00 EST

Notifications that happen on a background thread are posted to the UI thread.  If multiple notifications are posted, they are merged.  But at some point, the UI thread will kick in and handle what's there.  If the background thread is continuing to make changes that will definitely cause problems in the UI thread.  The transaction framework is designed to handle situations like this.  The basic edit framework is not.  You might specialize AdapterFactoryContentProvider.notifyChanged to accumulate a viewerRefresh until the whole job is finished.

Comment 5 Miles Parker 2011-02-16 21:41:43 EST

(In reply to comment #4)
> Notifications that happen on a background thread are posted to the UI thread. 
> If multiple notifications are posted, they are merged.  But at some point, the
> UI thread will kick in and handle what's there.  If the background thread is
> continuing to make changes that will definitely cause problems in the UI
> thread.  The transaction framework is designed to handle situations like this. 
> The basic edit framework is not.  You might specialize
> AdapterFactoryContentProvider.notifyChanged to accumulate a viewerRefresh until
> the whole job is finished.

OK, now I am remembering our prior discussion -- I think there is a lengthy thread in forums about it. Basically, there is no way OOTB to manage notifications such that they don't collide with changes from other threads. I did look into the methods you mentioned 

This still seems like a bug to me, in that there is no reason as far as the overall API contract (whatever that means :0)) is concerned why you should not be able to safely make changes to the model *at any point*, and that the view mechanism should be able to handle that *assuming that there is no attempt to modify the model itself concurrently*. As it is, the behavior of the system under stress isn't well defined. Why can't the view use an unmodifiable copy of the EList for iteration?

Comment 6 Ed Merks 2011-02-16 22:12:12 EST

Even making a copy of a list can produce exactly the problem you see.  You need to copy it while it's not changing...

At the end of the day, there is no way to ensure application level thread safety "simply" by sprinkling "just the right number of synchronizes" throughout the low-level primitives.  Those tend to cause dead-lock and end up interfering with whatever application level threading policy one is trying to implement. I've had endless discussions with people about this type of thing while I was at IBM and in the forums. There just appears to be this baseless yet ingrained human faith that thread safety will come from thread safe primitives.  It won't and never will.  For example, even if list itself is thread safe (e.g., implemented by Vector), the overall behavior of the following code is not thread safe.

  int index = list.indexOf(o);
  list.remove(index);

A view trying to display the state of an object that's changing as the view is being computed is never going to produce a well defined result.  That will only come about by ensuring that the view sees an unchanging object

Comment 7 Miles Parker 2011-02-16 23:25:45 EST

(In reply to comment #6)
> Even making a copy of a list can produce exactly the problem you see.  You need
> to copy it while it's not changing...

I think all you need to do is copy it such that its own *internal state* is consistent.

> At the end of the day, there is no way to ensure application level thread
> safety "simply" by sprinkling "just the right number of synchronizes"
..
> primitives.  It won't and never will.  For example, even if list itself is
> thread safe (e.g., implemented by Vector), the overall behavior of the
> following code is not thread safe.
> 
>   int index = list.indexOf(o);
>   list.remove(index);

No, of course not. Philosophically, I'm with you. I'm sure that we've all had the experience of chasing synchronize blocks around a framework in the vain hope that we'll make it work. Having implemented a thread safe MVC framework supporting millions of agents on multiple processors, I can say that the only really workable robust  way to have views of a  model changing dynamically it is to punt on the whole issue: a) run a complete iteration or step of the model, b) stop making any changes to the model, c) run the entire set of visualization updates, and d) stop making any changes to visualization. Repeat. So that would be one approach to what I want to do, where you simply provide locking behavior at the whole model level. You can either update the view or model but not both. If you try, you get an exception. I'm not suggesting doing that for EMF but it is something that I might take a look at customizing internals to support something like that. Or maybe transactions will work. But that's not what I'm suggesting..

> A view trying to display the state of an object that's changing as the view is
> being computed is never going to produce a well defined result.  That will only
> come about by ensuring that the view sees an unchanging object

There are two ways to handle this. 1) Give up on being able to enforce safety, or 2) Give up on the idea of having a view that matches the model perfectly. I'm arguing for 2. Get a copy of the list that may or may not contain members that no longer exist that have subsequently changed and so on but that you can safely iterate in order to update a view. That view will be definition always be a moving target in any case, and you'll have another update queued to "fix" any mistakes in the last update. I realize that this doesn't solve the deep issue but it would arguably make it more manageable.

Comment 8 Miles Parker 2011-02-18 13:41:13 EST

Sorry to be a PITA, but I'd like clarification on this, so I'm changing summary, marking as enhancement and reopening.

I can accept that it is something that you don't think should be fixed, but it can't be WORKSFORME, because clearly the issue is occurring and reproducible. When I attempt to change a model from anywhere but UI thread, I can get a concurrent mod exception. If this issue is impossible to address then please feel free to close as INVALID. If you don't think it should ever be addressed within EMF.Edit then WONTFIX works. But if there is a way to fix it, even if very difficult or not worth the effort then perhaps you could un-assign yourself and we could leave it open on the chance that someone comes up with a way to support it (I'll might work on one but I don't know when).

The general issue is true of any SWT element actually but it is possible if difficult to avoid that case. But SWT changes are simple and EMF Edit aren't -- changes can be made that are lengthy enough and made in the UI thread lock up the entire workbench. There are similar issues for GEF, GEF3D and Zest. One approach would be to require that changes be made either from UI thread *or* from a *single* model editing thread in the manner of Display#asyncExec and syncExec where the two threads blocked against each other, still allowing model change notifications to be accumulated and later merged. I think these issues are worth thinking about as they will become more and more relevant as UIs become more sophisticated.

Comment 9 Ed Merks 2011-02-21 22:06:27 EST

The transaction framework helps ensure cooperative shared read and exclusive write access. We won't try to duplicate that in the core.  Even if we accumulate all notifications, the fact that the user can be interacting with the view while the underlying model is being modified and while the view might not be up-to-date with those changes can also be a problem.  It's a bit of rat hole...

Comment 10 Miles Parker 2011-02-21 22:11:47 EST

(In reply to comment #9)
> The transaction framework helps ensure cooperative shared read and exclusive
> write access. We won't try to duplicate that in the core.  Even if we
> accumulate all notifications, the fact that the user can be interacting with
> the view while the underlying model is being modified and while the view might
> not be up-to-date with those changes can also be a problem.  It's a bit of rat
> hole...

OK, I can accept that. :) I think I may have a use case for AMP (hopefully in shape to demo at EclipseCon) that will exercise the kind of automated model update coupled with user interaction I'm hoping to accomplish here.

Comment 11 Eike Stepper 2011-02-22 01:31:49 EST

Hi Miles, I haven't read all comments here, but I got the impression that CDO might be helpful for you. CDO does allow multi thread access to a single logical model by providing you with "cheap copies" for each thread and fine grained locking (optimistic or pessimistic). Let me know if you're interested in details...