From 7bf2ee64a8845d401bea813aa6bc109877efac89 Mon Sep 17 00:00:00 2001 From: akwizgran <akwizgran@users.sourceforge.net> Date: Tue, 29 Nov 2011 11:01:09 +0000 Subject: [PATCH] Use immutable collections for thread safety. --- .../net/sf/briar/db/DatabaseComponentImpl.java | 14 +++++++++----- .../net/sf/briar/i18n/FontManagerImpl.java | 3 ++- .../lifecycle/WindowsShutdownManagerImpl.java | 8 +++++--- .../net/sf/briar/plugins/PluginManagerImpl.java | 6 +++--- .../plugins/bluetooth/BluetoothPlugin.java | 3 ++- .../file/PollingRemovableDriveMonitor.java | 6 +++--- .../plugins/file/RemovableDriveFinder.java | 4 ++-- .../plugins/file/RemovableDrivePlugin.java | 8 +++++--- .../plugins/file/UnixRemovableDriveFinder.java | 3 ++- .../plugins/file/UnixRemovableDriveMonitor.java | 13 ++++++------- .../file/WindowsRemovableDriveFinder.java | 7 ++++--- components/net/sf/briar/serial/ReaderImpl.java | 12 +++++++----- components/net/sf/briar/serial/WriterImpl.java | 3 ++- .../briar/transport/ConnectionWindowImpl.java | 1 + .../transport/stream/StreamConnection.java | 17 +++++++++-------- 15 files changed, 62 insertions(+), 46 deletions(-) diff --git a/components/net/sf/briar/db/DatabaseComponentImpl.java b/components/net/sf/briar/db/DatabaseComponentImpl.java index 90e9a5e659..fff0e963fa 100644 --- a/components/net/sf/briar/db/DatabaseComponentImpl.java +++ b/components/net/sf/briar/db/DatabaseComponentImpl.java @@ -13,6 +13,7 @@ import java.util.Collection; import java.util.Collections; import java.util.HashSet; import java.util.Iterator; +import java.util.List; import java.util.Map; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.locks.ReentrantReadWriteLock; @@ -461,7 +462,7 @@ DatabaseCleaner.Callback { public boolean generateBatch(ContactId c, BatchWriter b) throws DbException, IOException { - Collection<MessageId> ids = new ArrayList<MessageId>(); + Collection<MessageId> ids; Collection<Bytes> messages = new ArrayList<Bytes>(); // Get some sendable messages from the database contactLock.readLock().lock(); @@ -593,7 +594,8 @@ DatabaseCleaner.Callback { public Collection<MessageId> generateOffer(ContactId c, OfferWriter o) throws DbException, IOException { - Collection<MessageId> sendable, sent = new ArrayList<MessageId>(); + Collection<MessageId> sendable; + List<MessageId> sent = new ArrayList<MessageId>(); contactLock.readLock().lock(); try { if(!containsContact(c)) throw new NoSuchContactException(); @@ -623,7 +625,7 @@ DatabaseCleaner.Callback { sent.add(m); } if(!sent.isEmpty()) o.finish(); - return sent; + return Collections.unmodifiableList(sent); } public void generateSubscriptionUpdate(ContactId c, @@ -1427,7 +1429,7 @@ DatabaseCleaner.Callback { public void setVisibility(GroupId g, Collection<ContactId> visible) throws DbException { - Collection<ContactId> affected; + List<ContactId> affected; contactLock.readLock().lock(); try { subscriptionLock.writeLock().lock(); @@ -1464,8 +1466,10 @@ DatabaseCleaner.Callback { contactLock.readLock().unlock(); } // Call the listeners outside the lock - if(!affected.isEmpty()) + if(!affected.isEmpty()) { + affected = Collections.unmodifiableList(affected); callListeners(new SubscriptionsUpdatedEvent(affected)); + } } public void subscribe(Group g) throws DbException { diff --git a/components/net/sf/briar/i18n/FontManagerImpl.java b/components/net/sf/briar/i18n/FontManagerImpl.java index 9ae1aa88dd..16b6ffccf0 100644 --- a/components/net/sf/briar/i18n/FontManagerImpl.java +++ b/components/net/sf/briar/i18n/FontManagerImpl.java @@ -35,7 +35,8 @@ public class FontManagerImpl implements FontManager { // Use Padauk for Burmese new BundledFont("Padauk.ttf", 14f, new String[] { "my" }), // Use DroidSansFallback for Chinese, Japanese and Korean - new BundledFont("DroidSansFallback.ttf", 12f, new String[] { "zh" , "ja", "ko" }), + new BundledFont("DroidSansFallback.ttf", 12f, + new String[] { "zh" , "ja", "ko" }) }; // Map from languages to fonts diff --git a/components/net/sf/briar/lifecycle/WindowsShutdownManagerImpl.java b/components/net/sf/briar/lifecycle/WindowsShutdownManagerImpl.java index b2057d8917..1949213209 100644 --- a/components/net/sf/briar/lifecycle/WindowsShutdownManagerImpl.java +++ b/components/net/sf/briar/lifecycle/WindowsShutdownManagerImpl.java @@ -1,5 +1,6 @@ package net.sf.briar.lifecycle; +import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; @@ -38,10 +39,11 @@ class WindowsShutdownManagerImpl extends ShutdownManagerImpl { WindowsShutdownManagerImpl() { // Use the Unicode versions of Win32 API calls - options = new HashMap<String, Object>(); - options.put(Library.OPTION_TYPE_MAPPER, W32APITypeMapper.UNICODE); - options.put(Library.OPTION_FUNCTION_MAPPER, + Map<String, Object> m = new HashMap<String, Object>(); + m.put(Library.OPTION_TYPE_MAPPER, W32APITypeMapper.UNICODE); + m.put(Library.OPTION_FUNCTION_MAPPER, W32APIFunctionMapper.UNICODE); + options = Collections.unmodifiableMap(m); } @Override diff --git a/components/net/sf/briar/plugins/PluginManagerImpl.java b/components/net/sf/briar/plugins/PluginManagerImpl.java index 3f6de4a0ff..391ce8b189 100644 --- a/components/net/sf/briar/plugins/PluginManagerImpl.java +++ b/components/net/sf/briar/plugins/PluginManagerImpl.java @@ -55,8 +55,8 @@ class PluginManagerImpl implements PluginManager { private final Poller poller; private final ConnectionDispatcher dispatcher; private final UiCallback uiCallback; - private final List<BatchPlugin> batchPlugins; - private final List<StreamPlugin> streamPlugins; + private final List<BatchPlugin> batchPlugins; // Locking: this + private final List<StreamPlugin> streamPlugins; // Locking: this @Inject PluginManagerImpl(DatabaseComponent db, Executor executor, Poller poller, @@ -156,7 +156,7 @@ class PluginManagerImpl implements PluginManager { List<Plugin> plugins = new ArrayList<Plugin>(); plugins.addAll(batchPlugins); plugins.addAll(streamPlugins); - poller.startPolling(plugins); + poller.startPolling(Collections.unmodifiableList(plugins)); // Return the number of plugins successfully started return batchPlugins.size() + streamPlugins.size(); } diff --git a/components/net/sf/briar/plugins/bluetooth/BluetoothPlugin.java b/components/net/sf/briar/plugins/bluetooth/BluetoothPlugin.java index e4d2b92633..ff1bccd6e0 100644 --- a/components/net/sf/briar/plugins/bluetooth/BluetoothPlugin.java +++ b/components/net/sf/briar/plugins/bluetooth/BluetoothPlugin.java @@ -241,7 +241,8 @@ class BluetoothPlugin extends AbstractPlugin implements StreamPlugin { } } ContactListener listener = new ContactListener(discoveryAgent, - addresses, uuids); + Collections.unmodifiableMap(addresses), + Collections.unmodifiableMap(uuids)); synchronized(discoveryLock) { try { discoveryAgent.startInquiry(DiscoveryAgent.GIAC, listener); diff --git a/components/net/sf/briar/plugins/file/PollingRemovableDriveMonitor.java b/components/net/sf/briar/plugins/file/PollingRemovableDriveMonitor.java index 0d582bc350..577449bdfa 100644 --- a/components/net/sf/briar/plugins/file/PollingRemovableDriveMonitor.java +++ b/components/net/sf/briar/plugins/file/PollingRemovableDriveMonitor.java @@ -2,7 +2,7 @@ package net.sf.briar.plugins.file; import java.io.File; import java.io.IOException; -import java.util.List; +import java.util.Collection; import java.util.logging.Level; import java.util.logging.Logger; @@ -47,7 +47,7 @@ class PollingRemovableDriveMonitor implements RemovableDriveMonitor, Runnable { public void run() { try { - List<File> drives = finder.findRemovableDrives(); + Collection<File> drives = finder.findRemovableDrives(); while(running) { synchronized(pollingLock) { try { @@ -58,7 +58,7 @@ class PollingRemovableDriveMonitor implements RemovableDriveMonitor, Runnable { } } if(!running) return; - List<File> newDrives = finder.findRemovableDrives(); + Collection<File> newDrives = finder.findRemovableDrives(); for(File f : newDrives) { if(!drives.contains(f)) callback.driveInserted(f); } diff --git a/components/net/sf/briar/plugins/file/RemovableDriveFinder.java b/components/net/sf/briar/plugins/file/RemovableDriveFinder.java index 46157a71c0..25bdd5c28e 100644 --- a/components/net/sf/briar/plugins/file/RemovableDriveFinder.java +++ b/components/net/sf/briar/plugins/file/RemovableDriveFinder.java @@ -2,9 +2,9 @@ package net.sf.briar.plugins.file; import java.io.File; import java.io.IOException; -import java.util.List; +import java.util.Collection; interface RemovableDriveFinder { - List<File> findRemovableDrives() throws IOException; + Collection<File> findRemovableDrives() throws IOException; } diff --git a/components/net/sf/briar/plugins/file/RemovableDrivePlugin.java b/components/net/sf/briar/plugins/file/RemovableDrivePlugin.java index af60b0ec87..3126e66e01 100644 --- a/components/net/sf/briar/plugins/file/RemovableDrivePlugin.java +++ b/components/net/sf/briar/plugins/file/RemovableDrivePlugin.java @@ -4,6 +4,7 @@ import java.io.File; import java.io.IOException; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.concurrent.Executor; import java.util.logging.Level; @@ -69,7 +70,8 @@ implements RemovableDriveMonitor.Callback { @Override protected File chooseOutputDirectory() { try { - List<File> drives = finder.findRemovableDrives(); + List<File> drives = + new ArrayList<File>(finder.findRemovableDrives()); if(drives.isEmpty()) return null; String[] paths = new String[drives.size()]; for(int i = 0; i < paths.length; i++) { @@ -96,7 +98,7 @@ implements RemovableDriveMonitor.Callback { @Override protected Collection<File> findFilesByName(String filename) { - Collection<File> matches = new ArrayList<File>(); + List<File> matches = new ArrayList<File>(); try { for(File drive : finder.findRemovableDrives()) { File[] files = drive.listFiles(); @@ -110,7 +112,7 @@ implements RemovableDriveMonitor.Callback { } catch(IOException e) { if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.getMessage()); } - return matches; + return Collections.unmodifiableList(matches); } public void driveInserted(File root) { diff --git a/components/net/sf/briar/plugins/file/UnixRemovableDriveFinder.java b/components/net/sf/briar/plugins/file/UnixRemovableDriveFinder.java index 0b5cac4014..4c636d0ba9 100644 --- a/components/net/sf/briar/plugins/file/UnixRemovableDriveFinder.java +++ b/components/net/sf/briar/plugins/file/UnixRemovableDriveFinder.java @@ -3,6 +3,7 @@ package net.sf.briar.plugins.file; import java.io.File; import java.io.IOException; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Scanner; @@ -34,6 +35,6 @@ abstract class UnixRemovableDriveFinder implements RemovableDriveFinder { } finally { s.close(); } - return drives; + return Collections.unmodifiableList(drives); } } diff --git a/components/net/sf/briar/plugins/file/UnixRemovableDriveMonitor.java b/components/net/sf/briar/plugins/file/UnixRemovableDriveMonitor.java index c4daf24709..18941b9a9a 100644 --- a/components/net/sf/briar/plugins/file/UnixRemovableDriveMonitor.java +++ b/components/net/sf/briar/plugins/file/UnixRemovableDriveMonitor.java @@ -11,10 +11,11 @@ import net.contentobjects.jnotify.JNotifyListener; abstract class UnixRemovableDriveMonitor implements RemovableDriveMonitor, JNotifyListener { + // Locking: this private final List<Integer> watches = new ArrayList<Integer>(); - private boolean started = false; - private Callback callback = null; + private boolean started = false; // Locking: this + private Callback callback = null; // Locking: this protected abstract String[] getPathsToWatch(); @@ -37,11 +38,9 @@ JNotifyListener { watches.clear(); } - public void fileCreated(int wd, String rootPath, String name) { - synchronized(this) { - if(!started) throw new IllegalStateException(); - callback.driveInserted(new File(rootPath + "/" + name)); - } + public synchronized void fileCreated(int wd, String rootPath, String name) { + if(!started) throw new IllegalStateException(); + callback.driveInserted(new File(rootPath + "/" + name)); } public void fileDeleted(int wd, String rootPath, String name) { diff --git a/components/net/sf/briar/plugins/file/WindowsRemovableDriveFinder.java b/components/net/sf/briar/plugins/file/WindowsRemovableDriveFinder.java index 78c2dfab14..7672cae66d 100644 --- a/components/net/sf/briar/plugins/file/WindowsRemovableDriveFinder.java +++ b/components/net/sf/briar/plugins/file/WindowsRemovableDriveFinder.java @@ -3,6 +3,8 @@ package net.sf.briar.plugins.file; import java.io.File; import java.io.IOException; import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; import java.util.List; import com.sun.jna.platform.win32.Kernel32; @@ -12,7 +14,7 @@ class WindowsRemovableDriveFinder implements RemovableDriveFinder { // http://msdn.microsoft.com/en-us/library/windows/desktop/aa364939.aspx private static final int DRIVE_REMOVABLE = 2; - public List<File> findRemovableDrives() throws IOException { + public Collection<File> findRemovableDrives() throws IOException { File[] roots = File.listRoots(); if(roots == null) throw new IOException(); List<File> drives = new ArrayList<File>(); @@ -24,7 +26,6 @@ class WindowsRemovableDriveFinder implements RemovableDriveFinder { throw new IOException(e.getMessage()); } } - return drives; + return Collections.unmodifiableList(drives); } - } diff --git a/components/net/sf/briar/serial/ReaderImpl.java b/components/net/sf/briar/serial/ReaderImpl.java index 90d5529b73..afb92a23ce 100644 --- a/components/net/sf/briar/serial/ReaderImpl.java +++ b/components/net/sf/briar/serial/ReaderImpl.java @@ -3,6 +3,7 @@ package net.sf.briar.serial; import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -14,12 +15,13 @@ import net.sf.briar.api.serial.Consumer; import net.sf.briar.api.serial.ObjectReader; import net.sf.briar.api.serial.Reader; +// This class is not thread-safe class ReaderImpl implements Reader { private static final byte[] EMPTY_BUFFER = new byte[] {}; private final InputStream in; - private final List<Consumer> consumers = new ArrayList<Consumer>(0); + private final Collection<Consumer> consumers = new ArrayList<Consumer>(0); private ObjectReader<?>[] objectReaders = new ObjectReader<?>[] {}; private boolean hasLookahead = false, eof = false; @@ -346,7 +348,7 @@ class ReaderImpl implements Reader { List<E> list = new ArrayList<E>(); while(!hasEnd()) list.add(readObject(e)); readEnd(); - return list; + return Collections.unmodifiableList(list); } else { int length = 0xFF & next ^ Tag.SHORT_LIST; return readList(e, length); @@ -358,7 +360,7 @@ class ReaderImpl implements Reader { if(length == 0) return Collections.emptyList(); List<E> list = new ArrayList<E>(); for(int i = 0; i < length; i++) list.add(readObject(e)); - return list; + return Collections.unmodifiableList(list); } private boolean hasEnd() throws IOException { @@ -478,7 +480,7 @@ class ReaderImpl implements Reader { throw new FormatException(); // Duplicate key } readEnd(); - return m; + return Collections.unmodifiableMap(m); } else { int size = 0xFF & next ^ Tag.SHORT_MAP; return readMap(k, v, size); @@ -494,7 +496,7 @@ class ReaderImpl implements Reader { if(m.put(readObject(k), readObject(v)) != null) throw new FormatException(); // Duplicate key } - return m; + return Collections.unmodifiableMap(m); } public boolean hasMapStart() throws IOException { diff --git a/components/net/sf/briar/serial/WriterImpl.java b/components/net/sf/briar/serial/WriterImpl.java index a212a13075..24f51ec9fb 100644 --- a/components/net/sf/briar/serial/WriterImpl.java +++ b/components/net/sf/briar/serial/WriterImpl.java @@ -12,10 +12,11 @@ import net.sf.briar.api.Bytes; import net.sf.briar.api.serial.Consumer; import net.sf.briar.api.serial.Writer; +// This class is not thread-safe class WriterImpl implements Writer { private final OutputStream out; - private final List<Consumer> consumers = new ArrayList<Consumer>(0); + private final Collection<Consumer> consumers = new ArrayList<Consumer>(0); WriterImpl(OutputStream out) { this.out = out; diff --git a/components/net/sf/briar/transport/ConnectionWindowImpl.java b/components/net/sf/briar/transport/ConnectionWindowImpl.java index adb76fa7b8..8aa1708bc9 100644 --- a/components/net/sf/briar/transport/ConnectionWindowImpl.java +++ b/components/net/sf/briar/transport/ConnectionWindowImpl.java @@ -10,6 +10,7 @@ import net.sf.briar.api.protocol.TransportIndex; import net.sf.briar.api.transport.ConnectionWindow; import net.sf.briar.util.ByteUtils; +// This class is not thread-safe class ConnectionWindowImpl implements ConnectionWindow { private final CryptoComponent crypto; diff --git a/components/net/sf/briar/transport/stream/StreamConnection.java b/components/net/sf/briar/transport/stream/StreamConnection.java index f24557cedf..18a2c918a9 100644 --- a/components/net/sf/briar/transport/stream/StreamConnection.java +++ b/components/net/sf/briar/transport/stream/StreamConnection.java @@ -6,7 +6,9 @@ import java.io.OutputStream; import java.util.ArrayList; import java.util.BitSet; import java.util.Collection; +import java.util.Collections; import java.util.LinkedList; +import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; @@ -58,11 +60,10 @@ abstract class StreamConnection implements DatabaseListener { protected final ContactId contactId; protected final StreamTransportConnection connection; - // These fields must only be accessed with this's lock held - private int writerFlags = 0; - private Collection<MessageId> offered = null; - private Collection<MessageId> requested = null; - private Offer incomingOffer = null; + private int writerFlags = 0; // Locking: this + private Collection<MessageId> offered = null; // Locking: this + private LinkedList<MessageId> requested = null; // Locking: this + private Offer incomingOffer = null; // Locking: this StreamConnection(ConnectionReaderFactory connReaderFactory, ConnectionWriterFactory connWriterFactory, DatabaseComponent db, @@ -143,15 +144,15 @@ abstract class StreamConnection implements DatabaseListener { } // Work out which messages were requested BitSet b = r.getBitmap(); - Collection<MessageId> req = new LinkedList<MessageId>(); - Collection<MessageId> seen = new ArrayList<MessageId>(); + LinkedList<MessageId> req = new LinkedList<MessageId>(); + List<MessageId> seen = new ArrayList<MessageId>(); int i = 0; for(MessageId m : off) { if(b.get(i++)) req.add(m); else seen.add(m); } // Mark the unrequested messages as seen - db.setSeen(contactId, seen); + db.setSeen(contactId, Collections.unmodifiableList(seen)); // Store the requested message IDs and notify the writer synchronized(this) { if(requested != null) -- GitLab