Android Surfaceview Threads and memory leaks

Question

Im creating a game in android, and i noticed that the game has a memory leak. Iv managed to isolate the memory leak into a smaller application so that i can see well try and work out, how to fix it.

The application uses a surfaceview for its view and has a thread attached to that in order to do all the drawing to the screen. The memory leak happens when i start a new activity and close the one that im currently using. I can see this when i do a memory dump on my test application as all it does is open and close an activity (activity a -> activity b -> activity a). Iv kind of ran out of ideas as to how i can fix this as iv tried nulling all my references that i do create to the view (inside the thread), iv tried removing the callback from the surfaceview when i destroy the view, and also inside the activity, it doesn't seem to make any difference.

MemoryLeakActivity.java

package memory.leak;

import memory.leak.view.MemoryLeak;
import android.app.Activity;
import android.os.Bundle;

public class MemoryLeakActivity extends Activity {
    /** Called when the activity is first created. */
    @Override
    public void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(new MemoryLeak(this));
    }
}

MemoryLeakViewThread.java

package memory.leak.thread;

import memory.leak.view.MemoryLeak;
import android.view.SurfaceHolder;
import android.graphics.Canvas;


public class MemoryLeakViewThread extends Thread {
    private MemoryLeak view;
    private boolean run =false;

    public MemoryLeakViewThread(MemoryLeak view) {
        this.view =view;
    }

    public void setRunning(boolean run) {
        this.run =run;
    }

    @Override
    public void run() {
        Canvas canvas =null;
        SurfaceHolder holder =this.view.getHolder();
        while(this.run) {
            canvas =holder.lockCanvas();
            if(canvas !=null) {
                this.view.onDraw(canvas);
                holder.unlockCanvasAndPost(canvas);
            }
        }
        holder =null;
        this.view =null;
    }
}

MemoryLeak.java

package memory.leak.view;

import memory.leak.TestActivity;
import memory.leak.thread.MemoryLeakViewThread;
import android.app.Activity;
import android.content.Context;
import android.content.Intent;
import android.graphics.Canvas;
import android.graphics.Color;
import android.view.GestureDetector;
import android.view.MotionEvent;
import android.view.SurfaceHolder;
import android.view.SurfaceView;
import android.view.GestureDetector.OnGestureListener;


public class MemoryLeak extends SurfaceView implements SurfaceHolder.Callback, OnGestureListener {
    private GestureDetector gesture;
    private MemoryLeakViewThread vThread;
    private Context context;

    public MemoryLeak(Context context) {
        super(context);

        this.getHolder().addCallback(this);
        this.vThread =new MemoryLeakViewThread(this);

        this.gesture =new GestureDetector(this);
        this.context =context;
    }

    public void surfaceChanged(SurfaceHolder holder, int format, int width, int height) {}

    public void surfaceCreated(SurfaceHolder holder) {
        if(!this.vThread.isAlive()) {
            this.vThread =new MemoryLeakViewThread(this);
            this.vThread.setRunning(true);
            this.vThread.start();
        }
    }

    public void surfaceDestroyed(SurfaceHolder holder) {
        boolean retry = true;

        if(this.vThread.isAlive()) {
            this.vThread.setRunning(false);
            while(retry) {
                try {
                    this.vThread.join();
                    retry =false;
                } catch(Exception ee) {}
            }
        }

        this.vThread =null;
        this.context =null;
    }

     public boolean onTouchEvent(MotionEvent event) {
         return this.gesture.onTouchEvent(event);
     }

    @Override
    protected void onSizeChanged(int w, int h, int oldw, int oldh) {
    }

    @Override
    public void onDraw(Canvas canvas) {
        canvas.drawColor(Color.WHITE);
    }   

    @Override
    public boolean onDown(MotionEvent e) {
        return true;
    }

    @Override
    public boolean onFling(MotionEvent e1, MotionEvent e2, float velocityX, float velocityY) {
        return false;
    }

    @Override
    public void onLongPress(MotionEvent e) {}

