Service + Threads = Memory leak

Question

Here's my code (simplified a little bit):

Service

public class TaskService extends Service {
// -----------------------------------------------------------------------
//
// Constants
//
// -----------------------------------------------------------------------
private static final String EXTRA_TASK = "EXTRA_TASK";

private static final String TASK_REGISTER_INSTALLATION = "TASK_REGISTER_INSTALLATION";

private static Handler sHandler = new Handler();

// -----------------------------------------------------------------------
//
// Statics
//
// -----------------------------------------------------------------------
public static void registerInstallation(Context context) {
    Intent intent = new Intent(context, TaskService.class);
    intent.putExtra(EXTRA_TASK, TASK_REGISTER_INSTALLATION);
    context.startService(intent);
}

// -----------------------------------------------------------------------
//
// Fields
//
// -----------------------------------------------------------------------
private List<BaseTask> mTasks = new ArrayList<BaseTask>();

// -----------------------------------------------------------------------
//
// Methods
//
// -----------------------------------------------------------------------
@Override
public IBinder onBind(Intent intent) {
    return null;
}

@Override
public int onStartCommand(Intent intent, int flags, int startId) {
    if (intent != null)
        handleIntent(intent);
    return Service.START_NOT_STICKY;
}

private void handleIntent(Intent intent) {
    String taskType = intent.getStringExtra(EXTRA_TASK);
    if (taskType.equalsIgnoreCase(TASK_REGISTER_INSTALLATION)) {
        RegistrationTask task = new RegistrationTask("",
                "", "", "", "");
        task.setTaskListener(sHandler, mRegistrationListener);
        mTasks.add(task);
        task.start();
    }
}

@Override
public void onDestroy() {
    for (BaseTask task : mTasks)
        task.interrupt();
    mTasks.clear();
    Log.d(TaskService.class.getSimpleName(), "onDestroy");
    super.onDestroy();
}

private TaskListener<String, String, String> mRegistrationListener = new TaskListener<String, String, String>() {
    @Override
    public void onResult(String result, BaseTask task) {
        mTasks.remove(task);
    }

    @Override
    public void onProgress(String progress, BaseTask task) {
        mTasks.remove(task);
    }

    @Override
    public void onError(String error, BaseTask task) {
        Toast.makeText(TaskService.this, error, Toast.LENGTH_SHORT).show();
    }
};

public static interface TaskListener<ResultType, ProgressType, ErrorType> {
    public void onError(ErrorType error, BaseTask task);

    public void onProgress(ProgressType progress, BaseTask task);

    public void onResult(ResultType result, BaseTask task);
}
 }

Thread

public class BaseTask<ResultType, ProgressType, ErrorType> extends Thread {
protected Handler mHandler;
protected TaskListener<ResultType, ProgressType, ErrorType> mTaskListener;
protected HttpRequestFactory mRequestFactory;

public BaseTask() {
    try {
        ApacheHttpTransport.Builder builder = new ApacheHttpTransport.Builder();

        mRequestFactory = builder.doNotValidateCertificate().build()
                .createRequestFactory();
    } catch (GeneralSecurityException e) {
        e.printStackTrace();
    }
}

public void setTaskListener(Handler handler,
        TaskListener<ResultType, ProgressType, ErrorType> listener) {
    mHandler = handler;
    mTaskListener = listener;
}

protected void onError(final ErrorType error) {
    mHandler.post(new Runnable() {
        @Override
        public void run() {
            mTaskListener.onError(error, BaseTask.this);
        }
    });
}

protected void onProgress(final ProgressType progress) {
    mHandler.post(new Runnable() {
        @Override
        public void run() {
            mTaskListener.onProgress(progress, BaseTask.this);
        }
    });
}

protected void onResult(final ResultType result) {
    mHandler.post(new Runnable() {
        @Override
        public void run() {
            mTaskListener.onResult(result, BaseTask.this);
        }
    });
}

  }

public class RegistrationTask extends BaseTask<String, String, String> {

public RegistrationTask(...) {
    super();
}

@Override
public void run() {
    try {
                //Simple web request executed here
    } catch (HttpResponseException e) {
        e.printStackTrace();
        onError(e.getContent());
    } catch (IOException e) {
        e.printStackTrace();
    }
}

I also have an Activity which send Intent each time onCreate method gets called to start the task. The issue is that after several restarts I get about 300rb leaked and even if I call GC from Eclipse I didn't get freed. What am I doing wrong?

UPDATE Activity code:

public class MainActivity extends Activity {

@Override
protected void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);
    setContentView(R.layout.activity_main);
    TaskService.registerInstallation(this);
}

@Override
public boolean onCreateOptionsMenu(Menu menu) {
    // Inflate the menu; this adds items to the action bar if it is present.
    getMenuInflater().inflate(R.menu.main, menu);
    return true;
}

}

As you can see it is very simple.

Answer 1

This answer won't give you a definitive solution, not because I'm not willing, but because it's impossible (and even harder without not just viewing your code, but knowing very well your code). But from my experience I can tell you that those kind of memory leaks doesn't occur just due to directly referenced objects - objects you declare (and keep referencing another classes/objects) in turn depends on many other classes and so on, and probably you're seeing a memory leak due to an incorrect handling of any of your instances which at the same time reference to other instances.

Debugging memory leaks is a often a very hard work, not just because as I said above it sometimes doesn't depend directly on what you've declared, but also because finding a solution might not be trivial. The best thing you can do is what you already seem to be doing: DDMS + HPROF. I don't know how much knowledge you have, but although it's not a universal method, this link helped me so much to find memory leaks in my code.

Although it seems trivial, the best way to debug those kind of things is progresively remove portions of your code (overall, those which implies working with instances of other classes) and see how the HPROF report change.

Answer 2

Your service does not destroy with your activity. Most likely, you are retaining a reference to your Activity in each thread that spawns in the service, which means it does not get destroyed.

If you need to reference your Activity, you should use a WeakReference:

How to use WeakReference in Java and Android development?

If that is not the issue, then objects like your RegistrationTask where we can't see the code are the culprits - but it sounds like you may have already tried removing code bits... you can always post more code.

Answer 3

Well once again I've just made a stupid mistake) I've made a web request which will always return an error (for tests) but in error handler of my service I forgot to remove Thead from the Threads list. Now everything works as I expected.