diff --git a/briar-android/src/net/sf/briar/android/AndroidModule.java b/briar-android/src/net/sf/briar/android/AndroidModule.java index 83ec34737014019708dcf2a0e0dcbeea57c9a106..3be4882c8c32bd8d34e16a88f40b25a3bf543fdf 100644 --- a/briar-android/src/net/sf/briar/android/AndroidModule.java +++ b/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()); } } diff --git a/briar-android/src/net/sf/briar/android/groups/GroupActivity.java b/briar-android/src/net/sf/briar/android/groups/GroupActivity.java index 59dbbca49f00d1830e647e4ce396b2f82b518971..deff171b85a52ce829bbc2effb32f5732001cc69 100644 --- a/briar-android/src/net/sf/briar/android/groups/GroupActivity.java +++ b/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 - displayHeaders(headers); + // Wait for the headers to be displayed in the UI + 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(); - for(GroupMessageHeader h : headers) adapter.add(h); - adapter.sort(AscendingHeaderComparator.INSTANCE); - selectFirstUnread(); + try { + adapter.clear(); + for(GroupMessageHeader h : headers) adapter.add(h); + 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(g.getMessage().getGroup().getId().equals(groupId)) { + if(((GroupMessageAddedEvent) e).getGroupId().equals(groupId)) { if(LOG.isLoggable(INFO)) LOG.info("Message added, reloading"); loadHeaders(); } diff --git a/briar-android/src/net/sf/briar/android/groups/GroupListActivity.java b/briar-android/src/net/sf/briar/android/groups/GroupListActivity.java index 6921e582fe5cebbbe66d1709b7b11d494ee9c7c8..f1885f61021b77860ade106310b6b5266c5e91c8 100644 --- a/briar-android/src/net/sf/briar/android/groups/GroupListActivity.java +++ b/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 - GroupListItem item = findGroup(g.getId()); - if(item != null) adapter.remove(item); - // Add a new item if there are any headers to display - if(!headers.isEmpty()) { - List<GroupMessageHeader> headerList = - new ArrayList<GroupMessageHeader>(headers); - adapter.add(new GroupListItem(g, headerList)); - adapter.sort(GroupComparator.INSTANCE); + try { + // Remove the old item, if any + GroupListItem item = findGroup(g.getId()); + if(item != null) adapter.remove(item); + // Add a new item if there are any headers to display + if(!headers.isEmpty()) { + List<GroupMessageHeader> headerList = + new ArrayList<GroupMessageHeader>(headers); + 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(g.getMessage().getGroup().getId()); + loadHeaders(((GroupMessageAddedEvent) e).getGroupId()); } 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"); - removeGroup(((SubscriptionRemovedEvent) e).getGroupId()); + // Reload the group, expecting NoSuchSubscriptionException + 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(); } } diff --git a/briar-android/src/net/sf/briar/android/messages/ConversationActivity.java b/briar-android/src/net/sf/briar/android/messages/ConversationActivity.java index 635eee594085948d4c528f387579759677672e77..5e2cbacc1d14adeb5f25e68d41ba36ad692a220a 100644 --- a/briar-android/src/net/sf/briar/android/messages/ConversationActivity.java +++ b/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 - displayHeaders(headers); + // Wait for the headers to be displayed in the UI + 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(); - for(PrivateMessageHeader h : headers) adapter.add(h); - adapter.sort(AscendingHeaderComparator.INSTANCE); - selectFirstUnread(); + try { + adapter.clear(); + for(PrivateMessageHeader h : headers) adapter.add(h); + 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; diff --git a/briar-android/src/net/sf/briar/android/messages/ConversationListActivity.java b/briar-android/src/net/sf/briar/android/messages/ConversationListActivity.java index a6f829e990d1dc990e27647b8c646d0cd5e12578..2078a521c526909089af404195678a16ae79455c 100644 --- a/briar-android/src/net/sf/briar/android/messages/ConversationListActivity.java +++ b/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 - ConversationListItem item = findConversation(c.getId()); - if(item != null) adapter.remove(item); - // Add a new item if there are any headers to display - if(!headers.isEmpty()) { - List<PrivateMessageHeader> headerList = - new ArrayList<PrivateMessageHeader>(headers); - adapter.add(new ConversationListItem(c, headerList)); - adapter.sort(ConversationComparator.INSTANCE); + try { + // Remove the old item, if any + ConversationListItem item = findConversation(c.getId()); + if(item != null) adapter.remove(item); + // Add a new item if there are any headers to display + if(!headers.isEmpty()) { + List<PrivateMessageHeader> headerList = + new ArrayList<PrivateMessageHeader>(headers); + 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"); - removeConversation(((ContactRemovedEvent) e).getContactId()); + // Reload the conversation, expecting NoSuchContactException + 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> { diff --git a/briar-api/src/net/sf/briar/api/android/DatabaseUiExecutor.java b/briar-api/src/net/sf/briar/api/android/DatabaseUiExecutor.java new file mode 100644 index 0000000000000000000000000000000000000000..1a28396e41d141867b045d3446667efe4d914d02 --- /dev/null +++ b/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 {}