Android:java.util.concurrent.ThreadPoolExecutor

Question

In my app there is RecyclerView with tons of images in it.Images are loaded as user scrolls RecyclerView with this code:

    if(Build.VERSION.SDK_INT >= Build.VERSION_CODES.HONEYCOMB)
        loader.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR,url);
    else
        loader.execute(url);

Unfortunately sometimes when user scrolls fast this error occurs:

Task android.os.AsyncTask$3@73f1d84 rejected from 
java.util.concurrent.ThreadPoolExecutor@8f5f96d[Running, pool size = 9, 
active threads = 9, queued tasks = 128, completed tasks = 279]

Is there way to detect if poolExecutor is full and skip image loading?

Whole Image class:

public class Image extends ImageView {
private AsyncTask<String,Integer,Bitmap> loader;

public Image(Context context) {
    super(context);
    this.setScaleType(ScaleType.FIT_XY);
}

public Image(Context context, AttributeSet attrs) {
    super(context, attrs);
    this.setScaleType(ScaleType.FIT_XY);
}

public void loadURL(String url) {
    if(loader!=null)
        loader.cancel(true);
    loader=new AsyncTask<String, Integer, Bitmap>() {
        @Override
        protected Bitmap doInBackground(String... params) {
            URL url = null;
            byte[] bytes = null;
            HttpURLConnection connection=null;
            try {
                url = new URL(params[0]);
                connection=(HttpURLConnection) url.openConnection();
                connection.setRequestProperty("Connection", "close");
                connection.setRequestMethod("GET");
                connection.setUseCaches(true);
                InputStream is = null;
                is=connection.getInputStream();
                bytes = IOUtils.toByteArray(is);
            } catch (MalformedURLException e) {
                e.printStackTrace();
            } catch (ProtocolException e) {
                e.printStackTrace();
            } catch (IOException e) {
                e.printStackTrace();
            }
            if (connection!=null)
                connection.disconnect();
            Bitmap res=null;
            if(!isCancelled() && bytes!=null)
                res=BitmapFactory.decodeByteArray(bytes,0,bytes.length);
            return res;
        }
        @Override
        protected void onPostExecute(Bitmap res) {
            if(res!=null) {
                setImageBitmap(res);
                _animate();
            }
        }
    };
    if (this.getDrawable()!=null) {
        Bitmap bmp=((BitmapDrawable) this.getDrawable()).getBitmap();
        this.setAnimation(null);
        if (bmp!=null) {
            bmp.recycle();
            //Log.d("image","recycled");
        }
        this.setImageBitmap(null);
    }
    /*
    ThreadPoolExecutor e =(ThreadPoolExecutor) Executors.newFixedThreadPool(9);
    Log.d("pool size",e.getActiveCount()+"/"+e.getMaximumPoolSize());
    if (e.getActiveCount() == e.getMaximumPoolSize()) {
    }
    */
    //start loading
    if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.HONEYCOMB)
        loader.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR, url);
    else
        loader.execute(url);
}
private void _animate() {
    ValueAnimator bgAnim= ValueAnimator.ofObject(new IntEvaluator(),0,255);
    bgAnim.setDuration(500);
    bgAnim.addUpdateListener(new ValueAnimator.AnimatorUpdateListener() {
        @Override
        public void onAnimationUpdate(ValueAnimator animation) {
            Image.this.getDrawable().setAlpha((int) (animation.getAnimatedValue()));
        }
    });
    bgAnim.start();
}

}

