From 844ae8f0a7b7e5f51d3dd468db622e3c33d323d9 Mon Sep 17 00:00:00 2001 From: akwizgran <akwizgran@users.sourceforge.net> Date: Thu, 8 Dec 2011 18:56:53 +0000 Subject: [PATCH] Plugins may dispose of resources differently depending on whether a connection was recognised. --- .../api/transport/BatchTransportReader.java | 8 +++--- .../api/transport/BatchTransportWriter.java | 7 ++--- .../transport/StreamTransportConnection.java | 8 +++--- .../BluetoothTransportConnection.java | 2 +- .../plugins/file/FileTransportReader.java | 4 +-- .../plugins/file/FileTransportWriter.java | 6 ++--- .../socket/SocketTransportConnection.java | 2 +- .../batch/IncomingBatchConnection.java | 4 +-- .../batch/OutgoingBatchConnection.java | 6 ++--- .../protocol/stream/StreamConnection.java | 26 +++++++++---------- .../transport/ConnectionDispatcherImpl.java | 12 ++++----- test/net/sf/briar/plugins/StreamTest.java | 8 +++--- .../file/RemovableDrivePluginTest.java | 8 +++--- .../socket/SimpleSocketPluginTest.java | 2 +- .../batch/BatchConnectionReadWriteTest.java | 7 +++-- .../batch/OutgoingBatchConnectionTest.java | 15 ++++++----- .../batch/TestBatchTransportReader.java | 24 ++++++++++++----- .../batch/TestBatchTransportWriter.java | 22 +++++++++------- 18 files changed, 97 insertions(+), 74 deletions(-) diff --git a/api/net/sf/briar/api/transport/BatchTransportReader.java b/api/net/sf/briar/api/transport/BatchTransportReader.java index 4faa004740..c2a42a2ea4 100644 --- a/api/net/sf/briar/api/transport/BatchTransportReader.java +++ b/api/net/sf/briar/api/transport/BatchTransportReader.java @@ -12,8 +12,10 @@ public interface BatchTransportReader { InputStream getInputStream(); /** - * Closes the reader and disposes of any associated state. The argument - * should be false if an exception was thrown while using the reader. + * Closes the reader and disposes of any associated resources. The first + * argument indicates whether the reader is being closed because of an + * exception and the second argument indicates whether the connection was + * recognised, which may affect how resources are disposed of. */ - void dispose(boolean success); + void dispose(boolean exception, boolean recognised); } diff --git a/api/net/sf/briar/api/transport/BatchTransportWriter.java b/api/net/sf/briar/api/transport/BatchTransportWriter.java index 92fcf38aab..4f2c6f0877 100644 --- a/api/net/sf/briar/api/transport/BatchTransportWriter.java +++ b/api/net/sf/briar/api/transport/BatchTransportWriter.java @@ -15,8 +15,9 @@ public interface BatchTransportWriter { OutputStream getOutputStream(); /** - * Closes the writer and disposes of any associated state. The argument - * should be false if an exception was thrown while using the writer. + * Closes the writer and disposes of any associated resources. The + * argument indicates whether the writer is being closed because of an + * exception, which may affect how resources are disposed of. */ - void dispose(boolean success); + void dispose(boolean exception); } diff --git a/api/net/sf/briar/api/transport/StreamTransportConnection.java b/api/net/sf/briar/api/transport/StreamTransportConnection.java index f6b28c1acd..337aae9300 100644 --- a/api/net/sf/briar/api/transport/StreamTransportConnection.java +++ b/api/net/sf/briar/api/transport/StreamTransportConnection.java @@ -18,8 +18,10 @@ public interface StreamTransportConnection { OutputStream getOutputStream() throws IOException; /** - * Closes the connection and disposes of any associated state. The argument - * should be false if an exception was thrown while using the connection. + * Closes the connection and disposes of any associated resources. The + * first argument indicates whether the connection is being closed because + * of an exception and the second argument indicates whether the connection + * was recognised, which may affect how resources are disposed of. */ - void dispose(boolean success); + void dispose(boolean exception, boolean recognised); } diff --git a/components/net/sf/briar/plugins/bluetooth/BluetoothTransportConnection.java b/components/net/sf/briar/plugins/bluetooth/BluetoothTransportConnection.java index 3ff9d853c8..9b04eb437a 100644 --- a/components/net/sf/briar/plugins/bluetooth/BluetoothTransportConnection.java +++ b/components/net/sf/briar/plugins/bluetooth/BluetoothTransportConnection.java @@ -29,7 +29,7 @@ class BluetoothTransportConnection implements StreamTransportConnection { return stream.openOutputStream(); } - public void dispose(boolean success) { + public void dispose(boolean exception, boolean recognised) { try { stream.close(); } catch(IOException e) { diff --git a/components/net/sf/briar/plugins/file/FileTransportReader.java b/components/net/sf/briar/plugins/file/FileTransportReader.java index 679fc86ba9..0f96e7134b 100644 --- a/components/net/sf/briar/plugins/file/FileTransportReader.java +++ b/components/net/sf/briar/plugins/file/FileTransportReader.java @@ -27,13 +27,13 @@ class FileTransportReader implements BatchTransportReader { return in; } - public void dispose(boolean success) { + public void dispose(boolean exception, boolean recognised) { try { in.close(); } catch(IOException e) { if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString()); } - if(success) { + if(recognised) { file.delete(); plugin.readerFinished(file); } diff --git a/components/net/sf/briar/plugins/file/FileTransportWriter.java b/components/net/sf/briar/plugins/file/FileTransportWriter.java index 2563e29629..fee646fc12 100644 --- a/components/net/sf/briar/plugins/file/FileTransportWriter.java +++ b/components/net/sf/briar/plugins/file/FileTransportWriter.java @@ -34,13 +34,13 @@ class FileTransportWriter implements BatchTransportWriter { return out; } - public void dispose(boolean success) { + public void dispose(boolean exception) { try { out.close(); } catch(IOException e) { if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString()); } - if(success) plugin.writerFinished(file); - else file.delete(); + if(exception) file.delete(); + else plugin.writerFinished(file); } } diff --git a/components/net/sf/briar/plugins/socket/SocketTransportConnection.java b/components/net/sf/briar/plugins/socket/SocketTransportConnection.java index 242e5e3f3e..f855f33100 100644 --- a/components/net/sf/briar/plugins/socket/SocketTransportConnection.java +++ b/components/net/sf/briar/plugins/socket/SocketTransportConnection.java @@ -28,7 +28,7 @@ class SocketTransportConnection implements StreamTransportConnection { return socket.getOutputStream(); } - public void dispose(boolean success) { + public void dispose(boolean exception, boolean recognised) { try { socket.close(); } catch(IOException e) { diff --git a/components/net/sf/briar/protocol/batch/IncomingBatchConnection.java b/components/net/sf/briar/protocol/batch/IncomingBatchConnection.java index dd28997d1e..e7f14c6054 100644 --- a/components/net/sf/briar/protocol/batch/IncomingBatchConnection.java +++ b/components/net/sf/briar/protocol/batch/IncomingBatchConnection.java @@ -79,10 +79,10 @@ class IncomingBatchConnection { throw new FormatException(); } } - transport.dispose(true); + transport.dispose(false, true); } catch(IOException e) { if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString()); - transport.dispose(false); + transport.dispose(true, true); } } diff --git a/components/net/sf/briar/protocol/batch/OutgoingBatchConnection.java b/components/net/sf/briar/protocol/batch/OutgoingBatchConnection.java index f58dc925ee..1e78871fb8 100644 --- a/components/net/sf/briar/protocol/batch/OutgoingBatchConnection.java +++ b/components/net/sf/briar/protocol/batch/OutgoingBatchConnection.java @@ -90,13 +90,13 @@ class OutgoingBatchConnection { } // Flush the output stream out.flush(); - transport.dispose(true); + transport.dispose(false); } catch(DbException e) { if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString()); - transport.dispose(false); + transport.dispose(true); } catch(IOException e) { if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString()); - transport.dispose(false); + transport.dispose(true); } } } diff --git a/components/net/sf/briar/protocol/stream/StreamConnection.java b/components/net/sf/briar/protocol/stream/StreamConnection.java index 5695ffca87..22486cdc06 100644 --- a/components/net/sf/briar/protocol/stream/StreamConnection.java +++ b/components/net/sf/briar/protocol/stream/StreamConnection.java @@ -166,13 +166,13 @@ abstract class StreamConnection implements DatabaseListener { } } writerTasks.add(CLOSE); - if(!disposed.getAndSet(true)) transport.dispose(true); + if(!disposed.getAndSet(true)) transport.dispose(false, true); } catch(DbException e) { if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString()); - if(!disposed.getAndSet(true)) transport.dispose(false); + if(!disposed.getAndSet(true)) transport.dispose(true, true); } catch(IOException e) { if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString()); - if(!disposed.getAndSet(true)) transport.dispose(false); + if(!disposed.getAndSet(true)) transport.dispose(true, true); } } @@ -203,17 +203,17 @@ abstract class StreamConnection implements DatabaseListener { if(task == CLOSE) break; task.run(); } - if(!disposed.getAndSet(true)) transport.dispose(true); + if(!disposed.getAndSet(true)) transport.dispose(false, true); } catch(DbException e) { if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString()); - if(!disposed.getAndSet(true)) transport.dispose(false); + if(!disposed.getAndSet(true)) transport.dispose(true, true); } catch(InterruptedException e) { if(LOG.isLoggable(Level.INFO)) LOG.info("Interrupted while waiting for task"); - if(!disposed.getAndSet(true)) transport.dispose(false); + if(!disposed.getAndSet(true)) transport.dispose(true, true); } catch(IOException e) { if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString()); - if(!disposed.getAndSet(true)) transport.dispose(false); + if(!disposed.getAndSet(true)) transport.dispose(true, true); } finally { db.removeListener(this); } @@ -308,7 +308,7 @@ abstract class StreamConnection implements DatabaseListener { writer.writeRequest(request); } catch(IOException e) { if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString()); - transport.dispose(false); + transport.dispose(true, true); } } } @@ -398,7 +398,7 @@ abstract class StreamConnection implements DatabaseListener { dbExecutor.execute(new GenerateAcks()); } catch(IOException e) { if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString()); - transport.dispose(false); + transport.dispose(true, true); } } } @@ -444,7 +444,7 @@ abstract class StreamConnection implements DatabaseListener { else dbExecutor.execute(new GenerateBatches(requested)); } catch(IOException e) { if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString()); - transport.dispose(false); + transport.dispose(true, true); } } } @@ -487,7 +487,7 @@ abstract class StreamConnection implements DatabaseListener { writer.writeOffer(offer); } catch(IOException e) { if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString()); - transport.dispose(false); + transport.dispose(true, true); } } } @@ -520,7 +520,7 @@ abstract class StreamConnection implements DatabaseListener { writer.writeSubscriptionUpdate(update); } catch(IOException e) { if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString()); - transport.dispose(false); + transport.dispose(true, true); } } } @@ -553,7 +553,7 @@ abstract class StreamConnection implements DatabaseListener { writer.writeTransportUpdate(update); } catch(IOException e) { if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString()); - transport.dispose(false); + transport.dispose(true, true); } } } diff --git a/components/net/sf/briar/transport/ConnectionDispatcherImpl.java b/components/net/sf/briar/transport/ConnectionDispatcherImpl.java index 3e4763cf56..b78c2a55c5 100644 --- a/components/net/sf/briar/transport/ConnectionDispatcherImpl.java +++ b/components/net/sf/briar/transport/ConnectionDispatcherImpl.java @@ -89,14 +89,14 @@ class ConnectionDispatcherImpl implements ConnectionDispatcher { try { byte[] tag = readTag(r.getInputStream()); ConnectionContext ctx = recogniser.acceptConnection(t, tag); - if(ctx == null) r.dispose(true); + if(ctx == null) r.dispose(false, false); else batchConnFactory.createIncomingConnection(ctx, r, tag); } catch(DbException e) { if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString()); - r.dispose(false); + r.dispose(true, false); } catch(IOException e) { if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString()); - r.dispose(false); + r.dispose(true, false); } } } @@ -116,14 +116,14 @@ class ConnectionDispatcherImpl implements ConnectionDispatcher { try { byte[] tag = readTag(s.getInputStream()); ConnectionContext ctx = recogniser.acceptConnection(t, tag); - if(ctx == null) s.dispose(true); + if(ctx == null) s.dispose(false, false); else streamConnFactory.createIncomingConnection(ctx, s, tag); } catch(DbException e) { if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString()); - s.dispose(false); + s.dispose(true, false); } catch(IOException e) { if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString()); - s.dispose(false); + s.dispose(true, false); } } } diff --git a/test/net/sf/briar/plugins/StreamTest.java b/test/net/sf/briar/plugins/StreamTest.java index abf4c5d342..04dd55a3f7 100644 --- a/test/net/sf/briar/plugins/StreamTest.java +++ b/test/net/sf/briar/plugins/StreamTest.java @@ -38,10 +38,10 @@ abstract class StreamTest { } else { System.out.println("No response"); } - s.dispose(true); + s.dispose(false, true); } catch(IOException e) { e.printStackTrace(); - s.dispose(false); + s.dispose(true, true); } } @@ -62,10 +62,10 @@ abstract class StreamTest { } else { System.out.println("No challenge"); } - s.dispose(true); + s.dispose(false, true); } catch(IOException e) { e.printStackTrace(); - s.dispose(false); + s.dispose(true, true); } } } diff --git a/test/net/sf/briar/plugins/file/RemovableDrivePluginTest.java b/test/net/sf/briar/plugins/file/RemovableDrivePluginTest.java index bb174a80b0..de91249727 100644 --- a/test/net/sf/briar/plugins/file/RemovableDrivePluginTest.java +++ b/test/net/sf/briar/plugins/file/RemovableDrivePluginTest.java @@ -283,11 +283,9 @@ public class RemovableDrivePluginTest extends TestCase { out.write(new byte[123]); out.flush(); out.close(); - assertEquals(123L, files[0].length()); - // Successfully disposing of the writer should not delete the file - writer.dispose(true); - files = drive1.listFiles(); - assertEquals(1, files.length); + // Disposing of the writer should not delete the file + writer.dispose(false); + assertTrue(files[0].exists()); assertEquals(123L, files[0].length()); context.assertIsSatisfied(); diff --git a/test/net/sf/briar/plugins/socket/SimpleSocketPluginTest.java b/test/net/sf/briar/plugins/socket/SimpleSocketPluginTest.java index 6cad2cdf6d..7d82bf984c 100644 --- a/test/net/sf/briar/plugins/socket/SimpleSocketPluginTest.java +++ b/test/net/sf/briar/plugins/socket/SimpleSocketPluginTest.java @@ -95,7 +95,7 @@ public class SimpleSocketPluginTest extends TestCase { assertTrue(latch.await(1, TimeUnit.SECONDS)); assertFalse(error.get()); // Clean up - conn.dispose(true); + conn.dispose(false, true); ss.close(); plugin.stop(); } diff --git a/test/net/sf/briar/protocol/batch/BatchConnectionReadWriteTest.java b/test/net/sf/briar/protocol/batch/BatchConnectionReadWriteTest.java index 0ae03f8fbc..2df1d1035b 100644 --- a/test/net/sf/briar/protocol/batch/BatchConnectionReadWriteTest.java +++ b/test/net/sf/briar/protocol/batch/BatchConnectionReadWriteTest.java @@ -118,7 +118,8 @@ public class BatchConnectionReadWriteTest extends TestCase { transport); // Write whatever needs to be written batchOut.write(); - assertTrue(transport.getSuccess()); + assertTrue(transport.getDisposed()); + assertFalse(transport.getException()); // Close Alice's database db.close(); // Return the contents of the batch connection @@ -172,7 +173,9 @@ public class BatchConnectionReadWriteTest extends TestCase { assertFalse(listener.messagesAdded); // Read whatever needs to be read batchIn.read(); - assertTrue(transport.getSuccess()); + assertTrue(transport.getDisposed()); + assertFalse(transport.getException()); + assertTrue(transport.getRecognised()); // The private message from Alice should have been added assertTrue(listener.messagesAdded); // Close Bob's database diff --git a/test/net/sf/briar/protocol/batch/OutgoingBatchConnectionTest.java b/test/net/sf/briar/protocol/batch/OutgoingBatchConnectionTest.java index 74bbc358e9..0e8759eaf0 100644 --- a/test/net/sf/briar/protocol/batch/OutgoingBatchConnectionTest.java +++ b/test/net/sf/briar/protocol/batch/OutgoingBatchConnectionTest.java @@ -87,8 +87,9 @@ public class OutgoingBatchConnectionTest extends TestCase { connection.write(); // Nothing should have been written assertEquals(0, out.size()); - // The transport should have been disposed with success == false - assertFalse(transport.getSuccess()); + // The transport should have been disposed with exception == true + assertTrue(transport.getDisposed()); + assertTrue(transport.getException()); context.assertIsSatisfied(); } @@ -122,8 +123,9 @@ public class OutgoingBatchConnectionTest extends TestCase { connection.write(); // Nothing should have been written assertEquals(0, out.size()); - // The transport should have been disposed with success == true - assertTrue(transport.getSuccess()); + // The transport should have been disposed with exception == false + assertTrue(transport.getDisposed()); + assertFalse(transport.getException()); context.assertIsSatisfied(); } @@ -171,8 +173,9 @@ public class OutgoingBatchConnectionTest extends TestCase { connection.write(); // Something should have been written assertTrue(out.size() > UniqueId.LENGTH + message.length); - // The transport should have been disposed with success == true - assertTrue(transport.getSuccess()); + // The transport should have been disposed with exception == false + assertTrue(transport.getDisposed()); + assertFalse(transport.getException()); context.assertIsSatisfied(); } } diff --git a/test/net/sf/briar/protocol/batch/TestBatchTransportReader.java b/test/net/sf/briar/protocol/batch/TestBatchTransportReader.java index 080339a783..c68a7e61b8 100644 --- a/test/net/sf/briar/protocol/batch/TestBatchTransportReader.java +++ b/test/net/sf/briar/protocol/batch/TestBatchTransportReader.java @@ -4,12 +4,11 @@ import java.io.InputStream; import net.sf.briar.api.transport.BatchTransportReader; -class TestBatchTransportReader -implements BatchTransportReader { +class TestBatchTransportReader implements BatchTransportReader { private final InputStream in; - private boolean success = false; + private boolean disposed = false, exception = false, recognised = false; TestBatchTransportReader(InputStream in) { this.in = in; @@ -19,11 +18,22 @@ implements BatchTransportReader { return in; } - public void dispose(boolean success) { - this.success = success; + public void dispose(boolean exception, boolean recognised) { + assert !disposed; + disposed = true; + this.exception = exception; + this.recognised = recognised; } - boolean getSuccess() { - return success; + boolean getDisposed() { + return disposed; + } + + boolean getException() { + return exception; + } + + boolean getRecognised() { + return recognised; } } \ No newline at end of file diff --git a/test/net/sf/briar/protocol/batch/TestBatchTransportWriter.java b/test/net/sf/briar/protocol/batch/TestBatchTransportWriter.java index 40da955372..80bafcb4f0 100644 --- a/test/net/sf/briar/protocol/batch/TestBatchTransportWriter.java +++ b/test/net/sf/briar/protocol/batch/TestBatchTransportWriter.java @@ -5,16 +5,14 @@ import java.io.OutputStream; import net.sf.briar.api.transport.BatchTransportWriter; -class TestBatchTransportWriter -implements BatchTransportWriter { +class TestBatchTransportWriter implements BatchTransportWriter { private final ByteArrayOutputStream out; private final long capacity; - private boolean success = false; + private boolean disposed = false, exception = false; - TestBatchTransportWriter(ByteArrayOutputStream out, - long capacity) { + TestBatchTransportWriter(ByteArrayOutputStream out, long capacity) { this.out = out; this.capacity = capacity; } @@ -27,11 +25,17 @@ implements BatchTransportWriter { return out; } - public void dispose(boolean success) { - this.success = success; + public void dispose(boolean exception) { + assert !disposed; + disposed = true; + this.exception = exception; } - boolean getSuccess() { - return success; + boolean getDisposed() { + return disposed; + } + + boolean getException() { + return exception; } } \ No newline at end of file -- GitLab