2013-04-25 16 views
5

Eine App von mir akkumuliert viele Thread Instanzen, die der GC nicht aufnehmen und löschen kann. Dieses Speicherleck stürzt die App auf lange Sicht ab.Warum werden meine Threads nicht abstürzen und ein Speicherleck verursachen?

Ich bin nicht 100% sicher, woher sie kommen, aber ich habe eine deutliche folgende Macht Gefühl der Code in Frage:

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); 
    } 
} 

Dieser Code ermöglicht es mir, einige zu laufen HTTP Verbindungen parallel, die einander nicht auf dem Standard AsyncTaskExecutor (die nur eine einzige Thread-Warteschlange ist) blockieren.

Ich habe überprüft, dass die AsyncTask s tatsächlich ihre onPostExecute() Methoden erreichen und nicht nur für immer laufen. Nach dem Überprüfen einiger Speicherabbilder vermute ich, dass die Umhüllung Thread -Objekte nicht mehr ausgeführt wird, nachdem die AsyncTask s abgeschlossen sind.

Ist es möglich, dass der obige Code immer noch für mein Speicherleck verantwortlich ist, oder sollte ich mich anderswo umsehen?

Jede Hilfe wird geschätzt.

Edit: Es sollte beachtet werden, dass sendRequest nur einmal aufgerufen wird. Andere Teile des Codes, die nicht im obigen Beispiel enthalten sind, stellen dies sicher.

Edit 2: der Super-Klasse sieht wie folgt aus:

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(); 
} 

Die AsyncTask sieht wie folgt aus:

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."); 
    } 
} 
+0

Nicht wirklich sicher, aber kann diese Ihnen helfen? http://www.androiddesignpatterns.com/2013/04/activities-threads-memory-leaks.html – dumazy

+1

Irgendetwas in den Konstruktoren der Super-Klassen von AbstractUraHostConnection, die vielleicht erschrecken? Wie sieht ConnectionTask aus? – ddmps

+0

Code beider Klassen hinzugefügt. – Chris

Antwort

1

Stellvertreter ein ThreadPoolExecutor für Ihre Executor, so dass Sie die Kontrolle über das haben Größe des Pools. Wenn ThreadPoolExecutor im Grunde ein Executor mit exposed-Methoden ist, kann es sich nur um einen Fall handeln, bei dem die standardmäßige maximale Poolgröße sehr hoch ist.

Offizielle doc here.

einen Blick insbesondere Nehmen Sie an:

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. 

Es gibt auch eine Alternative, wenn Sie weniger (bessere Idee codieren wollen, je nachdem wie viel Kontrolle Sie wirklich wollen und wie viel Code, den Sie werden es bekommen handeln).

Executor taskExecutor = Executors.newFixedThreadPool(x); 

wobei x = die Größe Ihres Pools

+0

Vielen Dank. Ja, das löst das Problem. Aber aus Neugier frage ich mich, was in meinem Code schief läuft. Nicht reagierende Threads nach einer Weile zu töten ist keine besonders elegante Lösung :-) – Chris

+1

Antwort bearbeitet für eine Alternative. Threads * sollten automatisch behandelt werden, aber wie wir wissen, macht GC, was GC tut, wenn es will, auch wenn wir direkt fragen.Ich wünschte, ich wüsste mehr über den zugrunde liegenden Mechanismus, aber sie nennen mich Kludge nicht umsonst ... Außerdem sollte es dich davon abhalten, Zombies auf eigene Faust zu jagen, wenn du den Pool einschränkst. – MarsAtomic