Use a single thread for DB access from the UI.

The UI may access the DB in response to UI or DB events; to maintain a
consistent view of the DB's contents, the tasks performing these
accesses must be prevented from overlapping, and must produce consistent
results if reordered. A single-threaded executor and latches are used to
prevent tasks from overlapping, without blocking non-UI access to the
DB.

briar-android/src/net/sf/briar/android/AndroidModule.java

@@ -1,7 +1,11 @@
 package net.sf.briar.android;
 
+import java.util.concurrent.Executor;
+import java.util.concurrent.Executors;
+
 import net.sf.briar.api.android.AndroidExecutor;
 import net.sf.briar.api.android.BundleEncrypter;
+import net.sf.briar.api.android.DatabaseUiExecutor;
 import net.sf.briar.api.android.ReferenceManager;
 
 import com.google.inject.AbstractModule;
@@ -16,5 +20,8 @@ public class AndroidModule extends AbstractModule {
 			Singleton.class);
 		bind(ReferenceManager.class).to(ReferenceManagerImpl.class).in(
 				Singleton.class);
+		// Use a single thread so DB accesses from the UI don't overlap
+		bind(Executor.class).annotatedWith(DatabaseUiExecutor.class).toInstance(
+				Executors.newSingleThreadExecutor());
 	}
 }

briar-android/src/net/sf/briar/android/groups/GroupActivity.java

@@ -6,6 +6,7 @@ import static java.util.logging.Level.INFO;
 import static java.util.logging.Level.WARNING;
 
 import java.util.Collection;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.Executor;
 import java.util.logging.Logger;
 
@@ -16,8 +17,8 @@ import net.sf.briar.android.BriarService;
 import net.sf.briar.android.BriarService.BriarServiceConnection;
 import net.sf.briar.android.widgets.CommonLayoutParams;
 import net.sf.briar.android.widgets.HorizontalBorder;
+import net.sf.briar.api.android.DatabaseUiExecutor;
 import net.sf.briar.api.db.DatabaseComponent;
-import net.sf.briar.api.db.DatabaseExecutor;
 import net.sf.briar.api.db.DbException;
 import net.sf.briar.api.db.GroupMessageHeader;
 import net.sf.briar.api.db.NoSuchSubscriptionException;