    @Override
    public boolean onScroll(MotionEvent e1, MotionEvent e2, float distanceX, float distanceY) {
        return false;
    }

    @Override
    public void onShowPress(MotionEvent e) {}

    @Override
    public boolean onSingleTapUp(MotionEvent e) {
        Intent helpScreenIntent =new Intent(this.context, TestActivity.class);
        this.context.startActivity(helpScreenIntent);

        if (this.context instanceof Activity)
            ((Activity) this.context).finish();

        return true;
    }
}

TestActivity.java

package memory.leak;

import memory.leak.view.Test;
import android.app.Activity;
import android.os.Bundle;

public class TestActivity extends Activity {
    /** Called when the activity is first created. */
    @Override
    public void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(new Test(this));
    }
}

TestViewThread.java

package memory.leak.thread;

import memory.leak.view.Test;
import android.view.SurfaceHolder;
import android.graphics.Canvas;


public class TestViewThread extends Thread {
    private Test panel;
    private boolean run =false;

    public TestViewThread(Test panel) {
        this.panel =panel;
    }

    public void setRunning(boolean run) {
        this.run =run;
    }

    @Override
    public void run() {
        Canvas canvas =null;
        SurfaceHolder holder =this.panel.getHolder();
        while(this.run) {
            canvas =holder.lockCanvas();
            if(canvas !=null) {
                this.panel.onDraw(canvas);
                holder.unlockCanvasAndPost(canvas);
            }
        }
        holder =null;
        this.panel =null;
    }
}

Test.java

package memory.leak.view;

import memory.leak.MemoryLeakActivity;
import memory.leak.thread.TestViewThread;
import android.app.Activity;
import android.content.Context;
import android.content.Intent;
import android.graphics.Canvas;
import android.graphics.Color;
import android.view.GestureDetector;
import android.view.MotionEvent;
import android.view.SurfaceHolder;
import android.view.SurfaceView;
import android.view.GestureDetector.OnGestureListener;


public class Test extends SurfaceView implements SurfaceHolder.Callback, OnGestureListener {
    private GestureDetector gesture;
    private TestViewThread vThread;
    private Context context;

    public Test(Context context) {
        super(context);

        this.getHolder().addCallback(this);
        this.vThread =new TestViewThread(this);

        this.gesture =new GestureDetector(this);
        this.context =context;
    }

    public void surfaceChanged(SurfaceHolder holder, int format, int width, int height) {}

    public void surfaceCreated(SurfaceHolder holder) {
        if(!this.vThread.isAlive()) {
            this.vThread =new TestViewThread(this);
            this.vThread.setRunning(true);
            this.vThread.start();
        }
    }

    public void surfaceDestroyed(SurfaceHolder holder) {
        boolean retry = true;

        if(this.vThread.isAlive()) {
            this.vThread.setRunning(false);
            while(retry) {
                try {
                    this.vThread.join();
                    retry =false;
                } catch(Exception ee) {}
            }
        }

        this.vThread =null;
        this.context =null;
    }

     public boolean onTouchEvent(MotionEvent event) {
         return this.gesture.onTouchEvent(event);
     }

    @Override
    protected void onSizeChanged(int w, int h, int oldw, int oldh) {
    }

    @Override
    public void onDraw(Canvas canvas) {
        canvas.drawColor(Color.RED);
    }   

    @Override
    public boolean onDown(MotionEvent e) {
        return true;
    }

    @Override
    public boolean onFling(MotionEvent e1, MotionEvent e2, float velocityX, float velocityY) {
        return false;
    }

    @Override
    public void onLongPress(MotionEvent e) {}

    @Override
    public boolean onScroll(MotionEvent e1, MotionEvent e2, float distanceX, float distanceY) {
        return false;
    }

    @Override
    public void onShowPress(MotionEvent e) {}

    @Override
    public boolean onSingleTapUp(MotionEvent e) {
        Intent helpScreenIntent =new Intent(this.context, MemoryLeakActivity.class);
        this.context.startActivity(helpScreenIntent);

        if (this.context instanceof Activity)
            ((Activity) this.context).finish();

        return true;
    }
}

