Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Why won't my Threads die and cause a memory leak?

An app of mine is accumulating a lot of Thread instances that the GC can't pick up and clear out. This memory leak crashes the app in the long run.

I'm not 100% sure where they come from, but I have a distinct feeling the following might be the code in question:

public class UraHostHttpConnection extends AbstractUraHostConnection {
    private Handler uiThreadHandler = new Handler(Looper.getMainLooper());
    private Executor taskExecutor = new Executor() {
         public void execute(Runnable command) {
             new Thread(command).start();
        }
    };
    private ConnectionTask task = null;

    @Override
    public void sendRequest(final HttpUriRequest request) {
        this.task = new ConnectionTask();
        this.uiThreadHandler.post(new Runnable() {
            public void run() {
                task.executeOnExecutor(taskExecutor, request);
            }
        });
   }

    @Override
    public void cancel() {
        if (this.task != null)
            this.task.cancel(true);
    }
}

This code allows me to run several HTTP connections in parallel that won't block each other on the default AsyncTask Executor (which is just a single threaded queue).

I checked, that the AsyncTasks are in fact reaching their onPostExecute() methods and don't just run forever. After inspecting some memory dumps I suspect the wrapping Thread-Objects to not stop running after the AsyncTasks are completed.

Is it possible that the above code is still responsible for my memory leak, or should I start looking elsewhere?

Any help is appreciated.

Edit: It should be noted, that sendRequest is only ever called once. Other parts of the code not in the sample above make sure of that.

Edit 2: the super-class looks like this:

public abstract class AbstractUraHostConnection {
    protected IUraHostConnectionListener listener = null;

    public void setListener(IUraHostConnectionListener listener) {
        this.listener = listener;
    }
    public abstract void sendRequest(HttpUriRequest request);
    public abstract void cancel();
}

The AsyncTask looks like this:

private class ConnectionTask extends AsyncTask<HttpUriRequest, Object, Void> {
    final byte[] buffer = new byte[2048];
    private ByteArrayBuffer receivedDataBuffer = new ByteArrayBuffer(524288);

    @Override
    protected Void doInBackground(HttpUriRequest... arg0) {
        UraHostHttpConnection.taskCounter++;
        AndroidHttpClient httpClient = AndroidHttpClient.newInstance("IVU.realtime.app");
        try {
            // Get response and notify listener
            HttpResponse response = httpClient.execute(arg0[0]);
            this.publishProgress(response);

            // Check status code OK before proceeding
            if (response.getStatusLine().getStatusCode() == 200) {
                HttpEntity entity = response.getEntity();
                InputStream inputStream = entity.getContent();
                int readCount = 0;

                // Read one kB of data and hand it over to the listener
                while ((readCount = inputStream.read(buffer)) != -1 && !this.isCancelled()) {
                    this.receivedDataBuffer.append(buffer, 0, readCount);
                    if (this.receivedDataBuffer.length() >= 524288 - 2048) {
                        this.publishProgress(receivedDataBuffer.toByteArray());
                        this.receivedDataBuffer.clear();
                    }
                }

                if (this.isCancelled()) {
                    if (arg0[0] != null && !arg0[0].isAborted()) {
                        arg0[0].abort();
                    }
                }
            }
        } catch (IOException e) {
            // forward any errors to listener
            e.printStackTrace();
            this.publishProgress(e);
        } finally {
            if (httpClient != null)
                httpClient.close();
        }

        return null;
    }

    @Override
    protected void onProgressUpdate(Object... payload) {
        // forward response
        if (payload[0] instanceof HttpResponse)
            listener.onReceiveResponse((HttpResponse) payload[0]);
        // forward error
        else if (payload[0] instanceof Exception)
            listener.onFailWithException((Exception) payload[0]);
        // forward data
        else if (payload[0] instanceof byte[])
            listener.onReceiveData((byte[]) payload[0]);
    }

    @Override
    protected void onPostExecute(Void result) {
        listener.onReceiveData(this.receivedDataBuffer.toByteArray());
        listener.onFinishLoading();
        UraHostHttpConnection.taskCounter--;
        Log.d(TAG, "There are " + UraHostHttpConnection.taskCounter + " running ConnectionTasks.");
    }
}
like image 623
Chris Avatar asked Nov 12 '22 06:11

Chris


1 Answers

Substitute a ThreadPoolExecutor for your Executor so that you have control over the size of the pool. If ThreadPoolExecutor is basically an Executor with exposed methods, it may just be a case where the default maximum pool size is set very high.

Official doc here.

Take a look in particular at:

setCorePoolSize(int corePoolSize)
//Sets the core number of threads.

setKeepAliveTime(long time, TimeUnit unit)
//Sets the time limit for which threads may remain idle before being terminated.

setMaximumPoolSize(int maximumPoolSize)
//Sets the maximum allowed number of threads.

There's also an alternative if you want to code less (better idea depending on how much control you really want and how much code you'll trade to get it).

Executor taskExecutor = Executors.newFixedThreadPool(x);

where x = the size of your pool

like image 198
MarsAtomic Avatar answered Nov 14 '22 21:11

MarsAtomic