From 30ec2bd072f75a54b3d94b1bc191f71720ec4bc0 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sun, 25 Jan 2015 00:21:19 +0100 Subject: [PATCH] Cleanup XMPPTCPConnection, mostly exception handling In initConnection, only initReaderAndWriter() throws IOException. connectUsingConfiguration doesn't need to take an argument. PacketReader.init does not throw a SmackException. Use Async.go() in PacketWriter, just like it's already done in PacketReader. --- .../compression/XMPPInputOutputStream.java | 5 +- .../smack/tcp/XMPPTCPConnection.java | 98 +++++++------------ 2 files changed, 39 insertions(+), 64 deletions(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/compression/XMPPInputOutputStream.java b/smack-core/src/main/java/org/jivesoftware/smack/compression/XMPPInputOutputStream.java index 0092e9d01..be2528015 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/compression/XMPPInputOutputStream.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/compression/XMPPInputOutputStream.java @@ -16,6 +16,7 @@ */ package org.jivesoftware.smack.compression; +import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; @@ -47,9 +48,9 @@ public abstract class XMPPInputOutputStream { public abstract boolean isSupported(); - public abstract InputStream getInputStream(InputStream inputStream) throws Exception; + public abstract InputStream getInputStream(InputStream inputStream) throws IOException; - public abstract OutputStream getOutputStream(OutputStream outputStream) throws Exception; + public abstract OutputStream getOutputStream(OutputStream outputStream) throws IOException; public enum FlushMethod { FULL_FLUSH, diff --git a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java index 0998ecbb2..43570bef2 100644 --- a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java +++ b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java @@ -516,7 +516,7 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { } } - private void connectUsingConfiguration(XMPPTCPConnectionConfiguration config) throws SmackException, IOException { + private void connectUsingConfiguration() throws IOException, ConnectionException { populateHostAddresses(); List failedAddresses = new LinkedList(); @@ -574,70 +574,50 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { * @throws SmackException if the server failes to respond back or if there is anther error. * @throws IOException */ - private void initConnection() throws SmackException, IOException { + private void initConnection() throws IOException { boolean isFirstInitialization = packetReader == null || packetWriter == null; compressionHandler = null; // Set the reader and writer instance variables initReaderAndWriter(); - try { - if (isFirstInitialization) { - packetWriter = new PacketWriter(); - packetReader = new PacketReader(); + if (isFirstInitialization) { + packetWriter = new PacketWriter(); + packetReader = new PacketReader(); - // If debugging is enabled, we should start the thread that will listen for - // all packets and then log them. - if (config.isDebuggerEnabled()) { - addAsyncPacketListener(debugger.getReaderListener(), null); - if (debugger.getWriterListener() != null) { - addPacketSendingListener(debugger.getWriterListener(), null); - } + // If debugging is enabled, we should start the thread that will listen for + // all packets and then log them. + if (config.isDebuggerEnabled()) { + addAsyncPacketListener(debugger.getReaderListener(), null); + if (debugger.getWriterListener() != null) { + addPacketSendingListener(debugger.getWriterListener(), null); } } - // Start the packet writer. This will open a XMPP stream to the server - packetWriter.init(); - // Start the packet reader. The startup() method will block until we - // get an opening stream packet back from server - packetReader.init(); - - if (isFirstInitialization) { - // Notify listeners that a new connection has been established - for (ConnectionCreationListener listener : getConnectionCreationListeners()) { - listener.connectionCreated(this); - } - } - } - catch (SmackException ex) { - // An exception occurred in setting up the connection. Note that - // it's important here that we do an instant shutdown here, as this - // will not send a closing stream element, which will destroy - // Stream Management state on the server, which is not what we want. - instantShutdown(); - // Everything stopped. Now throw the exception. - throw ex; + // Start the packet writer. This will open a XMPP stream to the server + packetWriter.init(); + // Start the packet reader. The startup() method will block until we + // get an opening stream packet back from server + packetReader.init(); + + if (isFirstInitialization) { + // Notify listeners that a new connection has been established + for (ConnectionCreationListener listener : getConnectionCreationListeners()) { + listener.connectionCreated(this); + } } } - private void initReaderAndWriter() throws IOException, SmackException { - try { - InputStream is = socket.getInputStream(); - OutputStream os = socket.getOutputStream(); - if (compressionHandler != null) { - is = compressionHandler.getInputStream(is); - os = compressionHandler.getOutputStream(os); - } - // OutputStreamWriter is already buffered, no need to wrap it into a BufferedWriter - writer = new OutputStreamWriter(os, "UTF-8"); - reader = new BufferedReader(new InputStreamReader(is, "UTF-8")); - } - catch (IOException e) { - throw e; - } - catch (Exception e) { - throw new SmackException(e); + private void initReaderAndWriter() throws IOException { + InputStream is = socket.getInputStream(); + OutputStream os = socket.getOutputStream(); + if (compressionHandler != null) { + is = compressionHandler.getInputStream(is); + os = compressionHandler.getOutputStream(os); } + // OutputStreamWriter is already buffered, no need to wrap it into a BufferedWriter + writer = new OutputStreamWriter(os, "UTF-8"); + reader = new BufferedReader(new InputStreamReader(is, "UTF-8")); // If debugging is enabled, we open a window and write out all network traffic. initDebugger(); @@ -817,7 +797,7 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { protected void connectInternal() throws SmackException, IOException, XMPPException { throwAlreadyConnectedExceptionIfAppropriate(); // Establishes the connection, readers and writers - connectUsingConfiguration(config); + connectUsingConfiguration(); socketClosed = false; initConnection(); @@ -920,10 +900,8 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { /** * Initializes the reader in order to be used. The reader is initialized during the * first connection and when reconnecting due to an abruptly disconnection. - * - * @throws SmackException if the parser could not be reset. */ - void init() throws SmackException { + void init() { done = false; Async.go(new Runnable() { @@ -1148,8 +1126,6 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { private final ArrayBlockingQueueWithShutdown queue = new ArrayBlockingQueueWithShutdown( QUEUE_SIZE, true); - private Thread writerThread; - /** * Needs to be protected for unit testing purposes. */ @@ -1179,14 +1155,12 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { } queue.start(); - writerThread = new Thread() { + Async.go(new Runnable() { + @Override public void run() { writePackets(); } - }; - writerThread.setName("Smack Packet Writer (" + getConnectionCounter() + ")"); - writerThread.setDaemon(true); - writerThread.start(); + }, "Smack Packet Writer (" + getConnectionCounter() + ")"); } private boolean done() {