--Edit-- I made changes to the view class to its surfaceDestroyed(SurfaceHolder holder) so that it will set the view that the thread has to null when the thread is told to stop. The changes i made are

public void surfaceDestroyed(SurfaceHolder holder) {
        boolean retry = true;

        if(this.vThread.isAlive()) {
            this.vThread.setRunning(false);
            while(retry) {
                try {
                    this.vThread.join();
                    retry =false;
                } catch(Exception ee) {}
            }
            this.vThread.setRunning(false, null);
        }

        this.vThread =null;
        this.context =null;
        this.gesture =null;
    }

you also need to change the surfaceCreated(SurfaceHolder holder) method to

public void surfaceCreated(SurfaceHolder holder) {
        if(!this.vThread.isAlive()) {
            this.vThread =new MemoryLeakViewThread();
            this.vThread.setRunning(true, this);
            this.vThread.start();
        }
    }

then on the thread class we need to change the following

public MemoryLeakViewThread() {
    }

    public void setRunning(boolean run) {
        this.run =run;
    }

    public void setRunning(boolean run, MemoryLeak view) {
        this.run =run;
        this.view =view;
    }

By doing this it seemed to of fixed the problem, the only problem now is the thread seems to stay in memory, due to the thread class and thread group. But im thinking this might be due to the debugger.

Adding exception and stack-trace will help to know the issue. — Arslan Anwar, Aug 29 '11 at 09:33
Iv managed to fix most of the memory leak issues by passing the view when i set the running state of the thread, and then setting that to null when i set the running state to false. This has removed both the activity and view from memory now the only thing that stays is the thread which seems to be stuck inside the threadgroup class. I do remember reading something about threads will get stuck in there if there not started i just cant find the link to it now. — Spider, Aug 29 '11 at 09:47
http://code.google.com/p/android/issues/detail?id=7979 thats it. As for the stack-trace, where do you want me to add the exception? It isn't throwing any, well it will eventually when it runs out of memory, but as im not using much memory with my test application it will take a while. Im using MAT to analyze a heap dump so i can see what is getting stuck in memory — Spider, Aug 29 '11 at 09:50
Out of interest, I ran your code, and added some logging. You say the activities are removed - however if I run 'adb shell dumpsys meminfo memory.leak' on the command line, I see a new activity added to the stack on every switch even though onDestroy() IS called and the thread loops seem to exit OK. So I don't know how to fix it, but I thought that might be useful information for you. — NickT, Aug 29 '11 at 11:22
Yeah thats the original problem i had, iv managed to get it to a point so that the threads just stay in memory, im thinking that has something to do with the debugger though. — Spider, Aug 29 '11 at 11:29

score 6 · Accepted Answer · edited May 23 '17 at 10:29

6

You should not create new Thread in the constructor when you are creating it in onSurfaceCreated. Compare your code to my example: How can I use the animation framework inside the canvas?

edited May 23 '17 at 10:29

Community

1
1

answered Aug 29 '11 at 15:14

Lumis

21,517
8
63
67

Tryed what you said and removed the creation of the thread out of the constructor method and placed it inside the Surfacecreated method instead. This stopped the threads from getting stuck in memory. :D all my memory issues are fixed now. – Spider Aug 30 '11 at 08:30

score -1 · Answer 2 · answered Aug 29 '11 at 12:37

-1

As you can see here:

http://developer.android.com/resources/articles/avoiding-memory-leaks.html

The easiest way to start a memory leak in Android is to pass a view's constructor the whole activity instead of the application context. Have you try to change this line:

setContentView(new MemoryLeak(this));

into this one:

setContentView(new MemoryLeak(Context.getApplicationContext()));

?

Hope it helps.

answered Aug 29 '11 at 12:37

Luis Ollero

383
1
2
8

1

Well you obviously haven't tried it, or you would have seen what happens when you try to compile a static reference to a non static method. – NickT Aug 29 '11 at 13:51

Android Surfaceview Threads and memory leaks

2 Answers2

Linked