2
public class CopyService extends Service {

private List<CustomFile> taskList;
private AsyncTask fileTask;

@Override
public void onCreate() {
    super.onCreate();
    taskList = new ArrayList<>();
    fileTask = new fileTaskAsync();
}

@Override
public int onStartCommand(Intent intent, int flags, int startId) {

    String filePath = intent.getStringExtra("filePath");
    String fileType = intent.getStringExtra("fileType");
    String taskType = intent.getStringExtra("taskType");
    String fileName = intent.getStringExtra("fileName");

    CustomFile customFile = new CustomFile();
    customFile.filePath = filePath;
    customFile.fileType = fileType;
    customFile.taskType = taskType;
    customFile.fileName = fileName;
    taskList.add(customFile);

    Notification notification = getNotification();

    startForeground(787, notification);

    if (fileTask.getStatus() != AsyncTask.Status.RUNNING) {

        CustomFile current = taskList.get(0);
        taskList.remove(current);

        fileTask = new fileTaskAsync().execute(current);
    }

    stopSelf();
    return START_NOT_STICKY;
}

@Nullable
@Override
public IBinder onBind(Intent intent) {
    return null;
}

private class fileTaskAsync extends AsyncTask<CustomFile, Void, String> {

    @Override
    protected String doInBackground(CustomFile... customFiles) {

        CustomFile customFile = customFiles[0];

        FileUtils.doFileTask(customFile.filePath, customFile.fileType,
                customFile.taskType);

        return customFile.fileName;
    }

    @Override
    protected void onPostExecute(String name) {
        sendResult(name);

        if (!taskList.isEmpty()) {
            CustomFile newCurrent = taskList.get(0);
            taskList.remove(newCurrent);
            fileTask = new fileTaskAsync().execute(newCurrent);
        }
    }
}

private void sendResult(String name) {
    Intent intent = new Intent("taskStatus");
    intent.putExtra("taskName", name);
    LocalBroadcastManager.getInstance(this).sendBroadcast(intent);
}

}

I need to execute multiple tasks in a service one by one. Task is either copying or moving local files. Suppose, user is copying a big file and he wants to copy or move other files. I need the subsequent tasks to be queued and exected one by one.

Currently, I'm creating a list inside the service and running an async task. In onPostExecute, I check for remaining tasks in the list and start the async task again from there. As shown in the code.

But, I'm concerned about memory leaks. And I'm very new to programming so, I don't know what's the best practice in such situations.

I can't use IntentService, because I want the task to continue even if the user hits home button to open some other app.

whiteShadoww
  • 81
  • 1
  • 9
  • What kind of work is it you're doing in your Service? Could you post the code you've written so far? – PPartisan Apr 20 '19 at 20:38
  • @PPartisan I edited my post and included the code. The reason I didn't post code earlier is because I wanted to know a general solution regardless of what the code is. But, I guess I was a bit unclear in my question. – whiteShadoww Apr 21 '19 at 10:36
  • 1
    I think your solution is reasonable. For the work you're doing (long running work that needs to be executed immediately) a `ForegroundService` is the best solution. As an alternative to `AsyncTask`, you could use an `ExecutorService` or (if you don't mind incorporating an extra library) `RxJava`. I would probably get all the work that needs to be done though and execute it at once, rather than killing/restarting multiple tasks for each operation. You can also make your `AsyncTask` class `static`, so it doesn't hold an implicit reference to the `Service`. Use `application context` if needed. – PPartisan Apr 21 '19 at 10:53
  • Can you please tell me how to use application context? Is is getApplication() or getApplicationContext() ? – whiteShadoww Apr 21 '19 at 11:18
  • Make your inner class `static` (non-static nested classes hold an implicit reference to their containing class, which *can* result in memory leaks when the enclosing class extends `Context` (which `Service` does) and threading is involved), pass in the `Service` as a constructor argument, and only retain a reference to `context.getApplicationContext()`. The Application Context is a de facto singleton, so you don't need to worry about "leaking" it. I'll fill out an answer. – PPartisan Apr 21 '19 at 13:13

1 Answers1

2

AS I said in the comments, I think your solution is reasonable. A Foreground Service is a good candidate for long running work that needs to be executed immediately, and from your description your file copying task matches that criteria.

That said, I don't believe AsyncTask is a good candidate for your problem. AsyncTasks are best deployed when you need to do some quick work off the main thread, in the order of a few hundred milliseconds at most, whereas your copy task could presumably take several seconds.

As you have multiple tasks to complete which aren't directly dependent on one another, I would recommend you make use of a thread pool to conduct this work. For that you can use an ExecutorService:

public class CopyService extends Service {

private final Deque<CustomFile> tasks = new ArrayDeque<>();
private final Deque<Future<?>> futures = new LinkedBlockingDequeue<>();
private final ExecutorService executor = Executors.newCachedThreadPool();

@Override
public int onStartCommand(Intent intent, int flags, int startId) {

    //May as well add a factory method to your CustomFile that creates one from an Intent
    CustomFile customFile = CustomFile.fromIntent(intent);
    tasks.offer(customFile);

    //...Add any other tasks to this queue...

    Notification notification = getNotification();

    startForeground(787, notification);

    for(CustomFile file : tasks) {
       final Future<?> future = executor.submit(new Runnable() {
           @Override
           public void run() {
               final CustomFile file = tasks.poll();
               //Ddo work with the file...
               LocalBroadcastManager.getInstance(CopyService.this).sendBroadcast(...);
               //Check to see whether we've now executed all tasks. If we have, kill the Service.
               if(tasks.isEmpty()) stopSelf();
           }
       });
       futures.offer(future);
    }
    return START_NOT_STICKY;
}

@Override
public void onDestroy() {
    super.onDestroy();
    //Clear pending and active work if the Service is being shutdown
    //You may want to think about whether you want to reschedule any work here too
    for(Future<?> future : futures) {
        if(!future.isDone() && !future.isCancelled()) {
            future.cancel(true); //May pass "false" here. Terminating work immediately may produce side effects.
        } 
    }
}

@Nullable
@Override
public IBinder onBind(Intent intent) {
    return null;
}

This shouldn't cause any memory leaks, as any pending work is destroyed along with the Service.

PPartisan
  • 8,173
  • 4
  • 29
  • 48
  • Hi, first, let me thank you for the answer. This seem like exactly what I wanted, as it isn't specific to my posted code only and I can use it for whatever future projects. However, there seems to be a little problem. I'm unable to make the global variables tasks, future and executor final. It says cannot assign value to final variable. And stopself() doesn't get called for some reason. And the notification goes on forever even after the task is completed. – whiteShadoww Apr 21 '19 at 16:11
  • 1
    Ah, you're right - those `final` variables will need to be assigned in the constructor of the Service. I was writing this without an IDE so I may have missed a few minor points. I think I can see why `stopSelf()` may never be called as well - the check should come at the end of execution in the Runnable. I've updated my answer to account for this. – PPartisan Apr 21 '19 at 16:21
  • Thanks, it is working perfectly! However, could you please tell me how to update the foreground service notification with each task? – whiteShadoww Apr 21 '19 at 17:14
  • 1
    Incorporate it into the `Runnable` you pass into `executor#submit`. Once your copy has finished you can update your notification. – PPartisan Apr 21 '19 at 17:24