Bug 528134 - Concurrent changes can lead to data loss in the text widget
Summary: Concurrent changes can lead to data loss in the text widget
Status: RESOLVED FIXED
Alias: None
Product: EEF
Classification: Modeling
Component: User Interface (show other bugs)
Version: 1.9.0   Edit
Hardware: All All
: P3 major
Target Milestone: 1.9.2   Edit
Assignee: Pierre-Charles David CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-12-05 03:36 EST by Pierre-Charles David CLA
Modified: 2017-12-20 04:05 EST (History)
1 user (show)

See Also:


Attachments
A patch for Sirius's debugging view which adds buttons to simulate background model changes (4.04 KB, patch)
2017-12-05 03:51 EST, Pierre-Charles David CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pierre-Charles David CLA 2017-12-05 03:36:14 EST
Steps to reproduce (in a Sirius context as it is easier, but the issue is not Sirius-specific):
1. Open a Sirius diagram which provides EEF-based properties view, and select an element which provides an editable text field (most obvious case: select an EClass in EcoreTools and use it's Name field). Say the class is named "A" initially, and thus the text field displays "A".
2. Launch a background job/thread which waits a few seconds and then renames "A" into "A2". I'll attach a patch to the Sirius Debugging View which provides buttons to launch such actions for testing.
3. While the job is waiting, start to edit the text field, for example into "A3". DO NOT VALIDATE YOUR EDITION.
4. When the background job finally performs the model change, the value being edited ("A3") is lost and unconditionally replaced with the new value computed from the new model state, "A2" => KO.

The example is a little contrived and simplified, but there are concrete scenarios where the resulting behavior is very bad: in particular when the user was editing a multi-line text field, which can take some time, and some background process (for example synchronization with remote changes) overwrites everything he had written with no chance at all to get it back.
Comment 1 Pierre-Charles David CLA 2017-12-05 03:50:49 EST
Note that when a model change occurs, because the rules used in Sirius to produce the properties view are so open, we have no choice but to make a full refresh of the view. Only once we have evaluated all the expression can we know if the text field still exists and would still show the same initial text, as the expression used in the Sirius VSM to compute the text field value is a black box that can do arbitrary computation on any part of the model.

There are actually two different resulting cases from the user point of view:
* the background job makes a completely unrelated change in the semantic model, which would produce the exact same initial value ("A") for the text field being edited. Losing the text being edited in this scenario is particularly bad.
* the background job makes a model change which would produce a different text for the widget that the one originally seen by the user. In this case, assuming the user can understand the relationship between the change and the widget's value, it may be a little less critical (though still bad) to overwrite his current text, but we may still want to offer him some choice.
Comment 2 Pierre-Charles David CLA 2017-12-05 03:51:39 EST
Created attachment 271773 [details]
A patch for Sirius's debugging view which adds buttons to simulate background model changes
Comment 3 Pierre-Charles David CLA 2017-12-05 04:02:53 EST
Note that https://git.eclipse.org/r/#/c/106598/, which is already merged, is a first attempt at fixing this. It was made as a temporary fix for testing before this ticket was created, and thus does not reference this bug in its commit message.
Comment 4 Eclipse Genie CLA 2017-12-07 08:12:45 EST
New Gerrit change created: https://git.eclipse.org/r/113003
Comment 6 Pierre-Charles David CLA 2017-12-20 04:05:38 EST
Fixed by 51a0df0bd2c22027f9005691b33fcac6b369d0e6.