From d1fedaed577718e4ff269584ab98aaaf0b3fe030 Mon Sep 17 00:00:00 2001 From: akwizgran <michael@briarproject.org> Date: Thu, 14 Feb 2013 13:04:51 +0000 Subject: [PATCH] Replaced InvitationManager with a generic ReferenceManager for Android. --- .../net/sf/briar/android/AndroidModule.java | 4 ++ .../briar/android/ReferenceManagerImpl.java | 61 +++++++++++++++++++ .../invitation/AddContactActivity.java | 51 ++++++++++++++-- .../briar/api/android/ReferenceManager.java | 30 +++++++++ .../api/invitation/InvitationManager.java | 22 ------- .../briar/api/invitation/InvitationTask.java | 3 - .../api/invitation/InvitationTaskFactory.java | 8 +++ .../sf/briar/invitation/ConnectorGroup.java | 22 ++----- .../invitation/InvitationManagerImpl.java | 58 ------------------ .../sf/briar/invitation/InvitationModule.java | 4 +- .../invitation/InvitationTaskFactoryImpl.java | 36 +++++++++++ 11 files changed, 191 insertions(+), 108 deletions(-) create mode 100644 briar-android/src/net/sf/briar/android/ReferenceManagerImpl.java create mode 100644 briar-api/src/net/sf/briar/api/android/ReferenceManager.java delete mode 100644 briar-api/src/net/sf/briar/api/invitation/InvitationManager.java create mode 100644 briar-api/src/net/sf/briar/api/invitation/InvitationTaskFactory.java delete mode 100644 briar-core/src/net/sf/briar/invitation/InvitationManagerImpl.java create mode 100644 briar-core/src/net/sf/briar/invitation/InvitationTaskFactoryImpl.java diff --git a/briar-android/src/net/sf/briar/android/AndroidModule.java b/briar-android/src/net/sf/briar/android/AndroidModule.java index d32534a969..29a745489a 100644 --- a/briar-android/src/net/sf/briar/android/AndroidModule.java +++ b/briar-android/src/net/sf/briar/android/AndroidModule.java @@ -1,13 +1,17 @@ package net.sf.briar.android; import net.sf.briar.api.android.AndroidExecutor; +import net.sf.briar.api.android.ReferenceManager; import com.google.inject.AbstractModule; +import com.google.inject.Singleton; public class AndroidModule extends AbstractModule { @Override protected void configure() { bind(AndroidExecutor.class).to(AndroidExecutorImpl.class); + bind(ReferenceManager.class).to(ReferenceManagerImpl.class).in( + Singleton.class); } } diff --git a/briar-android/src/net/sf/briar/android/ReferenceManagerImpl.java b/briar-android/src/net/sf/briar/android/ReferenceManagerImpl.java new file mode 100644 index 0000000000..3da1ce3be8 --- /dev/null +++ b/briar-android/src/net/sf/briar/android/ReferenceManagerImpl.java @@ -0,0 +1,61 @@ +package net.sf.briar.android; + +import static java.util.logging.Level.INFO; + +import java.util.HashMap; +import java.util.Map; +import java.util.logging.Logger; + +import net.sf.briar.api.android.ReferenceManager; + +// This class is not thread-safe. +class ReferenceManagerImpl implements ReferenceManager { + + private static final Logger LOG = + Logger.getLogger(ReferenceManagerImpl.class.getName()); + + private final Map<Class<?>, Map<Long, Object>> outerMap = + new HashMap<Class<?>, Map<Long, Object>>(); + + private long nextHandle = 0; + + public <T> T getReference(long handle, Class<T> c) { + Map<Long, Object> innerMap = outerMap.get(c); + if(innerMap == null) { + if(LOG.isLoggable(INFO)) + LOG.info("0 handles for " + c.getName()); + return null; + } + if(LOG.isLoggable(INFO)) + LOG.info(innerMap.size() + " handles for " + c.getName()); + Object o = innerMap.get(handle); + return c.cast(o); + } + + public <T> long putReference(T reference, Class<T> c) { + Map<Long, Object> innerMap = outerMap.get(c); + if(innerMap == null) { + innerMap = new HashMap<Long, Object>(); + outerMap.put(c, innerMap); + } + long handle = nextHandle++; + innerMap.put(handle, reference); + if(LOG.isLoggable(INFO)) { + LOG.info(innerMap.size() + " handles for " + c.getName() + + " after put"); + } + return handle; + } + + public <T> T removeReference(long handle, Class<T> c) { + Map<Long, Object> innerMap = outerMap.get(c); + if(innerMap == null) return null; + Object o = innerMap.remove(handle); + if(innerMap.isEmpty()) outerMap.remove(c); + if(LOG.isLoggable(INFO)) { + LOG.info(innerMap.size() + " handles for " + c.getName() + + " after remove"); + } + return c.cast(o); + } +} diff --git a/briar-android/src/net/sf/briar/android/invitation/AddContactActivity.java b/briar-android/src/net/sf/briar/android/invitation/AddContactActivity.java index 32398eeeac..a29c0763cf 100644 --- a/briar-android/src/net/sf/briar/android/invitation/AddContactActivity.java +++ b/briar-android/src/net/sf/briar/android/invitation/AddContactActivity.java @@ -1,10 +1,11 @@ package net.sf.briar.android.invitation; +import net.sf.briar.api.android.ReferenceManager; import net.sf.briar.api.crypto.CryptoComponent; import net.sf.briar.api.invitation.InvitationListener; -import net.sf.briar.api.invitation.InvitationManager; import net.sf.briar.api.invitation.InvitationState; import net.sf.briar.api.invitation.InvitationTask; +import net.sf.briar.api.invitation.InvitationTaskFactory; import roboguice.activity.RoboActivity; import android.os.Bundle; @@ -14,11 +15,13 @@ public class AddContactActivity extends RoboActivity implements InvitationListener { @Inject private CryptoComponent crypto; - @Inject private InvitationManager invitationManager; + @Inject private InvitationTaskFactory invitationTaskFactory; + @Inject private ReferenceManager referenceManager; // All of the following must be accessed on the UI thread private AddContactView view = null; private InvitationTask task = null; + private long taskHandle = -1; private String networkName = null; private boolean useBluetooth = false; private int localInvitationCode = -1, remoteInvitationCode = -1; @@ -37,8 +40,9 @@ implements InvitationListener { // Restore the activity's state networkName = state.getString("net.sf.briar.NETWORK_NAME"); useBluetooth = state.getBoolean("net.sf.briar.USE_BLUETOOTH"); - int handle = state.getInt("net.sf.briar.TASK_HANDLE", -1); - task = invitationManager.getTask(handle); + taskHandle = state.getLong("net.sf.briar.TASK_HANDLE", -1); + task = referenceManager.getReference(taskHandle, + InvitationTask.class); if(task == null) { // No background task - we must be in an initial or final state localInvitationCode = state.getInt("net.sf.briar.LOCAL_CODE"); @@ -110,7 +114,7 @@ implements InvitationListener { state.putBoolean("net.sf.briar.FAILED", connectionFailed); state.putBoolean("net.sf.briar.MATCHED", localMatched && remoteMatched); if(task != null) - state.putInt("net.sf.briar.TASK_HANDLE", task.getHandle()); + state.putLong("net.sf.briar.TASK_HANDLE", taskHandle); } @Override @@ -127,6 +131,7 @@ implements InvitationListener { void reset(AddContactView view) { task = null; + taskHandle = -1; networkName = null; useBluetooth = false; localInvitationCode = -1; @@ -162,8 +167,10 @@ implements InvitationListener { void remoteInvitationCodeEntered(int code) { setView(new ConnectionView(this)); // FIXME: These calls are blocking the UI thread for too long - task = invitationManager.createTask(localInvitationCode, code); + task = invitationTaskFactory.createTask(localInvitationCode, code); + taskHandle = referenceManager.putReference(task, InvitationTask.class); task.addListener(AddContactActivity.this); + task.addListener(new ReferenceCleaner(referenceManager, taskHandle)); task.connect(); } @@ -223,4 +230,36 @@ implements InvitationListener { } }); } + + /** + * Cleans up the reference to the invitation task when the task completes. + * This class is static to prevent memory leaks. + */ + private static class ReferenceCleaner implements InvitationListener { + + private final ReferenceManager referenceManager; + private final long handle; + + private ReferenceCleaner(ReferenceManager referenceManager, + long handle) { + this.referenceManager = referenceManager; + this.handle = handle; + } + + public void connectionSucceeded(int localCode, int remoteCode) { + // Wait for remote confirmation to succeed or fail + } + + public void connectionFailed() { + referenceManager.removeReference(handle, InvitationTask.class); + } + + public void remoteConfirmationSucceeded() { + referenceManager.removeReference(handle, InvitationTask.class); + } + + public void remoteConfirmationFailed() { + referenceManager.removeReference(handle, InvitationTask.class); + } + } } diff --git a/briar-api/src/net/sf/briar/api/android/ReferenceManager.java b/briar-api/src/net/sf/briar/api/android/ReferenceManager.java new file mode 100644 index 0000000000..21588adb05 --- /dev/null +++ b/briar-api/src/net/sf/briar/api/android/ReferenceManager.java @@ -0,0 +1,30 @@ +package net.sf.briar.api.android; + +/** + * Manages mappings between object references and serialisable handles. This + * enables references to be passed between Android UI objects that belong to + * the same process but can only communicate via serialisation. + * <p> + * This interface is designed to be accessed from the UI thread, so + * implementations may not be thread-safe. + */ +public interface ReferenceManager { + + /** + * Returns the object with the given handle, or null if no mapping exists + * for the handle. + */ + <T> T getReference(long handle, Class<T> c); + + /** + * Creates a mapping between the given reference and a handle, and returns + * the handle. + */ + <T> long putReference(T reference, Class<T> c); + + /** + * Removes and returns the object with the given handle, or returns null + * if no mapping exists for the handle. + */ + <T> T removeReference(long handle, Class<T> c); +} diff --git a/briar-api/src/net/sf/briar/api/invitation/InvitationManager.java b/briar-api/src/net/sf/briar/api/invitation/InvitationManager.java deleted file mode 100644 index 642b15fdb3..0000000000 --- a/briar-api/src/net/sf/briar/api/invitation/InvitationManager.java +++ /dev/null @@ -1,22 +0,0 @@ -package net.sf.briar.api.invitation; - -/** Creates and manages tasks for exchanging invitations with remote peers. */ -public interface InvitationManager { - - /** Creates a task using the given invitation codes. */ - InvitationTask createTask(int localCode, int remoteCode); - - /** - * Returns the previously created task with the given handle, unless the - * task has subsequently removed itself. - */ - InvitationTask getTask(int handle); - - /** Called by tasks to add themselves to the manager when they start. */ - void putTask(int handle, InvitationTask task); - - /** - * Called by tasks to remove themselves from the manager when they finish. - */ - void removeTask(int handle); -} diff --git a/briar-api/src/net/sf/briar/api/invitation/InvitationTask.java b/briar-api/src/net/sf/briar/api/invitation/InvitationTask.java index 43d39d7ffd..534582d33f 100644 --- a/briar-api/src/net/sf/briar/api/invitation/InvitationTask.java +++ b/briar-api/src/net/sf/briar/api/invitation/InvitationTask.java @@ -3,9 +3,6 @@ package net.sf.briar.api.invitation; /** A task for exchanging invitations with a remote peer. */ public interface InvitationTask { - /** Returns the task's unique handle. */ - int getHandle(); - /** * Adds a listener to be informed of state changes and returns the * task's current state. diff --git a/briar-api/src/net/sf/briar/api/invitation/InvitationTaskFactory.java b/briar-api/src/net/sf/briar/api/invitation/InvitationTaskFactory.java new file mode 100644 index 0000000000..5e61986ff5 --- /dev/null +++ b/briar-api/src/net/sf/briar/api/invitation/InvitationTaskFactory.java @@ -0,0 +1,8 @@ +package net.sf.briar.api.invitation; + +/** Creates tasks for exchanging invitations with remote peers. */ +public interface InvitationTaskFactory { + + /** Creates a task using the given invitation codes. */ + InvitationTask createTask(int localCode, int remoteCode); +} diff --git a/briar-core/src/net/sf/briar/invitation/ConnectorGroup.java b/briar-core/src/net/sf/briar/invitation/ConnectorGroup.java index 50f0865c1a..448fb34b97 100644 --- a/briar-core/src/net/sf/briar/invitation/ConnectorGroup.java +++ b/briar-core/src/net/sf/briar/invitation/ConnectorGroup.java @@ -14,7 +14,6 @@ import java.util.logging.Logger; import net.sf.briar.api.clock.Clock; import net.sf.briar.api.crypto.CryptoComponent; import net.sf.briar.api.invitation.InvitationListener; -import net.sf.briar.api.invitation.InvitationManager; import net.sf.briar.api.invitation.InvitationState; import net.sf.briar.api.invitation.InvitationTask; import net.sf.briar.api.plugins.PluginManager; @@ -28,13 +27,12 @@ class ConnectorGroup extends Thread implements InvitationTask { private static final Logger LOG = Logger.getLogger(ConnectorGroup.class.getName()); - private final InvitationManager invitationManager; private final CryptoComponent crypto; private final ReaderFactory readerFactory; private final WriterFactory writerFactory; private final Clock clock; private final PluginManager pluginManager; - private final int handle, localInvitationCode, remoteInvitationCode; + private final int localInvitationCode, remoteInvitationCode; private final Collection<InvitationListener> listeners; private final AtomicBoolean connected; private final CountDownLatch localConfirmationLatch; @@ -50,18 +48,16 @@ class ConnectorGroup extends Thread implements InvitationTask { private boolean localCompared = false, remoteCompared = false; private boolean localMatched = false, remoteMatched = false; - ConnectorGroup(InvitationManager invitationManager, CryptoComponent crypto, - ReaderFactory readerFactory, WriterFactory writerFactory, - Clock clock, PluginManager pluginManager, int handle, - int localInvitationCode, int remoteInvitationCode) { + ConnectorGroup(CryptoComponent crypto, ReaderFactory readerFactory, + WriterFactory writerFactory, Clock clock, + PluginManager pluginManager, int localInvitationCode, + int remoteInvitationCode) { super("ConnectorGroup"); - this.invitationManager = invitationManager; this.crypto = crypto; this.readerFactory = readerFactory; this.writerFactory = writerFactory; this.clock = clock; this.pluginManager = pluginManager; - this.handle = handle; this.localInvitationCode = localInvitationCode; this.remoteInvitationCode = remoteInvitationCode; listeners = new CopyOnWriteArrayList<InvitationListener>(); @@ -69,10 +65,6 @@ class ConnectorGroup extends Thread implements InvitationTask { localConfirmationLatch = new CountDownLatch(1); } - public int getHandle() { - return handle; - } - public synchronized InvitationState addListener(InvitationListener l) { listeners.add(l); return new InvitationState(localInvitationCode, remoteInvitationCode, @@ -90,8 +82,6 @@ class ConnectorGroup extends Thread implements InvitationTask { @Override public void run() { - // Add this task to the manager - invitationManager.putTask(handle, this); // Start the connection threads final Collection<Connector> connectors = new ArrayList<Connector>(); // Alice is the party with the smaller invitation code @@ -126,8 +116,6 @@ class ConnectorGroup extends Thread implements InvitationTask { } for(InvitationListener l : listeners) l.connectionFailed(); } - // Remove this task from the manager - invitationManager.removeTask(handle); } public void localConfirmationSucceeded() { diff --git a/briar-core/src/net/sf/briar/invitation/InvitationManagerImpl.java b/briar-core/src/net/sf/briar/invitation/InvitationManagerImpl.java deleted file mode 100644 index 3de8768c18..0000000000 --- a/briar-core/src/net/sf/briar/invitation/InvitationManagerImpl.java +++ /dev/null @@ -1,58 +0,0 @@ -package net.sf.briar.invitation; - -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.atomic.AtomicInteger; - -import net.sf.briar.api.clock.Clock; -import net.sf.briar.api.crypto.CryptoComponent; -import net.sf.briar.api.invitation.InvitationManager; -import net.sf.briar.api.invitation.InvitationTask; -import net.sf.briar.api.plugins.PluginManager; -import net.sf.briar.api.serial.ReaderFactory; -import net.sf.briar.api.serial.WriterFactory; - -import com.google.inject.Inject; - -class InvitationManagerImpl implements InvitationManager { - - private final CryptoComponent crypto; - private final ReaderFactory readerFactory; - private final WriterFactory writerFactory; - private final Clock clock; - private final PluginManager pluginManager; - - private final AtomicInteger nextHandle; - private final Map<Integer, InvitationTask> tasks; - - @Inject - InvitationManagerImpl(CryptoComponent crypto, ReaderFactory readerFactory, - WriterFactory writerFactory, Clock clock, - PluginManager pluginManager) { - this.crypto = crypto; - this.readerFactory = readerFactory; - this.writerFactory = writerFactory; - this.clock = clock; - this.pluginManager = pluginManager; - nextHandle = new AtomicInteger(0); - tasks = new ConcurrentHashMap<Integer, InvitationTask>(); - } - - public InvitationTask createTask(int localCode, int remoteCode) { - int handle = nextHandle.incrementAndGet(); - return new ConnectorGroup(this, crypto, readerFactory, writerFactory, - clock, pluginManager, handle, localCode, remoteCode); - } - - public InvitationTask getTask(int handle) { - return tasks.get(handle); - } - - public void putTask(int handle, InvitationTask task) { - tasks.put(handle, task); - } - - public void removeTask(int handle) { - tasks.remove(handle); - } -} diff --git a/briar-core/src/net/sf/briar/invitation/InvitationModule.java b/briar-core/src/net/sf/briar/invitation/InvitationModule.java index 89ae4ea49d..b2e476572f 100644 --- a/briar-core/src/net/sf/briar/invitation/InvitationModule.java +++ b/briar-core/src/net/sf/briar/invitation/InvitationModule.java @@ -1,6 +1,6 @@ package net.sf.briar.invitation; -import net.sf.briar.api.invitation.InvitationManager; +import net.sf.briar.api.invitation.InvitationTaskFactory; import com.google.inject.AbstractModule; import com.google.inject.Singleton; @@ -9,7 +9,7 @@ public class InvitationModule extends AbstractModule { @Override protected void configure() { - bind(InvitationManager.class).to(InvitationManagerImpl.class).in( + bind(InvitationTaskFactory.class).to(InvitationTaskFactoryImpl.class).in( Singleton.class); } } diff --git a/briar-core/src/net/sf/briar/invitation/InvitationTaskFactoryImpl.java b/briar-core/src/net/sf/briar/invitation/InvitationTaskFactoryImpl.java new file mode 100644 index 0000000000..2debc5e7eb --- /dev/null +++ b/briar-core/src/net/sf/briar/invitation/InvitationTaskFactoryImpl.java @@ -0,0 +1,36 @@ +package net.sf.briar.invitation; + +import net.sf.briar.api.clock.Clock; +import net.sf.briar.api.crypto.CryptoComponent; +import net.sf.briar.api.invitation.InvitationTask; +import net.sf.briar.api.invitation.InvitationTaskFactory; +import net.sf.briar.api.plugins.PluginManager; +import net.sf.briar.api.serial.ReaderFactory; +import net.sf.briar.api.serial.WriterFactory; + +import com.google.inject.Inject; + +class InvitationTaskFactoryImpl implements InvitationTaskFactory { + + private final CryptoComponent crypto; + private final ReaderFactory readerFactory; + private final WriterFactory writerFactory; + private final Clock clock; + private final PluginManager pluginManager; + + @Inject + InvitationTaskFactoryImpl(CryptoComponent crypto, + ReaderFactory readerFactory, WriterFactory writerFactory, + Clock clock, PluginManager pluginManager) { + this.crypto = crypto; + this.readerFactory = readerFactory; + this.writerFactory = writerFactory; + this.clock = clock; + this.pluginManager = pluginManager; + } + + public InvitationTask createTask(int localCode, int remoteCode) { + return new ConnectorGroup(crypto, readerFactory, writerFactory, clock, + pluginManager, localCode, remoteCode); + } +} -- GitLab