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.
This commit is contained in:
Florian Schmaus 2014-04-27 11:39:38 +02:00
parent 9b235d0d8f
commit d17f64ed9a
2 changed files with 16 additions and 21 deletions

View File

@ -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

View File

@ -776,8 +776,9 @@ public class XMPPTCPConnection extends XMPPConnection {
* <p>
*
* @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("<compress xmlns='http://jabber.org/protocol/compress'>");
writer.write("<method>" + method + "</method></compress>");
writer.flush();
}
catch (IOException e) {
notifyConnectionError(e);
}
private void requestStreamCompression(String method) throws IOException {
writer.write("<compress xmlns='http://jabber.org/protocol/compress'>");
writer.write("<method>" + method + "</method></compress>");
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();
}