@@ -56,7 +57,7 @@ OnClickListener, OnItemClickListener {
 
 	// Fields that are accessed from DB threads must be volatile
 	@Inject private volatile DatabaseComponent db;
-	@Inject @DatabaseExecutor private volatile Executor dbExecutor;
+	@Inject @DatabaseUiExecutor private volatile Executor dbUiExecutor;
 	private volatile GroupId groupId = null;
 
 	@Override
@@ -107,7 +108,7 @@ OnClickListener, OnItemClickListener {
 	}
 
 	private void loadHeaders() {
-		dbExecutor.execute(new Runnable() {
+		dbUiExecutor.execute(new Runnable() {
 			public void run() {
 				try {
 					// Wait for the service to be bound and started
@@ -119,8 +120,10 @@ OnClickListener, OnItemClickListener {
 					long duration = System.currentTimeMillis() - now;
 					if(LOG.isLoggable(INFO))
 						LOG.info("Load took " + duration + " ms");
-					// Display the headers in the UI
+					// Wait for the headers to be displayed in the UI
-					displayHeaders(headers);
+					CountDownLatch latch = new CountDownLatch(1);
+					displayHeaders(latch, headers);
+					latch.await();
 				} catch(NoSuchSubscriptionException e) {
 					if(LOG.isLoggable(INFO)) LOG.info("Subscription removed");
 					finishOnUiThread();
@@ -129,20 +132,25 @@ OnClickListener, OnItemClickListener {
 						LOG.log(WARNING, e.toString(), e);
 				} catch(InterruptedException e) {
 					if(LOG.isLoggable(INFO))
-						LOG.info("Interrupted while waiting for service");
+						LOG.info("Interrupted while loading headers");
 					Thread.currentThread().interrupt();
 				}
 			}
 		});
 	}
 
-	private void displayHeaders(final Collection<GroupMessageHeader> headers) {
+	private void displayHeaders(final CountDownLatch latch,
+			final Collection<GroupMessageHeader> headers) {
 		runOnUiThread(new Runnable() {
 			public void run() {
-				adapter.clear();
+				try {
-				for(GroupMessageHeader h : headers) adapter.add(h);
+					adapter.clear();
-				adapter.sort(AscendingHeaderComparator.INSTANCE);
+					for(GroupMessageHeader h : headers) adapter.add(h);
-				selectFirstUnread();
+					adapter.sort(AscendingHeaderComparator.INSTANCE);
+					selectFirstUnread();
+				} finally {
+					latch.countDown();
+				}
 			}
 		});
 	}
@@ -184,11 +192,9 @@ OnClickListener, OnItemClickListener {
 		unbindService(serviceConnection);
 	}
 
-	// FIXME: Load operations may overlap, resulting in an inconsistent view
 	public void eventOccurred(DatabaseEvent e) {
 		if(e instanceof GroupMessageAddedEvent) {
-			GroupMessageAddedEvent g = (GroupMessageAddedEvent) e;
+			if(((GroupMessageAddedEvent) e).getGroupId().equals(groupId)) {
-			if(g.getMessage().getGroup().getId().equals(groupId)) {
 				if(LOG.isLoggable(INFO)) LOG.info("Message added, reloading");
 				loadHeaders();
 			}

briar-android/src/net/sf/briar/android/groups/GroupListActivity.java

@@ -16,6 +16,7 @@ import java.util.Arrays;
 import java.util.Collection;
 import java.util.Comparator;
 import java.util.List;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.Executor;
 import java.util.logging.Logger;
 
@@ -26,6 +27,7 @@ import net.sf.briar.android.BriarService.BriarServiceConnection;
 import net.sf.briar.android.widgets.CommonLayoutParams;
 import net.sf.briar.android.widgets.HorizontalBorder;
 import net.sf.briar.api.ContactId;
+import net.sf.briar.api.android.DatabaseUiExecutor;
 import net.sf.briar.api.crypto.CryptoComponent;
 import net.sf.briar.api.db.DatabaseComponent;
 import net.sf.briar.api.db.DatabaseExecutor;
@@ -70,6 +72,7 @@ implements OnClickListener, DatabaseListener {
 	@Inject private volatile CryptoComponent crypto;
 	@Inject private volatile DatabaseComponent db;
 	@Inject @DatabaseExecutor private volatile Executor dbExecutor;
+	@Inject @DatabaseUiExecutor private volatile Executor dbUiExecutor;
 	@Inject private volatile AuthorFactory authorFactory;
 	@Inject private volatile GroupFactory groupFactory;
 	@Inject private volatile MessageFactory messageFactory;
@@ -220,58 +223,68 @@ implements OnClickListener, DatabaseListener {
 	}
 
 	private void loadHeaders() {
-		dbExecutor.execute(new Runnable() {
+		dbUiExecutor.execute(new Runnable() {
 			public void run() {
 				try {
 					// Wait for the service to be bound and started
 					serviceConnection.waitForStartup();
 					// Load the subscribed groups from the DB
+					Collection<CountDownLatch> latches =
+							new ArrayList<CountDownLatch>();
+					long now = System.currentTimeMillis();
 					for(Group g : db.getSubscriptions()) {
 						// Filter out restricted groups
 						if(g.getPublicKey() != null) continue;
 						try {
-							long now = System.currentTimeMillis();
 							// Load the headers from the database
 							Collection<GroupMessageHeader> headers =
 									db.getMessageHeaders(g.getId());
-							long duration = System.currentTimeMillis() - now;
-							if(LOG.isLoggable(INFO))
-								LOG.info("Full load took " + duration + " ms");
 							// Display the headers in the UI
-							displayHeaders(g, headers);
+							CountDownLatch latch = new CountDownLatch(1);
+							displayHeaders(latch, g, headers);
+							latches.add(latch);
 						} catch(NoSuchSubscriptionException e) {
 							if(LOG.isLoggable(INFO))
 								LOG.info("Subscription removed");
 						}
 					}
+					long duration = System.currentTimeMillis() - now;
+					if(LOG.isLoggable(INFO))
+						LOG.info("Full load took " + duration + " ms");
+					// Wait for the headers to be displayed in the UI
+					for(CountDownLatch latch : latches) latch.await();
 				} catch(DbException e) {
 					if(LOG.isLoggable(WARNING))
 						LOG.log(WARNING, e.toString(), e);
 				} catch(InterruptedException e) {
 					if(LOG.isLoggable(INFO))
-						LOG.info("Interrupted while waiting for service");
+						LOG.info("Interrupted while loading headers");
 					Thread.currentThread().interrupt();
 				}
 			}
 		});
 	}
 
-	private void displayHeaders(final Group g,
+	private void displayHeaders(final CountDownLatch latch, final Group g,
 			final Collection<GroupMessageHeader> headers) {
 		runOnUiThread(new Runnable() {
 			public void run() {
-				// Remove the old item, if any
+				try {
-				GroupListItem item = findGroup(g.getId());
+					// Remove the old item, if any
-				if(item != null) adapter.remove(item);
+					GroupListItem item = findGroup(g.getId());
-				// Add a new item if there are any headers to display
+					if(item != null) adapter.remove(item);
-				if(!headers.isEmpty()) {
+					// Add a new item if there are any headers to display
-					List<GroupMessageHeader> headerList =
+					if(!headers.isEmpty()) {
-							new ArrayList<GroupMessageHeader>(headers);
+						List<GroupMessageHeader> headerList =
-					adapter.add(new GroupListItem(g, headerList));
+								new ArrayList<GroupMessageHeader>(headers);
-					adapter.sort(GroupComparator.INSTANCE);
+						adapter.add(new GroupListItem(g, headerList));
+						adapter.sort(GroupComparator.INSTANCE);
+					}
+					selectFirstUnread();
+				} finally {
+					latch.countDown();
 				}
-				selectFirstUnread();
+			} 
-			}
 		});
 	}
 
@@ -312,23 +325,22 @@ implements OnClickListener, DatabaseListener {
 		startActivity(new Intent(this, WriteGroupMessageActivity.class));
 	}
 
-	// FIXME: Load operations may overlap, resulting in an inconsistent view
 	public void eventOccurred(DatabaseEvent e) {
 		if(e instanceof GroupMessageAddedEvent) {
 			if(LOG.isLoggable(INFO)) LOG.info("Message added, reloading");
-			GroupMessageAddedEvent g = (GroupMessageAddedEvent) e;
+			loadHeaders(((GroupMessageAddedEvent) e).getGroupId());
-			loadHeaders(g.getMessage().getGroup().getId());
 		} else if(e instanceof MessageExpiredEvent) {
 			if(LOG.isLoggable(INFO)) LOG.info("Message expired, reloading");
 			loadHeaders(); // FIXME: Don't reload everything
 		} else if(e instanceof SubscriptionRemovedEvent) {
-			if(LOG.isLoggable(INFO)) LOG.info("Removing group");
+			// Reload the group, expecting NoSuchSubscriptionException
-			removeGroup(((SubscriptionRemovedEvent) e).getGroupId());
+			if(LOG.isLoggable(INFO)) LOG.info("Group removed, reloading");
+			loadHeaders(((SubscriptionRemovedEvent) e).getGroupId());
 		}
 	}
 
 	private void loadHeaders(final GroupId g) {
-		dbExecutor.execute(new Runnable() {
+		dbUiExecutor.execute(new Runnable() {
 			public void run() {
 				try {
 					serviceConnection.waitForStartup();
@@ -339,15 +351,18 @@ implements OnClickListener, DatabaseListener {
 					long duration = System.currentTimeMillis() - now;
 					if(LOG.isLoggable(INFO))
 						LOG.info("Partial load took " + duration + " ms");
-					displayHeaders(group, headers);
+					CountDownLatch latch = new CountDownLatch(1);
+					displayHeaders(latch, group, headers);
+					latch.await();
 				} catch(NoSuchSubscriptionException e) {
 					if(LOG.isLoggable(INFO)) LOG.info("Subscription removed");
+					removeGroup(g);
 				} catch(DbException e) {
 					if(LOG.isLoggable(WARNING))
 						LOG.log(WARNING, e.toString(), e);
 				} catch(InterruptedException e) {
 					if(LOG.isLoggable(INFO))
-						LOG.info("Interrupted while waiting for service");
+						LOG.info("Interrupted while loading headers");
 					Thread.currentThread().interrupt();
 				}
 			}

briar-android/src/net/sf/briar/android/messages/ConversationActivity.java

@@ -6,6 +6,7 @@ import static java.util.logging.Level.INFO;
 import static java.util.logging.Level.WARNING;
 
 import java.util.Collection;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.Executor;
 import java.util.logging.Logger;
 
@@ -17,8 +18,8 @@ import net.sf.briar.android.BriarService.BriarServiceConnection;
 import net.sf.briar.android.widgets.CommonLayoutParams;
 import net.sf.briar.android.widgets.HorizontalBorder;
 import net.sf.briar.api.ContactId;
+import net.sf.briar.api.android.DatabaseUiExecutor;
 import net.sf.briar.api.db.DatabaseComponent;
-import net.sf.briar.api.db.DatabaseExecutor;
 import net.sf.briar.api.db.DbException;
 import net.sf.briar.api.db.NoSuchContactException;
 import net.sf.briar.api.db.PrivateMessageHeader;
@@ -54,7 +55,7 @@ implements DatabaseListener, OnClickListener, OnItemClickListener {
 
 	// Fields that are accessed from DB threads must be volatile
 	@Inject private volatile DatabaseComponent db;
-	@Inject @DatabaseExecutor private volatile Executor dbExecutor;
+	@Inject @DatabaseUiExecutor private volatile Executor dbUiExecutor;
 	private volatile ContactId contactId = null;
 
 	@Override
@@ -105,7 +106,7 @@ implements DatabaseListener, OnClickListener, OnItemClickListener {
 	}
 
 	private void loadHeaders() {
-		dbExecutor.execute(new Runnable() {
+		dbUiExecutor.execute(new Runnable() {
 			public void run() {
 				try {
 					// Wait for the service to be bound and started
@@ -117,8 +118,10 @@ implements DatabaseListener, OnClickListener, OnItemClickListener {
 					long duration = System.currentTimeMillis() - now;
 					if(LOG.isLoggable(INFO))
 						LOG.info("Load took " + duration + " ms");
-					// Display the headers in the UI
+					// Wait for the headers to be displayed in the UI
-					displayHeaders(headers);
+					CountDownLatch latch = new CountDownLatch(1);
+					displayHeaders(latch, headers);
+					latch.await();
 				} catch(NoSuchContactException e) {
 					if(LOG.isLoggable(INFO)) LOG.info("Contact removed");
 					finishOnUiThread();
@@ -127,21 +130,25 @@ implements DatabaseListener, OnClickListener, OnItemClickListener {
 						LOG.log(WARNING, e.toString(), e);
 				} catch(InterruptedException e) {
 					if(LOG.isLoggable(INFO))
-						LOG.info("Interrupted while waiting for service");
+						LOG.info("Interrupted while loading headers");
 					Thread.currentThread().interrupt();
 				}
 			}
 		});
 	}
 
-	private void displayHeaders(
+	private void displayHeaders(final CountDownLatch latch,
 			final Collection<PrivateMessageHeader> headers) {
 		runOnUiThread(new Runnable() {
 			public void run() {
-				adapter.clear();
+				try {
-				for(PrivateMessageHeader h : headers) adapter.add(h);
+					adapter.clear();
-				adapter.sort(AscendingHeaderComparator.INSTANCE);
+					for(PrivateMessageHeader h : headers) adapter.add(h);
-				selectFirstUnread();
+					adapter.sort(AscendingHeaderComparator.INSTANCE);
+					selectFirstUnread();
+				} finally {
+					latch.countDown();
+				}
 			}
 		});
 	}
@@ -176,14 +183,13 @@ implements DatabaseListener, OnClickListener, OnItemClickListener {
 		super.onPause();
 		db.removeListener(this);
 	}
-	
+
 	@Override
 	public void onDestroy() {
 		super.onDestroy();
 		unbindService(serviceConnection);
 	}
 
-	// FIXME: Load operations may overlap, resulting in an inconsistent view
 	public void eventOccurred(DatabaseEvent e) {
 		if(e instanceof ContactRemovedEvent) {
 			ContactRemovedEvent c = (ContactRemovedEvent) e;

briar-android/src/net/sf/briar/android/messages/ConversationListActivity.java

@@ -11,6 +11,7 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Comparator;
 import java.util.List;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.Executor;
 import java.util.logging.Logger;
 
@@ -22,6 +23,7 @@ import net.sf.briar.android.widgets.CommonLayoutParams;
 import net.sf.briar.android.widgets.HorizontalBorder;
 import net.sf.briar.api.Contact;
 import net.sf.briar.api.ContactId;
+import net.sf.briar.api.android.DatabaseUiExecutor;
 import net.sf.briar.api.db.DatabaseComponent;
 import net.sf.briar.api.db.DatabaseExecutor;
 import net.sf.briar.api.db.DbException;
@@ -59,6 +61,7 @@ implements OnClickListener, DatabaseListener {
 	// Fields that are accessed from DB threads must be volatile
 	@Inject private volatile DatabaseComponent db;
 	@Inject @DatabaseExecutor private volatile Executor dbExecutor;
+	@Inject @DatabaseUiExecutor private volatile Executor dbUiExecutor;
 	@Inject private volatile MessageFactory messageFactory;
 
 	@Override
@@ -164,55 +167,65 @@ implements OnClickListener, DatabaseListener {
 	}
 
 	private void loadHeaders() {
-		dbExecutor.execute(new Runnable() {
+		dbUiExecutor.execute(new Runnable() {
 			public void run() {
 				try {
 					// Wait for the service to be bound and started
 					serviceConnection.waitForStartup();
 					// Load the contact list from the database
+					Collection<CountDownLatch> latches =
+							new ArrayList<CountDownLatch>();
+					long now = System.currentTimeMillis();
 					for(Contact c : db.getContacts()) {
 						try {
 							// Load the headers from the database
-							long now = System.currentTimeMillis();
 							Collection<PrivateMessageHeader> headers =
 									db.getPrivateMessageHeaders(c.getId());
-							long duration = System.currentTimeMillis() - now;
-							if(LOG.isLoggable(INFO))
-								LOG.info("Full load took " + duration + " ms");
 							// Display the headers in the UI
-							displayHeaders(c, headers);
+							CountDownLatch latch = new CountDownLatch(1);
+							displayHeaders(latch, c, headers);
+							latches.add(latch);
 						} catch(NoSuchContactException e) {
 							if(LOG.isLoggable(INFO))
 								LOG.info("Contact removed");
 						}
 					}
+					long duration = System.currentTimeMillis() - now;
+					if(LOG.isLoggable(INFO))
+						LOG.info("Full load took " + duration + " ms");
+					// Wait for the headers to be displayed in the UI
+					for(CountDownLatch latch : latches) latch.await();
 				} catch(DbException e) {
 					if(LOG.isLoggable(WARNING))
 						LOG.log(WARNING, e.toString(), e);
 				} catch(InterruptedException e) {
 					if(LOG.isLoggable(INFO))
-						LOG.info("Interrupted while waiting for service");
+						LOG.info("Interrupted while loading headers");
 					Thread.currentThread().interrupt();
 				}
 			}
 		});
 	}
 
-	private void displayHeaders(final Contact c,
+	private void displayHeaders(final CountDownLatch latch, final Contact c,
 			final Collection<PrivateMessageHeader> headers) {
 		runOnUiThread(new Runnable() {
 			public void run() {
-				// Remove the old item, if any
+				try {
-				ConversationListItem item = findConversation(c.getId());
+					// Remove the old item, if any
-				if(item != null) adapter.remove(item);
+					ConversationListItem item = findConversation(c.getId());
-				// Add a new item if there are any headers to display
+					if(item != null) adapter.remove(item);
-				if(!headers.isEmpty()) {
+					// Add a new item if there are any headers to display
-					List<PrivateMessageHeader> headerList =
+					if(!headers.isEmpty()) {
-							new ArrayList<PrivateMessageHeader>(headers);
+						List<PrivateMessageHeader> headerList =
-					adapter.add(new ConversationListItem(c, headerList));
+								new ArrayList<PrivateMessageHeader>(headers);
-					adapter.sort(ConversationComparator.INSTANCE);
+						adapter.add(new ConversationListItem(c, headerList));
+						adapter.sort(ConversationComparator.INSTANCE);
+					}
+					selectFirstUnread();
+				} finally {
+					latch.countDown();
 				}
-				selectFirstUnread();
 			}
 		});
 	}
@@ -243,7 +256,7 @@ implements OnClickListener, DatabaseListener {
 		super.onPause();
 		db.removeListener(this);
 	}
-	
+
 	@Override
 	public void onDestroy() {
 		super.onDestroy();
@@ -254,11 +267,11 @@ implements OnClickListener, DatabaseListener {
 		startActivity(new Intent(this, WritePrivateMessageActivity.class));
 	}
 
-	// FIXME: Load operations may overlap, resulting in an inconsistent view
 	public void eventOccurred(DatabaseEvent e) {
 		if(e instanceof ContactRemovedEvent) {
-			if(LOG.isLoggable(INFO)) LOG.info("Removing conversation");
+			// Reload the conversation, expecting NoSuchContactException
-			removeConversation(((ContactRemovedEvent) e).getContactId());
+			if(LOG.isLoggable(INFO)) LOG.info("Contact removed, reloading");
+			loadHeaders(((ContactRemovedEvent) e).getContactId());
 		} else if(e instanceof MessageExpiredEvent) {
 			if(LOG.isLoggable(INFO)) LOG.info("Message expired, reloading");
 			loadHeaders(); // FIXME: Don't reload everything
@@ -268,20 +281,8 @@ implements OnClickListener, DatabaseListener {
 		}
 	}
 
-	private void removeConversation(final ContactId c) {
-		runOnUiThread(new Runnable() {
-			public void run() {
-				ConversationListItem item = findConversation(c);
-				if(item != null) {
-					adapter.remove(item);
-					selectFirstUnread();
-				}
-			}
-		});
-	}
-
 	private void loadHeaders(final ContactId c) {
-		dbExecutor.execute(new Runnable() {
+		dbUiExecutor.execute(new Runnable() {
 			public void run() {
 				try {
 					serviceConnection.waitForStartup();
@@ -292,21 +293,36 @@ implements OnClickListener, DatabaseListener {
 					long duration = System.currentTimeMillis() - now;
 					if(LOG.isLoggable(INFO))
 						LOG.info("Partial load took " + duration + " ms");
-					displayHeaders(contact, headers);
+					CountDownLatch latch = new CountDownLatch(1);
+					displayHeaders(latch, contact, headers);
+					latch.await();
 				} catch(NoSuchContactException e) {
 					if(LOG.isLoggable(INFO)) LOG.info("Contact removed");
+					removeConversation(c);
 				} catch(DbException e) {
 					if(LOG.isLoggable(WARNING))
 						LOG.log(WARNING, e.toString(), e);
 				} catch(InterruptedException e) {
 					if(LOG.isLoggable(INFO))
-						LOG.info("Interrupted while waiting for service");
+						LOG.info("Interrupted while loading headers");
 					Thread.currentThread().interrupt();
 				}
 			}
 		});
 	}
 
+	private void removeConversation(final ContactId c) {
+		runOnUiThread(new Runnable() {
+			public void run() {
+				ConversationListItem item = findConversation(c);
+				if(item != null) {
+					adapter.remove(item);
+					selectFirstUnread();
+				}
+			}
+		});
+	}
+
 	private static class ConversationComparator
 	implements Comparator<ConversationListItem> {
 

briar-api/src/net/sf/briar/api/android/DatabaseUiExecutor.java

@@ -0,0 +1,18 @@
+package net.sf.briar.api.android;
+
+import static java.lang.annotation.ElementType.FIELD;
+import static java.lang.annotation.ElementType.PARAMETER;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+import com.google.inject.BindingAnnotation;
+
+/**
+ * Annotation for injecting the executor for accessing the database from the UI.
+ */
+@BindingAnnotation
+@Target({ FIELD, PARAMETER })
+@Retention(RUNTIME)
+public @interface DatabaseUiExecutor {}