Possible duplicate of [android app crashes when exceeded Thread pool 9 and queued tasks 128](https://stackoverflow.com/questions/34395004/android-app-crashes-when-exceeded-thread-pool-9-and-queued-tasks-128) — RoShan Shan, Jun 05 '17 at 08:59
Instead of creating a new async task for uploading ever image, simply create one and upload all the images. — Srikar Reddy, Jun 05 '17 at 08:59
With single AsyncTask it throws. java.lang.IllegalStateException: Cannot execute task: the task is already running. — undefined, Jun 05 '17 at 09:51

score 8 · Answer 1 · answered Jun 14 '17 at 18:43

8

I answer that before (here, here, here and here and probably others) and I answer it again for you: Do not try to re-invent the wheel!

Image loading/caching is a very complex task in Android and a lot of good very developers already did that. Threading is just one of the issues, but I can see from your code you have a memory leak, there's no caching so you'll re-download images again if scroll back to it, HttpURLConnection is a crappy network layer.

So, the way to solve this (IMHO) it's just to re-use work done by other developers. Good examples of libraries you should consider for that are:

Picasso - https://github.com/square/picasso
Glide - https://github.com/bumptech/glide
Fresco - https://github.com/facebook/fresco

Picasso is my favorite, so to use it you need to simply call:

Picasso.with(context).load(url).into(imgView);

and it all be handled for you.

answered Jun 14 '17 at 18:43

Budius

39,391
16
102
144

where do you see memory leak? I do cache images with this line `connection.setUseCaches(true);` and I don't see other issues except this crash which I already fixed with try/catch – undefined Jun 15 '17 at 14:44
@undefined the AsyncTask have a reference to the ImageView which have reference to the Activity context. The thread is running and the Activity gets destroyed -> memory leak. – Budius Jun 15 '17 at 18:16
I see no references to `ImageView` within AsyncTask(except `this` reference which can't lead to a leak) – undefined Jun 16 '17 at 14:18
Anonymous inner classes (your AsyncTask is one) always have a reference to the object around them. That's why you can on your `onPostExecute` to call `setImageBitmap` that belongs to the `ImageView`. It is holding a reference to it. – Budius Jun 16 '17 at 17:14
yeah but reference to AsyncTask(loader) should be garbage collected with ImageView instance isn't it so? – undefined Jun 17 '17 at 18:27
No. Because the thread that executes the `doInBackground` method is active and holds the AsyncTask (which holds the ImageView, which holds the Activity). This is 1 of the 2 most common memory leaks in Android: 1) `Thread` doing background job holds something that holds the `Activity`; 2) `static` reference. – Budius Jun 17 '17 at 18:51
but when `doInBackground`is finished will GC be able to kill ImageView with all references? – undefined Jun 18 '17 at 19:18
"when it finishes" yes. But that is an undetermined time in the future, as developers we shouldn't work with that type of stuff. – Budius Jun 18 '17 at 19:21

score 4 · Answer 2 · edited Jun 05 '17 at 09:05

4

You can check if active threads count is equal to thread pool maximum size then your thread pool is full by using this

ThreadPoolExecutor e =(ThreadPoolExecutor)Executors.newFixedThreadPool(totalnofthreads);
if (e.getActiveCount() == e.getMaximumPoolSize())
{

}

edited Jun 05 '17 at 09:05

Ravindra Kushwaha

7,846
14
53
103

answered Jun 05 '17 at 09:01

Meenu Meena

111
7

Size of your threadpool, as its showing 9 in logcat in your question. java.util.concurrent.ThreadPoolExecutor@8f5f96d[Running, pool size = 9, active threads = 9, queued tasks = 128, completed tasks = 279] – Meenu Meena Jun 05 '17 at 09:23
just tried e.getActiveCount() always returns 0 although defenetely there are some active threads. – undefined Jun 05 '17 at 10:04

score 3 · Accepted Answer · answered Jun 10 '17 at 07:58

I just relized I can wrap loading code with try/catch:

try {
     if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.HONEYCOMB)
            loader.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR, url);
        else
            loader.execute(url);
    } catch (RejectedExecutionException e){
        e.printStackTrace();
}

Looks like this would be optional solution.

score 2 · Answer 4 · answered Jun 08 '17 at 15:53

To detect if user scrolls fast you can use onFlingListener()

recyclerView.setOnFlingListener(new RecyclerView.OnFlingListener() {

    @Override
    public boolean onFling(int velocityX, int velocityY) {
        isFlinging = checkFlinging(velocityY);

        if (isFlinging) {
            //Stop image loading here
        }

        return false;
    }
});

private boolean checkFlinging(int velocityY) {
    return (velocityY < 0 && velocityY < -RECYCLER_VIEW_FLING_VELOCITY) 
            || (velocityY > 0 && velocityY > RECYCLER_VIEW_FLING_VELOCITY);
}

Let me explain a bit, velocityY because I use recyclerView with vertical scrolling (for horizontal scrolling just change this parameter to velocityX), RECYCLER_VIEW_FLING_VELOCITY - your fling velocity, for me RECYCLER_VIEW_FLING_VELOCITY = 7000.

score 2 · Answer 5 · answered Jun 13 '17 at 12:09

Instead of using AsyncTask.THREAD_POOL_EXECUTOR you can use your own Executor instance with a RejectedExecutionHandler, for example:

