Fix probable race condition in DownloadManager

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=195570401
This commit is contained in:
olly 2018-05-06 00:15:40 -07:00 committed by Oliver Woodman
parent 5adfd7e9d9
commit c5e8f6ff9e

View File

@ -39,6 +39,7 @@ import java.io.IOException;
import java.lang.annotation.Retention; import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy; import java.lang.annotation.RetentionPolicy;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.CopyOnWriteArraySet; import java.util.concurrent.CopyOnWriteArraySet;
/** /**
@ -178,10 +179,13 @@ public final class DownloadManager {
} }
/** /**
* Stops all of the tasks and releases resources. If the action file isn't up to date, * Stops all of the tasks and releases resources. If the action file isn't up to date, waits for
* waits for the changes to be written. * the changes to be written. The manager must not be accessed after this method has been called.
*/ */
public void release() { public void release() {
if (released) {
return;
}
released = true; released = true;
for (int i = 0; i < tasks.size(); i++) { for (int i = 0; i < tasks.size(); i++) {
tasks.get(i).stop(); tasks.get(i).stop();
@ -200,6 +204,7 @@ public final class DownloadManager {
/** Stops all of the download tasks. Call {@link #startDownloads()} to restart tasks. */ /** Stops all of the download tasks. Call {@link #startDownloads()} to restart tasks. */
public void stopDownloads() { public void stopDownloads() {
Assertions.checkState(!released);
if (!downloadsStopped) { if (!downloadsStopped) {
downloadsStopped = true; downloadsStopped = true;
for (int i = 0; i < activeDownloadTasks.size(); i++) { for (int i = 0; i < activeDownloadTasks.size(); i++) {
@ -211,6 +216,7 @@ public final class DownloadManager {
/** Starts the download tasks. */ /** Starts the download tasks. */
public void startDownloads() { public void startDownloads() {
Assertions.checkState(!released);
if (downloadsStopped) { if (downloadsStopped) {
downloadsStopped = false; downloadsStopped = false;
maybeStartTasks(); maybeStartTasks();
@ -245,6 +251,7 @@ public final class DownloadManager {
* @throws IOException If an error occurs deserializing the action. * @throws IOException If an error occurs deserializing the action.
*/ */
public int handleAction(byte[] actionData) throws IOException { public int handleAction(byte[] actionData) throws IOException {
Assertions.checkState(!released);
ByteArrayInputStream input = new ByteArrayInputStream(actionData); ByteArrayInputStream input = new ByteArrayInputStream(actionData);
DownloadAction action = DownloadAction.deserializeFromStream(deserializers, input); DownloadAction action = DownloadAction.deserializeFromStream(deserializers, input);
return handleAction(action); return handleAction(action);
@ -258,31 +265,32 @@ public final class DownloadManager {
* @return The id of the newly created task. * @return The id of the newly created task.
*/ */
public int handleAction(DownloadAction action) { public int handleAction(DownloadAction action) {
Task task = createTask(action); Assertions.checkState(!released);
Task task = addTaskForAction(action);
if (actionFileLoadCompleted) {
notifyListenersTaskStateChange(task);
saveActions(); saveActions();
if (downloadsStopped && !action.isRemoveAction) {
logd("Can't start the task as downloads are stopped", task);
} else {
maybeStartTasks(); maybeStartTasks();
} }
return task.id; return task.id;
} }
private Task createTask(DownloadAction action) { private Task addTaskForAction(DownloadAction action) {
Task task = new Task(nextTaskId++, this, action, minRetryCount); Task task = new Task(nextTaskId++, this, action, minRetryCount);
tasks.add(task); tasks.add(task);
logd("Task is added", task); logd("Task is added", task);
notifyListenersTaskStateChange(task);
return task; return task;
} }
/** Returns the current number of tasks. */ /** Returns the current number of tasks. */
public int getTaskCount() { public int getTaskCount() {
Assertions.checkState(!released);
return tasks.size(); return tasks.size();
} }
/** Returns the state of a task, or null if no such task exists */ /** Returns the state of a task, or null if no such task exists */
public @Nullable TaskState getTaskState(int taskId) { public @Nullable TaskState getTaskState(int taskId) {
Assertions.checkState(!released);
for (int i = 0; i < tasks.size(); i++) { for (int i = 0; i < tasks.size(); i++) {
Task task = tasks.get(i); Task task = tasks.get(i);
if (task.id == taskId) { if (task.id == taskId) {
@ -294,6 +302,7 @@ public final class DownloadManager {
/** Returns the states of all current tasks. */ /** Returns the states of all current tasks. */
public TaskState[] getAllTaskStates() { public TaskState[] getAllTaskStates() {
Assertions.checkState(!released);
TaskState[] states = new TaskState[tasks.size()]; TaskState[] states = new TaskState[tasks.size()];
for (int i = 0; i < states.length; i++) { for (int i = 0; i < states.length; i++) {
states[i] = tasks.get(i).getDownloadState(); states[i] = tasks.get(i).getDownloadState();
@ -303,6 +312,7 @@ public final class DownloadManager {
/** Returns whether there are no active tasks. */ /** Returns whether there are no active tasks. */
public boolean isIdle() { public boolean isIdle() {
Assertions.checkState(!released);
if (!actionFileLoadCompleted) { if (!actionFileLoadCompleted) {
return false; return false;
} }
@ -327,7 +337,7 @@ public final class DownloadManager {
* If the task is a remove action then preceding conflicting tasks are canceled. * If the task is a remove action then preceding conflicting tasks are canceled.
*/ */
private void maybeStartTasks() { private void maybeStartTasks() {
if (released) { if (!actionFileLoadCompleted || released) {
return; return;
} }
@ -427,16 +437,26 @@ public final class DownloadManager {
new Runnable() { new Runnable() {
@Override @Override
public void run() { public void run() {
try { actionFileLoadCompleted = true;
if (released) {
return;
}
List<Task> pendingTasks = new ArrayList<>(tasks);
tasks.clear();
for (DownloadAction action : actions) { for (DownloadAction action : actions) {
createTask(action); Task task = addTaskForAction(action);
notifyListenersTaskStateChange(task);
}
if (!pendingTasks.isEmpty()) {
for (int i = 0; i < pendingTasks.size(); i++) {
Task pendingTask = pendingTasks.get(i);
tasks.add(pendingTask);
notifyListenersTaskStateChange(pendingTask);
}
saveActions();
} }
logd("Tasks are created."); logd("Tasks are created.");
maybeStartTasks(); maybeStartTasks();
} finally {
actionFileLoadCompleted = true;
maybeNotifyListenersIdle();
}
} }
}); });
} }
@ -444,7 +464,7 @@ public final class DownloadManager {
} }
private void saveActions() { private void saveActions() {
if (!actionFileLoadCompleted || released) { if (released) {
return; return;
} }
final DownloadAction[] actions = new DownloadAction[tasks.size()]; final DownloadAction[] actions = new DownloadAction[tasks.size()];