1

Hi i am trying to create executor for binrary file downloading, i have around 100-200 files to be downloaded and stored in disk.

This is my DownloadExecutor.java

public final class DownloadExecutor {

  private static DownloadExecutor executor;

  private ExecutorService executorService;

  private static final ThreadFactory sThreadFactory = new ThreadFactory() {
    private final AtomicInteger mCount = new AtomicInteger(1);

    public Thread newThread(Runnable r) {
      return new Thread(r, "DownloadExecutor #" + mCount.getAndIncrement());
    }
  };

  public static DownloadExecutor getInstance() {
    if (executor == null) {
      synchronized (DownloadExecutor.class) {
        executor = new DownloadExecutor();
      }
    }
    return executor;
  }

  private DownloadExecutor() {
    final BlockingQueue<Runnable> sPoolWorkQueue =
        new LinkedBlockingQueue<Runnable>(128);
    final int CPU_COUNT = Runtime.getRuntime().availableProcessors();
    // We want at least 2 threads and at most 4 threads in the core pool,
    // preferring to have 1 less than the CPU count to avoid saturating
    // the CPU with background work
    final int CORE_POOL_SIZE = Math.max(2, Math.min(CPU_COUNT - 1, 4));
    final int MAXIMUM_POOL_SIZE = CPU_COUNT * 2 + 1;
    final int KEEP_ALIVE_SECONDS = 2;
    StringBuilder stringBuilder = new StringBuilder();
    stringBuilder.append("CPU: " + CPU_COUNT);
    stringBuilder.append(",CORE POOL: " + CORE_POOL_SIZE);
    stringBuilder.append(",MAX POOL: " + MAXIMUM_POOL_SIZE);
    System.out.println("Executor log: " + stringBuilder.toString());
    ThreadPoolExecutor threadPoolExecutor = new ThreadPoolExecutor(
        CORE_POOL_SIZE, MAXIMUM_POOL_SIZE, KEEP_ALIVE_SECONDS, TimeUnit.SECONDS,
        sPoolWorkQueue, sThreadFactory);
    threadPoolExecutor.allowCoreThreadTimeOut(true);
    executorService = threadPoolExecutor;
  }

  public void execute(Callable<?> callable) {
    System.out.println("Adding");
    executorService.submit(callable);
  }
}

I submit the task using following code DownloadExecutor.getInstance().execute(someCallable);

Initially when i trigger this all downloads success but when i trigger next time it throws java.util.concurrent.RejectedExecutionException.

Note i do no want to use shutDown() on this, is this wish possible to omit shutDown. I came to know java.util.concurrent.RejectedExecutionException occur when you try to submit a task on terminated service.

Code for calling

for (int i = 0; i < totalVideos; i++) {
      try {
        DownloadExecutor.getInstance().execute(new YoutubeFilewriter(downloadRepository,
            videoDao, listResource.data.get(i), parentPath, YoutubeVideoDownloader.this));
      } catch (IOException e) {
        e.printStackTrace();
        ++failedVideos;
      }
    }

Imagine on every button click this code is fired.

silentsudo
  • 6,730
  • 6
  • 39
  • 81
  • 1
    Sidenote, unrelated to question: Consider using `enum` for Singletons in Java to save yourself a lot of trouble with Concurrency, Serialization, etc. – Ben Jun 13 '18 at 09:28
  • Thank your for suggestion. I will consider them from here afterwards. – silentsudo Jun 13 '18 at 09:30
  • Just to continue on this sidenote, your Singleton is indeed quite unsafe for Concurrency. By simply calling `getInstance` from 8 different threads at the same time I could instantiate it 8 times. – Ben Jun 13 '18 at 09:34
  • Any suggestion you wish to suggest? on the existing block of code. – silentsudo Jun 13 '18 at 09:39

1 Answers1

0

Your problem is

final BlockingQueue<Runnable> sPoolWorkQueue = new LinkedBlockingQueue<Runnable>(128);

If all executor threads are busy and your queue is full, RejectedExecutionException is thrown.

Either increase your pool size or use unbounded queue.

Btw. do not use double check locking for your singleton (https://wiki.sei.cmu.edu/confluence/display/java/LCK10-J.+Use+a+correct+form+of+the+double-checked+locking+idiom).

Michal
  • 970
  • 7
  • 11
  • i am pushing only 10 task on executor on each trigger still same error. I was thinking that in total 128 task i submitted 10 when they finished on next trigger i can push new 10 tasks which doesnt seems to be working. – silentsudo Jun 13 '18 at 09:37
  • Maybe add the code where you are submitting the tasks – Michal Jun 13 '18 at 09:44
  • @Mchal i have added calling code please take a look. – silentsudo Jun 13 '18 at 09:46
  • print out executorService.getTaskCount() before each task submission so you can see approximate number of currently scheduled tasks. If you want to execute tasks in batches take a look here https://stackoverflow.com/questions/19348248/waiting-on-a-list-of-future – Michal Jun 13 '18 at 10:01
  • ty for reference my my scope is limited to android, i will check and let you know, i have checked that all my futures are completed succesfully before triggering next call on executor – silentsudo Jun 13 '18 at 10:04
  • I have executed your code with Test Callable and it is working fine. Can you please share your Callable task that you are submitting. – dassum Jun 13 '18 at 11:24