Bugzilla – Full Text Bug Listing
| Summary: | fix NPEs resulting out of disposal of RemoteEmfConsumer | ||
|---|---|---|---|
| Product: | z_Archived | Reporter: | Steffen Pingel <steffen.pingel> |
| Component: | Mylyn | Assignee: | Project Inbox <mylyn-triaged> |
| Status: | RESOLVED WORKSFORME | QA Contact: | |
| Severity: | normal | ||
| Priority: | P2 | CC: | tomasz.zarna |
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Hardware: | Macintosh | ||
| OS: | Mac OS X | ||
| Whiteboard: | |||
| Bug Depends on: | 410580, 413561, 414799 | ||
| Bug Blocks: | |||
|
Description
Steffen Pingel
So the immediate cause is not that you want to not have the Review refer to the Repository, but that you don't want the repository to hang on to the review. But since these are defined as opposite containment features, removing the review from a repository has the effect of setting repos to null.
But the real issue isn't that the repos is getting set to null, but that we're attempting to use a model which has effecitvly been disposed ("deactivated", really) already, without having re-opened it. We need to prevent the model from being disposed when other models are still using it and in fact, there is already a mechanism for this: as long as an observer exists for a given model object/consumer it should not be possible to dispose of it.
So I think there are two design principles here (which I should document in API once we have this all clarified.):
1. We should never be hanging on to a reference to a Review object directly. We should always get at a Review directly through its consumer.
2. We should register as an observer for the model as soon as we're interested in that model object, and not remove that observer until after we're no longer interested.
Given those constraints (and the upcoming fixes to the notificaiton mechansim (bug 413480)), it "should" be impossible to have a model object get disposed while it's still being used.
Is there any reason for the containment relationship? Just wondering if we could simplify things if the review just had a reference to the repository but wasn't contained in it (or the other way around). (In reply to comment #2) > Is there any reason for the containment relationship? Just wondering if we could > simplify things if the review just had a reference to the repository but wasn't > contained in it (or the other way around). That's a *really* good question. TO the first part of your question, there is actually no strict reason for it to be a containment relationship. That's just an obvious but perhaps not helpful assumption we're making. However, I'm not sure that would really solve the issue, since even if it wasn't a containment reference, EMF would still want to store the review references in the repository. We could mark the reference as volatile/transient, which is the solution that R4E used, but I'm leery of that approach. (In reply to comment #1) > Given those constraints (and the upcoming fixes to the notificaiton mechansim > (bug 413480)), it "should" be impossible to have a model object get disposed > while it's still being used. Tomek is still seeing the issue w/ notificaitons under the latest bug 413480 so this isn't the case. The issue there I think is with disposal across different levels of containment. That is, somehow we still have a reference to a patch set but we've disposed of the observers for the review. So I think the Review is getting unloaded (Repos set to null) but the patch set is still available. I think we perhaps jus tneed a solution that demand loads the repository as a special case. Is this issue still happening? Tomek, have you seen it? Nope, I haven't seen it for a while. Closing. Tomek, Sam, please reopen if you observe this behaviour again. |