From d17f64ed9ae5de39aeb797f21954ac9dd19b973e Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sun, 27 Apr 2014 11:39:38 +0200 Subject: [PATCH] Improve stream compression for TCP connections Fix a race condition in useCompression where the compression request was send outside the synchronized block, this could cause notify() to be called without a previoius call to wait(). s/streamCompressionDenied/streamCompressionNegotiationDone/ and re-use the method once the server ack'd stream compression. Also don't call notifyConnectionError() when requestStreamCompression() encounters an exception, instead throw the exception. --- .../org/jivesoftware/smack/PacketReader.java | 2 +- .../jivesoftware/smack/XMPPTCPConnection.java | 35 ++++++++----------- 2 files changed, 16 insertions(+), 21 deletions(-) diff --git a/tcp/src/main/java/org/jivesoftware/smack/PacketReader.java b/tcp/src/main/java/org/jivesoftware/smack/PacketReader.java index 3b57754e4..648041f60 100644 --- a/tcp/src/main/java/org/jivesoftware/smack/PacketReader.java +++ b/tcp/src/main/java/org/jivesoftware/smack/PacketReader.java @@ -231,7 +231,7 @@ class PacketReader { // Stream compression has been denied. This is a recoverable // situation. It is still possible to authenticate and // use the connection but using an uncompressed connection - connection.streamCompressionDenied(); + connection.streamCompressionNegotiationDone(); } else { // SASL authentication has failed. The server may close the connection diff --git a/tcp/src/main/java/org/jivesoftware/smack/XMPPTCPConnection.java b/tcp/src/main/java/org/jivesoftware/smack/XMPPTCPConnection.java index 91d8e3e63..dcad1efa3 100644 --- a/tcp/src/main/java/org/jivesoftware/smack/XMPPTCPConnection.java +++ b/tcp/src/main/java/org/jivesoftware/smack/XMPPTCPConnection.java @@ -776,8 +776,9 @@ public class XMPPTCPConnection extends XMPPConnection { *

* * @return true if stream compression negotiation was successful. + * @throws IOException if the compress stanza could not be send */ - private boolean useCompression() { + private boolean useCompression() throws IOException { // If stream compression was offered by the server and we want to use // compression then send compression request to the server if (authenticated) { @@ -785,9 +786,9 @@ public class XMPPTCPConnection extends XMPPConnection { } if ((compressionHandler = maybeGetCompressionHandler()) != null) { - requestStreamCompression(compressionHandler.getCompressionMethod()); - // Wait until compression is being used or a timeout happened synchronized (this) { + requestStreamCompression(compressionHandler.getCompressionMethod()); + // Wait until compression is being used or a timeout happened try { wait(getPacketReplyTimeout()); } @@ -804,24 +805,20 @@ public class XMPPTCPConnection extends XMPPConnection { * Request the server that we want to start using stream compression. When using TLS * then negotiation of stream compression can only happen after TLS was negotiated. If TLS * compression is being used the stream compression should not be used. + * @throws IOException if the compress stanza could not be send */ - private void requestStreamCompression(String method) { - try { - writer.write(""); - writer.write("" + method + ""); - writer.flush(); - } - catch (IOException e) { - notifyConnectionError(e); - } + private void requestStreamCompression(String method) throws IOException { + writer.write(""); + writer.write("" + method + ""); + writer.flush(); } /** * Start using stream compression since the server has acknowledged stream compression. * - * @throws Exception if there is an exception starting stream compression. + * @throws IOException if there is an exception starting stream compression. */ - void startStreamCompression() throws Exception { + void startStreamCompression() throws IOException { serverAckdCompression = true; // Initialize the reader and writer with the new secured version initReaderAndWriter(); @@ -831,16 +828,14 @@ public class XMPPTCPConnection extends XMPPConnection { // Send a new opening stream to the server packetWriter.openStream(); // Notify that compression is being used - synchronized (this) { - this.notify(); - } + streamCompressionNegotiationDone(); } /** - * Notifies the XMPP connection that stream compression was denied so that - * the connection process can proceed. + * Notifies the XMPP connection that stream compression negotiation is done so that the + * connection process can proceed. */ - synchronized void streamCompressionDenied() { + synchronized void streamCompressionNegotiationDone() { this.notify(); }