ActionBarSherlock icon indicating copy to clipboard operation
ActionBarSherlock copied to clipboard

memory leak: shareTargetSelectedListener reference stays forever

Open veonua opened this issue 11 years ago • 8 comments

actionProvider.setOnShareTargetSelectedListener(shareTargetSelectedListener);
actionProvider.setOnShareTargetSelectedListener(null);

the shareTargetSelectedListener stays forever, and could not be nulled, because of

/**
 * Set the activity chooser policy of the model backed by the current
 * share history file if needed which is if there is a registered callback.
 */
private void setActivityChooserPolicyIfNeeded() {
    if (mOnShareTargetSelectedListener == null) { // << this line
        return;
    }
    if (mOnChooseActivityListener == null) {
        mOnChooseActivityListener = new ShareAcitivityChooserModelPolicy();
    }
    ActivityChooserModel dataModel = ActivityChooserModel.get(mContext, mShareHistoryFileName);
    dataModel.setOnChooseActivityListener(mOnChooseActivityListener);
}

also ActivityChooserModels are static, so shareTargetSelectedListener stays while the application working, this is a huge memory leak (in my case shareTargetSelectedListener needs a context and locks the whole activity in memory)

veonua avatar Apr 24 '13 14:04 veonua

I don't see how that line means shareTargetSelectedListener can't be nulled. It's nulled in setOnShareTargetSelectedListener.

SimonVT avatar Apr 24 '13 14:04 SimonVT

as far as I can see -

ActivityChooserModel keeps all model policies, when we create a ShareAcitivityChooserModelPolicy it keeps whole ShareActionProvider as $.this, and keeps mOnShareTargetSelectedListener as the reference to the fragment (my case).


I discovered this bug in situation when I have couple of fragments (attached to different activities), both have a ShareActionProvider (with default history filename) in ActionBar, but in FragmentA I have setOnShareTargetSelectedListener(this)

I noticed click on the Share button on ActivityB fires FragmentA.onShareTargetSelected() (which should be destroyed when ActivityA stops), - reason ActivityChooserModel.get returns the reference to the Policy that keeps FragmentA.

On Wed, Apr 24, 2013 at 5:58 PM, Simon Vig Therkildsen [email protected] wrote:

I don't see how that line means shareTargetSelectedListener can't be nulled. It's nulled in setOnShareTargetSelectedListener.

— Reply to this email directly or view it on GitHub.

veonua avatar Apr 24 '13 15:04 veonua

mOnShareTargetSelectedListener is nulled when you call setOnShareTargetSelectedListener(null)

SimonVT avatar Apr 24 '13 15:04 SimonVT

ok, when should I null it? Should I null it manually?

On Wed, Apr 24, 2013 at 6:23 PM, Simon Vig Therkildsen < [email protected]> wrote:

mOnShareTargetSelectedListener is nulled when you call setOnShareTargetSelectedListener(null)

— Reply to this email directly or view it on GitHubhttps://github.com/JakeWharton/ActionBarSherlock/issues/913#issuecomment-16938398 .

veonua avatar Apr 24 '13 15:04 veonua

I don't know, I haven't used ShareActionProvider. It works just like the one provided by the framework, so you should ask on stackoverflow or the android developers mailing list.

SimonVT avatar Apr 24 '13 15:04 SimonVT

unfortunately, could reproduce using framework ShareActionProvider. it's suck =(((

veonua avatar Apr 24 '13 18:04 veonua

We've gotten in the habit of just writing our own with a more sane behavior and code using the same concept.

JakeWharton avatar Apr 24 '13 18:04 JakeWharton

if you interested https://gist.github.com/veonua/5454621 sample code that reproduces the bug

On Wed, Apr 24, 2013 at 9:54 PM, Jake Wharton [email protected]:

We've gotten in the habit of just writing our own with a more sane behavior and code using the same concept.

— Reply to this email directly or view it on GitHubhttps://github.com/JakeWharton/ActionBarSherlock/issues/913#issuecomment-16956888 .

veonua avatar Apr 24 '13 19:04 veonua