7

I am a total beginner in Java and have created a simple Java Android snippet where in a Runnable after 1,5 seconds I change the TextView from Hello World to Hola Mundo. It works flawlessly, basically a WeakReference should prevent this memory leak from happening right? I have a doubt if there's absolutely no memory leak whenever device orientation occurs. I would love to check this but can't manage to change orientation in my emulated Android.

This is the code:

package com.example.helloworld;

import android.app.Activity;
import android.os.Bundle;
import android.os.Handler;
import android.widget.TextView;
import android.util.Log;
import java.lang.ref.WeakReference;

public class HelloWorldActivity extends Activity
{
    private Handler h = new Handler();
    private static TextView txtview;
    /** Called when the activity is first created. */
    @Override
    public void onCreate(Bundle savedInstanceState)
    {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.main);
        txtview = (TextView) findViewById(R.id.mainview);

        h.postDelayed(new WeakRunnable(txtview),1500);
    }

    private static final class WeakRunnable implements Runnable {
        private final WeakReference<TextView> mtextview;

        protected WeakRunnable(TextView textview){
            mtextview = new WeakReference<TextView>(textview);
        }

            @Override
            public void run() {
                TextView textview = mtextview.get();
                if (textview != null) {
                    txtview.setText("Hola Mundo");
                    textview = null; // No idea if setting to null afterwards is a good idea
                }
                Log.d("com.example.helloworld", "" + textview);
            }
    }           

}

EDIT

It's safe from memory leaks but a few answers were also concerned with UI thread blocking. In fact this code runs the Handler in the main (UI) thread. To spawn a new thread I'm spawning a thread manually as follows:

package com.example.helloworld;

import android.app.Activity;
import android.os.Bundle;
import android.os.Handler;
import android.widget.TextView;
import android.util.Log;
import java.lang.ref.WeakReference;

public class HelloWorldActivity extends Activity
{

    private static TextView txtview;
    /** Called when the activity is first created. */
    @Override
    public void onCreate(Bundle savedInstanceState)
    {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.main);
        txtview = (TextView) findViewById(R.id.mainview);

        Thread t = new Thread(new WeakRunnable(txtview));
        t.start();
    }

    private static final class WeakRunnable implements Runnable {
        private final WeakReference<TextView> mtextview;

        protected WeakRunnable(TextView textview){
            mtextview = new WeakReference<TextView>(textview);
        }

            @Override
            public void run() {
                TextView textview = mtextview.get();
                if (textview != null) {
                    /*
                    try {
                        Thread.sleep(1500);
                    } catch (InterruptedException e) {
                        e.printStackTrace();
                    }
                    */
                    txtview.setText("Hola Mundo");
                    textview = null;
                }
                Log.d("com.example.helloworld", "" + Thread.currentThread().getName()); // Outputs "Thread-<num>" if not running on UI thread
            }
    }           

}

The issue now is that I can't seem to delay the spawned thread in any way, otherwise it works.

This:

try {
    Thread.sleep(1500);
} catch (InterruptedException e) {
    e.printStackTrace();
}

makes the app quit itself and I don't get why. Something tells me I'm delaying it the wrong way.

EDIT2

Thanks to the link @EugenMatynov give me: update ui from another thread in android I understood why the app quitted. It all comes down to the reason You can't call UI methods from threads other than the main thread. and it's bad practice to update the UI from another thread.

Community
  • 1
  • 1
Alper Turan
  • 1,220
  • 11
  • 24

4 Answers4

7

I have a doubt if there's absolutely no memory leak whenever device orientation occurs.

It could be. For 1.5seconds. After the queue is emptied the handler can be garbage collected, and also the old Activity. To be safe override onPause, and call handler.removeCallbacks(null); to clear the Handler's queue

