From 525ee09ea15d7843e20d2d376f343cd5fe8ab76a Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Thu, 17 Sep 2020 23:04:55 +0200 Subject: [PATCH] [tcp] Do not send SM ack after we send a Do net put an ack to the queue if it has already been shutdown. Some servers, like ejabberd, like to request an ack even after we have send a stream close (and hance the queue was shutdown). If we would not check here, then the ack would dangle around in the queue, and be send on the next re-connection attempt even before the stream open. See the following trace of the MUC bookmarks integration test. The fact that it is a MUC test does not matter, but this test does disconnect the connection and reconnect it. Not how the server, ejabberd in this case, requests an SM ack by sending an even though we already send the : 22:22:05 SENT (4): Nick-P2VXD7 22:22:05 RECV (4): 22:22:05 SENT (4): 22:22:05 RECV (4): Nick-P2VXD7
22:22:05 RECV (4): 22:22:05 SENT (4): 22:22:05 RECV (4): 22:22:05 RECV (4): 22:22:05 XMPPConnection closed (XMPPTCPConnection[sinttest-7in7j-4@salem.geekplace.eu/1415073683806426185213090] (4)) 22:22:05 SENT (4): 22:22:05 SENT (4): 22:22:05 RECV (4): ?xml version='1.0'?> 22:22:05 XMPPConnection closed due to an exception (XMPPTCPConnection[sinttest-7in7j-4@salem.geekplace.eu/1415073683806426185213090] (4)) org.jivesoftware.smack.XMPPException$StreamErrorException: invalid-xml You can read more about the meaning of this stream error at http://xmpp.org/rfcs/rfc6120.html#streams-error-conditions at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader.parsePackets(XMPPTCPConnection.java:981) at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader.access$700(XMPPTCPConnection.java:913) at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader$1.run(XMPPTCPConnection.java:936) at java.base/java.lang.Thread.run(Thread.java:834) --- .../util/ArrayBlockingQueueWithShutdown.java | 24 +++++++++++++++++++ .../smack/tcp/XMPPTCPConnection.java | 7 +++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/util/ArrayBlockingQueueWithShutdown.java b/smack-core/src/main/java/org/jivesoftware/smack/util/ArrayBlockingQueueWithShutdown.java index 01938c43c..da61c4724 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/util/ArrayBlockingQueueWithShutdown.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/util/ArrayBlockingQueueWithShutdown.java @@ -293,6 +293,30 @@ public class ArrayBlockingQueueWithShutdown extends AbstractQueue implemen } } + /** + * Put if the queue has not been shutdown yet. + * + * @param e the element to put into the queue. + * @return true if the element has been put into the queue, false if the queue was shutdown. + * @throws InterruptedException if the calling thread was interrupted. + * @since 4.4 + */ + public boolean putIfNotShutdown(E e) throws InterruptedException { + checkNotNull(e); + lock.lockInterruptibly(); + + try { + if (isShutdown) { + return false; + } + + putInternal(e, true); + return true; + } finally { + lock.unlock(); + } + } + public void putAll(Collection elements) throws InterruptedException { checkNotNull(elements); lock.lockInterruptibly(); 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 2d283d802..aeb9d109b 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 @@ -1592,7 +1592,12 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { } private void sendSmAcknowledgementInternal() throws NotConnectedException, InterruptedException { - packetWriter.sendStreamElement(new AckAnswer(clientHandledStanzasCount)); + AckAnswer ackAnswer = new AckAnswer(clientHandledStanzasCount); + // Do net put an ack to the queue if it has already been shutdown. Some servers, like ejabberd, like to request + // an ack even after we have send a stream close (and hance the queue was shutdown). If we would not check here, + // then the ack would dangle around in the queue, and be send on the next re-connection attempt even before the + // stream open. + packetWriter.queue.putIfNotShutdown(ackAnswer); } /**