Bug 410404 - Consumer objects not being released
Summary: Consumer objects not being released
Status: RESOLVED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: Mylyn (show other bugs)
Version: unspecified   Edit
Hardware: Macintosh Mac OS X
: P2 major (vote)
Target Milestone: 2.0.1   Edit
Assignee: Miles Parker CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-06-11 00:16 EDT by Miles Parker CLA
Modified: 2013-06-19 18:08 EDT (History)
1 user (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Miles Parker CLA 2013-06-11 00:16:24 EDT 
When opening a review (particularly large ones?) I sometimes see more than one patch set update job. The consumer design should prevent this from happening for the same model object, so something is amiss here.

(Not a significant enough issue to hold 2.0 release, scheduling for 2.0.1.)
Comment 1 Miles Parker CLA 2013-06-18 15:55:28 EDT 
I've figured out the pattern now. Each time you manually refresh a review, one more patch set is added for a given patch set. This means that we're not disposing of reviews properly. I believe this issue is incidentally fixed in https://git.eclipse.org/r/#/c/13829/5/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/remote/GerritReviewRemoteFactory.java Line 252, but I'm going to test that out to verify.
Comment 2 Miles Parker CLA 2013-06-18 15:56:09 EDT 
(In reply to comment #1)
> I've figured out the pattern now. Each time you manually refresh a review, one
> more patch set is added for a given patch set. This means that we're not
> disposing of [reviews] properly.

Correction: [Review PathSet consumers]
Comment 3 Miles Parker CLA 2013-06-18 16:27:59 EDT 
This is a more important issue than first thought. We really need to release consumers in every case. Otherwise, the remote objects and model objects are never released out of memory, which is obviously not a good thing.

I'm going to run some memory profiling to ensure that we aren't doing anything else that could have memory impacts.
Comment 4 Miles Parker CLA 2013-06-18 16:28:26 EDT 
https://git.eclipse.org/r/#/c/13883/
Comment 5 Miles Parker CLA 2013-06-19 14:39:17 EDT 
Merged https://git.eclipse.org/r/#/c/13883/
Comment 6 Steffen Pingel CLA 2013-06-19 17:25:14 EDT 
Miles, please ensure that the assignee is set properly when closing bugs. Thanks!
Comment 7 Miles Parker CLA 2013-06-19 17:58:16 EDT 
Sorry, I swear I've been checking this before resolving. I don't know how I keep missing it.
Comment 8 Miles Parker CLA 2013-06-19 17:58:34 EDT 
.
Comment 9 Miles Parker CLA 2013-06-19 18:08:10 EDT 
(In reply to comment #7)
> Sorry, I swear I've been checking this before resolving. I don't know how I keep
> missing it.

Ahah! I figured out what has been going wrong with my workflow that kept screwing this up. I typically check "Accept (Change Status to Assigned)" assuming that changes the Assignee to myself. But it doesn't -- it just sets the bug status to Assigned (e.g. Triage). Which is totally confusing because a) you're "accepting" for someone else and b) how can it have a value for Assigned to: if it was never Assigned? Anyway, it shouldn't happen again.