Bugzilla – Full Text Bug Listing
| Summary: | TransactionChangeRecorder doesn't remove itself | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Modeling] EMF Services | Reporter: | Rick Warren <rick.warren> | ||||
| Component: | Transaction | Assignee: | Christian Damus <give.a.damus> | ||||
| Status: | VERIFIED FIXED | QA Contact: | |||||
| Severity: | enhancement | ||||||
| Priority: | P3 | CC: | rose | ||||
| Version: | unspecified | Keywords: | plan | ||||
| Target Milestone: | --- | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
Rick Warren
The TransactionChangeRecorder inherits the ChangeRecorder's property of being "sticky". Change recorders, in general, do not remove themselves in response to the reverse of the change that added them, because they need to continue recording changes to objects while they are detached from the model, in case they should be re-attached later (e.g., by undoing via ChangeDescription.applyAndReverse). What a ChangeRecorder is able to do, however, is to remove itself from all of its targets when it is dispose()d. Unfortunately, this is difficult for the TransactionChangeRecorder, because it does not track its targets (an optimization). The TransactionalEditingDomain works on the premise that it controls the complete lifecycle of the model that its resource set contains. i.e., it assumes that you would never want to remove a resource from its resource set unless you're unloading it, in which case all adapters are removed from all of its contents when their proxy URIs are set. Perhaps what is needed is to provide a utility that will do whatever is needed to remove an object or resource from an editing domain's control (requiring that it is already detached). Might that meet your requirement? Re: comment #1: Yes, such a utility would be very helpful. Presumably, such a utility would also solve bug #161168, which I also filed recently. As an aside, it would be very helpful if there were more documentation regarding the assumptions that editing domains make about their resource sets and the contents of those resource sets. (Such as, that an editing domain seems to assume that its resource set is not shared with anyone else, and as you say, that it owns and manages the complete lifecycle of everything inside.) These assumptions have a very deep and fundamental impact on the way that an application handles files and the references between them, and without this documentation, developers are left to deduce the intended behavior through trial and error. I'm curious what you mean when you say that it is difficult for a TransactionChangeRecorder to remove itself when dispose() is called, because TransactionalEditingDomainImpl does just that. Yes, this is exactly the kind of information that is planned for the next EMFT Transaction release (in line with Eclipse 3.3/Europa), in which we are planning to provide a more complete Developer Guide documentation. TransactionalEditingDomainImpl.dispose() disposes its change recorder because ChangeRecorder.dispose() is a public API, which can be extended by clients that extend the transaction API. In the M5 milestone of GMF 1.0, for example, there was a specialization of the editing domain that provided a custom change recorder, which had additional stuff to clean up. A new utility for removing the change recorder from a resource set would be used in a TransactionChangeRecorder.dispose() implementation to do what it currently doesn't. The problem today is that the implementation inherited from ChangeRecorder doesn't work because the TransactionChangeRecorder doesn't keep track of its targets. This can be improved by finding all of the objects currently in the editing domain's resource set, to remove the recorder from them, but this doesn't account for the objects that are no longer attached. For objects that are no longer attached, these can be handled lazily. When the TransactionChangeRecorder of an editing domain that has been disposed receives an event from one of its targets, it can simply ignore the event and remove itself from its target at that time. This will ensure that the change recorder is at worst innocuous and cleans up objects that are still in use. Est.: .5w Created attachment 54674 [details]
Proposed fix
Attached is a patch with changes to the TransactionChangeRecorder and a new TransactionUtil.disconnectFromEditingDomain() API that makes it possible to move Resources and model elements from a transactional editing domain to another editing domain (transactional or otherwise).
For examples of how to use it, please see the new JUnit tests (included in the patch).
Rick, would you be able to have a look at this patch to let me know whether it meets your requirements?
Committed the proposed fix to HEAD, with a few tweaks to ensure that:
- the change recorder forgets its editing domain, to allow it to be
reclaimed by GC even if the recorder lingers on free-floating objects
- the disconnectFromEditingDomain() API accounts for the possibility
of an object being in the control of multiple domains simultaneously
(although this is much discouraged)
Thanks for the fix. I haven't had a chance to try it out, but the diff looks like it does what I'm looking for. When will the updated API be generally available -- 2.0? No problem. If it doesn't work for you, just re-open the bug ;-) This will be available in the 1.1 release, and should be in next week's integration build. Move to verified as per bug 206558. |