private final Executor mExecutor = new ThreadPoolExecutor(0, 8,
           1, TimeUnit.SECONDS,
           new LinkedBlockingQueue<Runnable>(16),
           new ThreadPoolExecutor.DiscardPolicy());

This will create an Executor that runs at most 8 threads, keeps 16 tasks in the queue and drops any tasks above those limits (DiscardPolicy is a predefined RejectedExecutionHandler that does exactly that). You can then pass it to executeOnExecutor().

Alternatively, you might want the images to eventually load. In that case you can use an Executor with an unbounded (i.e. limitless) queue. It will never throw a RejectedExecutionException. Executors utility class has some nice factory method for creating those:

private final Executor mExecutor = Executors.newFixedThreadPool(8);

score 2 · Answer 6 · answered Jun 15 '17 at 04:40

when ImageView detached from window , cancel the corresponding task of loading url :

public class Image extends ImageView {

  @Override protected void onDetachedFromWindow() {
    super.onDetachedFromWindow();
    if(loader!=null)
      loader.cancel(true);
  }

}

Smile · Answer 7 · 2017-06-15T16:56:43.130

First of all, why are you still using AsyncTask? the ThreadPool exception is coming because while your scrolling fast the adapter is trying to set an image to a position which is no longer available usually to stop this issue you would disable recycling but that would only make your list slow when handling a large set of data. So i advise you use volley for image loading its easy to implement and handles caching easiily.

      <com.android.volley.toolbox.NetworkImageView
            android:layout_width="match_parent"
            android:layout_height="wrap_content"
            android:id="@+id/mainImage"
            android:scaleType="centerCrop"
            android:adjustViewBounds="true"
            android:maxHeight="270dp"
            />

Use above in place of your imageview and create the volleySingleton class to handle all network requests

    public class VolleySingleton {
private static VolleySingleton sInstance = null;
private RequestQueue mRequestQueue;
private ImageLoader imageLoader;

private VolleySingleton(){
    mRequestQueue = Volley.newRequestQueue(Application.getAppContext());
    imageLoader = new ImageLoader(mRequestQueue, new ImageLoader.ImageCache() {
        private final LruCache<String, Bitmap> cache = new LruCache<String, Bitmap>(200);


        @Override
        public Bitmap getBitmap(String url) {
            return cache.get(url);
        }


        @Override
        public void putBitmap(String url, Bitmap bitmap) {
            cache.put(url, bitmap);
        }

    });

}
public static VolleySingleton getsInstance(){
    if(sInstance == null){
        sInstance = new VolleySingleton();
    }
    return sInstance;
}
public RequestQueue getmRequestQueue(){
    return mRequestQueue;
}
public ImageLoader getImageLoader() {
    return imageLoader;
} }

Get an instance of your singleton class then add it to the imageView and your good to go

     imageLoader = VolleySingleton.getsInstance().getImageLoader();
    networkImageVeiw.setImageUrl(imageUrl, imageLoader);

Create a class extending android.app.Application so you can get the context in the volleySingleton class

    public class Application extends android.app.Application {
private static Application sInstance;

public static Application getsInstance() {
    return sInstance;
}

public static Context getAppContext() {
    return sInstance.getApplicationContext();
}

public static AppEventsLogger logger(){
    return logger;
}

@Override
public void onCreate() {
    super.onCreate();
    sInstance = this;
 } }

Don't forget to go to your manifest.xml and add the name attribute to the application tag to be the name of your application class you just extended

    <application
    android:name="com.example.ApplicationClass"

here is a link to get instructions on installing volley and some helpful tips volley library here

Why do you think `AsyncTask` is so bad?I believe these failed requests go for images which is not on the screen already so I can just skip them with try/catch.If you know any other disadvantages of `AsyncTask` I would like to hear it. — undefined, Jun 15 '17 at 15:09
you can use try catch but if your not handling the exception explicitly but just printing the stacktrace why not avoid it happening unless the code explicitly cannot be written without try catch. Besides I have nothing against AsyncTask its just its not the best use in this case maybe when android was not as developed as it is now it was usefull but now there are better tools to model your network which are asynchronous and have less stress to work with. If he was inserting into a db then AsyncTask would have been the best solution to do. — Smile, Jun 15 '17 at 16:53
Just to correct an impression, of AsyncTask being the best for database operations. Currently it's advised to use a loader, because AsyncTasks if not used properly can cause memory leaks. — Smile, Jan 09 '18 at 11:03

Android:java.util.concurrent.ThreadPoolExecutor

7 Answers7