Prevent Thread from being garbage collected, and prevent context leak

Question

I want to customize the process of obtaining the authentication token from AccountManager.

AccountManager has getAuthToken() and getAuthTokenByFeatures() methods, but I want to implement a customized flow, which includes switching between activities, etc...

I wanted to implement it the following way:

public AccountManagerFuture<Bundle> getAuthTokenForActiveAccount() {
    GetAuthTokenForActiveAccountFuture future =
            new GetAuthTokenForActiveAccountFuture(MyActivity.this);
    future.start();
    return future;
}

Using the following nested class in my activity:

  private static class GetAuthTokenForActiveAccountFuture extends Thread implements
                AccountManagerFuture<Bundle> {

    private final Activity mActivity;

    public GetAuthTokenForActiveAccountFuture(Activity activity) {
        mActivity = activity;
        // TODO: write this method
    }

    @Override
    public void run() {
         // TODO: write this method
    }

    @Override
    public boolean cancel(boolean b) {
        // TODO: write this method
        return false;
    }

    @Override
    public boolean isCancelled() {
        // TODO: write this method
        return false;
    }

    @Override
    public boolean isDone() {
        // TODO: write this method
        return false;
    }

    @Override
    public Bundle getResult() throws
            OperationCanceledException, IOException, AuthenticatorException {
        return internalGetResult(null, null);
    }

    @Override
    public Bundle getResult(long timeout, TimeUnit timeUnit) throws
            OperationCanceledException, IOException, AuthenticatorException {
        return internalGetResult(timeout, timeUnit);
    }

    private Bundle internalGetResult(Long timeout, TimeUnit timeUnit) throws
            OperationCanceledException, IOException, AuthenticatorException {
        // TODO: write this method
        return null;
    }
}

My idea was that I could create my own AccountManagerFuture object and "unblock" its getResult() method only after all the required steps were done (some of them include activity switching).

I got two issues here:

I need Activity context for switching to other activities when necessary, but the Activity I pass into constructor should be destroyed when I switch to other activity, but it won't because my Thread holds a reference to it... So I create a memory leak here. It seems that making the inner class non-static won't resolve this issue - the reference returned from getAuthTokenForActiveAccount() will still prevent from the outer Activity to be garbage collected. Is there any way I could achieve what I try to do without leaking the context?
Thread is eligible for garbage collection once its run() method returns, right? But in my case I want this thread to stick around because it also functions as AccountManagerFuture - it should be kept in memory until all references to it are gone. My question is this: is it enough to keep a (strong) reference to Thread for preventing it from being garbage collected? If not, how could I force this Thread to stick around until all references are gone?

score 1 · Accepted Answer · edited May 23 '17 at 12:23

1

At first. Making your Future non-static would make it having an implicit reference to its outer class - the Activity.

You should used some form of indirect communication between your future and your Activities..You should probably move it into Service anyway - did you think about any configuration change? Where do you hold the reference for your Future? I would advice you to either move your flow into fragments - then you wouldn't have to switch Activities - and place your future into a retained Fragment (to make it survive orientation change) or move it into a background service and communicate with your activities (or any sort of UI) through broadcastreceivers or event bus.
Thread won't be garbage collected as long as you keep some reference to it. No matter if its finished or not. I think that you are confusing this with the fact that a running Thread won't be garbage collected even without keeping references to it. (I guess tha JVM does so, but I have to admit I'm not sure about this)

edited May 23 '17 at 12:23

Community

1
1

answered May 22 '15 at 11:04

simekadam

7,334
11
56
79

Thanks for clarifying on Threads. As for the first part - I don't really want to re-implement `AccountManager`, therefore I'm still going to use its APIs (`addAccount()`, `getAuthToken()`, `newChooseAccountIntent()`, etc...) - part of these APIs reference `AccountAuthenticator` that I've already implemented, and some of them take care of switching activities, therefore they need an appropriate context... Yes, I could re-invent the wheel, but before I do, I want to make sure it is really my only option. – Vasiliy May 22 '15 at 11:23
All you need is to take your Future or i.e. the running background operation out of your activity. That's what are services made for. – simekadam May 22 '15 at 11:35
@simekadam disagree with 1 issue solution. Root cause of activity memory leak is hard reference to activity. Even lint suggest the solution based on holding activity in weakreference + declare class as static. – x90 May 22 '15 at 12:05
@x90 it depends on the usecase..sure weakreference prevents you from leaking your Activity. The other think is how to resolve a reference to your new Activity (after you switch them). – simekadam May 22 '15 at 12:12
@Vasiliy why do you actually need to have your Activity right in your Future? Isn't it only needed as an argument of the addAccount() method? – simekadam May 22 '15 at 12:12
@simekadam actually, i don't think that his architecture based on Future is reasonable in this case, but his question was about memory leaks. So for me better solution is to create Service. Not retained Fragment, because it is wrong way to use Fragment api. And communicate with service using iBinder and Service public api, not BroadcastReceiver or EventBus. I think that solutions like event-bus are bad practice in case when android provide inbox solution for communication with Service. – x90 May 22 '15 at 12:23
@x90 retained Fragments are OK for keeping worker threads. That's not an antipattern. Service might be better. It really depends..Also it does not matter whether or not are you binding to your service or communicating with it through broadcasts, or eventbus. I personally don't like eventbus either, seems too much like magic. Bound services are ok, but so are broadcasts and eventbus. (Actually bound service would probably be the way to go for me as well, just because this scenario seems to tightly couple to UI and having a direct reference to the service it is nice). – simekadam May 22 '15 at 12:38
Guys, thanks for your comments. I decided to revert to the standard `getAuthTokenByFeatures()` for now, but I'll definitely be looking into this issue again in the future, and , hopefully, remember to post my findings here. – Vasiliy May 31 '15 at 18:11

score 0 · Answer 2 · answered May 22 '15 at 11:01

0

issue 1 solution:
use private WeakReference mContextHolder. when you need context - call mContextHolder.get() and check on null;

issue 2 solution:
Use Service which will host your threads.

answered May 22 '15 at 11:01

x90

2,140
15
25

Prevent Thread from being garbage collected, and prevent context leak

2 Answers2