Blackbelt
  • 156,034
  • 29
  • 297
  • 305
  • 4
    In my case, I had to call `handler.removeCallbacksAndMessages(null);`, because as of the android version 23 implementation, `removeCallbacks();` does nothing when you pass null as a parameter(see MessageQueue's `removeMessages();`). – Mateus Gondim Nov 09 '15 at 13:40
4

I think your code is leak free if you use :

private static Handler h = new Handler(); 

or

txtview.postDelayed(new WeakRunnable(txtview),1500);

because you have stored the view as a WeakReference. the method:

txtview.postDelayed(new WeakRunnable(txtview),1500);

simply call main handler of the UI thread so if the activity is destroyed the view is null and the runnable dose nothing.

also because of the weakreference the activity can be garbage collected because there is no strong reference to it.

mmlooloo
  • 18,937
  • 5
  • 45
  • 64
  • After you accepted my answer again I reviewed my answer and your question and found a bug in my answer, my answer is only valid if you use `private static Handler h = new Handler();` or `txtview.postDelayed(new WeakRunnable(txtview),1500);` you can accept blackbeltt answe or let me edit it. also note that running a runnable by delay is different from sleeping on main thread. because I think people make you confusing :-) – mmlooloo Apr 17 '15 at 17:08
  • What's the difference between static and non-static version? – Alper Turan Apr 17 '15 at 17:37
  • Got it, nice explanation. I'm rather new to Java so I have to click some concepts. I opted for `txtview.postDelayed(new WeakRunnable(txtview),1500);` since it looks easier and takes relatively less visual space. Feel free to edit your answer :) – Alper Turan Apr 17 '15 at 17:51
  • 1
    static => member of class - non-static => member of objects – mmlooloo Apr 17 '15 at 17:57
2

h.postDelayed(new WeakRunnable(txtview),1500); I think it will be blocking UI Thread. here is a good sample for memory leak. https://github.com/badoo/android-weak-handler

birds
  • 187
  • 1
  • 8
  • 1
    can you add the relevant code in answer and where the code in question canbe improved? – Nikos M. Apr 17 '15 at 08:46
  • It will not block UI, as well code is exactly same code that written in blog related to library https://techblog.badoo.com/blog/2014/08/28/android-handler-memory-leaks – Eugen Martynov Apr 17 '15 at 08:49
  • That's weird. I thought `h.postDelayed(new WeakRunnable(txtview),1500);` basically started the newly created thread `private Handler h = new Handler();` outside of UI thread – Alper Turan Apr 17 '15 at 08:50
  • @EugenMartynov When executing `Thread.currentThread().getName()` I get it's on the main (UI) thread. So yeah, if it takes much time it will eventually block UI. – Alper Turan Apr 17 '15 at 10:53
  • 1
    It is bad practice to change UI elements out of Main thread http://stackoverflow.com/questions/7005756/update-ui-from-another-thread-in-android. Sure default `Handler` runs on Main thread but posting delayed runnable doesn't block ui. Also setting text should be quite fast so user will not notice anything – Eugen Martynov Apr 17 '15 at 11:03
  • @EugenMartynov Thanks I got it. Does it mean that if I delay the runnable for a few minutes and in the meantime I change another part of the UI it won't block right? – Alper Turan Apr 17 '15 at 11:10
  • 1
    @gizko if I understood you correctly - right. Delayed code will be not executed on scheduled thread. So this thread resource can be used by another activity – Eugen Martynov Apr 17 '15 at 11:13
  • @EugenMartynov Perfect, that's what I needed to know. Your link helped me click the concept. – Alper Turan Apr 17 '15 at 11:15
1

Please do this, otherwise you will be blocking UIThread and it is not recommended. To do this, you can also use a TimerTask, check it here: http://developer.android.com/reference/java/util/TimerTask.html

import android.widget.TextView;
import android.util.Log;
import java.lang.ref.WeakReference;

public class HelloWorldActivity extends Activity
{
    private Handler h = new Handler();
    private static TextView txtview;

    /** Called when the activity is first created. */
    @Override
    public void onCreate(Bundle savedInstanceState)
    {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.main);
        txtview = (TextView) findViewById(R.id.mainview);        

        h.postDelayed(new Runnable() {
           @Override
           public void run() {
              changeText();
           }
        }, 1500);
    }

    public void changeText(){
       txtview.setText("Hola mundo.");
       h.removeCallbacksAndMessages(null);
    }          

}

By the way, you can change orientation in your emulator this way: Ctrl+F12

Grender
  • 1,589
  • 2
  • 17
  • 44
  • **Non static inner class can create memory leaks as your code exactly does it:-)** – mmlooloo Apr 17 '15 at 09:02
  • I have edited the code, maybe removing callbacks and messages will make the code less susceptible to cause memory leak. What do you think? – Grender Apr 17 '15 at 09:07
  • 1
    your leak is caused by using anonymous new Runnable(), actually the OP code is correct, you can remove callbacks at onPause but may be the OP dose not like to do that because for example the user press the home button and after 2s he back to app so he wants to see the change. – mmlooloo Apr 17 '15 at 09:12
  • Unfortunately I have a Chromebook converted to Linux and have no F12 key :/ I have updated my question and spawning a new thread helps avoiding blocking the UIThread, but I can't seem to be able to delay its execution without the app quitting itself. – Alper Turan Apr 17 '15 at 10:56