From ad2d3e70d633587d8e286e25f81e56cd3fcf6f9b Mon Sep 17 00:00:00 2001
From: Torsten Grote <t@grobox.de>
Date: Mon, 17 Jun 2019 13:22:38 -0300
Subject: [PATCH] [android] address thread-safety issues of attachment creation

---
 .../android/attachment/AttachmentCreator.java | 20 ++++++++++++++-----
 .../conversation/ConversationViewModel.java   |  7 +------
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java
index 41dd9ea3b..c22d46024 100644
--- a/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java
+++ b/briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentCreator.java
@@ -103,7 +103,7 @@ public class AttachmentCreator {
 			AttachmentItemResult itemResult =
 					new AttachmentItemResult(uri, item);
 			itemResults.add(itemResult);
-			result.postValue(new AttachmentResult(itemResults, false));
+			result.postValue(getResult(false));
 		} catch (IOException | DbException e) {
 			logException(LOG, WARNING, e);
 			onAttachmentError(uri, e);
@@ -127,15 +127,13 @@ public class AttachmentCreator {
 		AttachmentItemResult itemResult =
 				new AttachmentItemResult(uri, errorMsg);
 		itemResults.add(itemResult);
-		result.postValue(new AttachmentResult(itemResults, false));
+		result.postValue(getResult(false));
 		// expect to receive a cancel from the UI
 	}
 
 	@IoExecutor
 	void onAttachmentCreationFinished() {
-		if (uris.size() != itemResults.size())
-			throw new IllegalStateException();
-		result.postValue(new AttachmentResult(itemResults, true));
+		result.postValue(getResult(true));
 	}
 
 	@UiThread
@@ -205,4 +203,16 @@ public class AttachmentCreator {
 		});
 	}
 
+	private AttachmentResult getResult(boolean finished) {
+		// Make a copy of the list,
+		// because our copy will continue to change in the background.
+		// (As it's a CopyOnWriteArrayList,
+		//  the code that receives the result can safely do simple things
+		//  like iterating over the list,
+		//  but anything that involves calling more than one list method
+		//  is still unsafe.)
+		Collection<AttachmentItemResult> items = new ArrayList<>(itemResults);
+		return new AttachmentResult(items, finished);
+	}
+
 }
diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java
index 0c5fd7649..a1f4428b9 100644
--- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java
+++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java
@@ -6,7 +6,6 @@ import android.arch.lifecycle.LiveData;
 import android.arch.lifecycle.MutableLiveData;
 import android.arch.lifecycle.Transformations;
 import android.net.Uri;
-import android.os.Handler;
 import android.support.annotation.Nullable;
 import android.support.annotation.UiThread;
 
@@ -47,7 +46,6 @@ import java.util.logging.Logger;
 
 import javax.inject.Inject;
 
-import static android.os.Looper.getMainLooper;
 import static java.util.Objects.requireNonNull;
 import static java.util.logging.Level.WARNING;
 import static java.util.logging.Logger.getLogger;
@@ -294,6 +292,7 @@ public class ConversationViewModel extends AndroidViewModel
 
 	private void storeMessage(PrivateMessage m, @Nullable String text,
 			List<AttachmentHeader> attachments) {
+		attachmentCreator.onAttachmentsSent(m.getMessage().getId());
 		dbExecutor.execute(() -> {
 			try {
 				long start = now();
@@ -304,10 +303,6 @@ public class ConversationViewModel extends AndroidViewModel
 						message.getId(), message.getGroupId(),
 						message.getTimestamp(), true, true, false, false,
 						text != null, attachments);
-				MessageId id = m.getMessage().getId();
-				// use the UiThread to call onAttachmentsSent
-				new Handler(getMainLooper()).post(() ->
-						attachmentCreator.onAttachmentsSent(id));
 				// TODO add text to cache when available here
 				addedHeader.postEvent(h);
 			} catch (DbException e) {