Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

possible alternative to static inner classes to prevent memory leaks in android/java?

lately i have been researching about memory leaks in java/android and pretty much everywhere it says that instead of anonymous classes i should use static inner classes with weak references.
so, in my android app i started doing that but very quickly got tired of it because it's a lot of boilerplate code... i think have an alternative solution which i would prefer to use, but i'm juts not sure that it is a valid alternative to static inner classes in terms of preventing memory leaks. as i said before, i haven't seen this solution suggested anywhere else (all say to use static inner classes) so thats why im not sure my alternative will work.

ill use a simple example from my app:
i have a class called WebClient which handles asynchronous web requests and it accepts an interface called iCallback which returns the response from the server to the caller, and in my activity once i get this callback i need to dismiss a dialog, and maybe perform some activity related things (like trigger onBackPressed() and setResult()).
so here is my static inner class i have created:

private static class CallBack implements WebClient.ICallback
{
    private WeakReference<ProgressDialog> mProgDiag;
    private WeakReference<BaseActivity> mActivity;

    public CallBack(BaseActivity activity, ProgressDialog progDiag)
    {
        this.mProgDiag = new WeakReference<>(progDiag);
        this.mActivity = new WeakReference<>(activity);
    }

    @Override
    public void onCallback(String data)
    {
        String responseAsString = Utils.extractStringFromResponse(...);

        final BaseActivity parentActivity = mActivity.get();
        ProgressDialog dialog = mProgDiag.get();

        if(dialog != null)
        {
            dialog.dismiss();
        }

        if (responseAsString == null)
        {
            if(parentActivity != null)
            {
                Utils.makeServerErrorDialog(parentActivity,
                                            new iDialogButtonClickedListener()
                                            {
                                                @Override
                                                public void onDialogButtonClicked()
                                                {
                                                    parentActivity.onBackPressed();
                                                }
                                            });
            }

            return;
        }

        //everything is ok
        if (responseAsString.equals("1"))
        {
            if(parentActivity != null)
            {
                Intent result = new Intent();
                result.putExtra(...);

                parentActivity.setResult(Activity.RESULT_OK, result);
            }
        }

        else
        {
            Utils.reportErrorToServer(...);

            if(parentActivity != null)
            {
                parentActivity.setResult(Activity.RESULT_CANCELED);
            }
        }

        if(parentActivity != null)
        {
            parentActivity.onBackPressed();
        }
    }
}

so for every variable i need in this static inner class i have to create a new weak reference, then retrieve the object itself, and then every time i want to access it i need to check whether it's null... that seems like a lot of code to me.

and here is my suggested alternative:

public abstract class BaseActivity extends AppCompatActivity
        implements WebClient.ICallback
{
    private static final String TAG = "BaseActivity";

    WebClient.ICallback mCallBack;
    ProgressDialog mProgDiag;

    @Override
    protected void onCreate(Bundle savedInstanceState)
    {
        super.onCreate(savedInstanceState);

        setContentView(...);

        mCallBack = this;

        //some code to invoke a server request on button click
        //and passing mCallBack to the request
    }

    @Override
    public void onCallback(String data)
    {
        String responseAsString = Utils.extractStringFromResponse(...);

        mProgDiag.dismiss();

        if (responseAsString == null)
        {
            Utils.makeServerErrorDialog(this,
                                        new iDialogButtonClickedListener()
                                        {
                                            @Override
                                            public void onDialogButtonClicked()
                                            {
                                                onBackPressed();
                                            }
                                        });

            return;
        }

        //everything is ok
        if (responseAsString.equals("1"))
        {
            Intent result = new Intent();
            result.putExtra(...);

            setResult(Activity.RESULT_OK, result);
        }

        else
        {
            Utils.reportErrorToServer(...);

            setResult(Activity.RESULT_CANCELED);
        }

        onBackPressed();
    }

    @Override
    protected void onPause()
    {
        mCallBack = null;

        super.onPause();
    }

    @Override
    protected void onResume()
    {
        super.onResume();

        mCallBack = this;
    }
}

to me this seems much cleaner: no creating and retrieving instances of weak references for every variable i need access to, i can directly invoke activity methods (e.g. onBackPressed()), and no checking for null everywhere.
the only place i would now have to check for null is inside WebClient class before invoking the callBack method.

so my question is, does this approach achieve the same result in terms of preventing memory leaks? is it a "worthy" alternative to static inner classes?

like image 949
or_dvir Avatar asked May 04 '17 09:05

or_dvir


1 Answers

Unfortunately, your approach does not work. By implementing the WebClient.ICallback in your activity, rather than an inner class, you don't get rid of the leak. The leak happens not because the references to activity and dialog are implicit in an anonymous class, or in lambda, or in a non-static inner class instance; the happens when the WebClient keeps this reference while the activity is gone (it is not destroyed, because there is a strong reference to it).

The special mCallBack that you set to null when the activity is paused, gains nothing. Just as well, you can simply pass your activity instance to the WebClient. Now there is a strong reference to your activity, which is managed by someone (async handlers of the WebClient), who is not under your control. If you are unlucky, the async handler will get stuck somewhere and will never release this reference.

Please read this detailed explanation.

Note that WebView itself can cause a memory leak, if special measures are not undertaken!

like image 133
Alex Cohn Avatar answered Oct 11 '22 16:10

Alex Cohn