This is based on the patch provided by Georg Lukas, modified to take
XMPP domain names that can not be represented as DNS names into
account. See c42d2eea24 (r269250137)
Fixes SMACK-870.
In case an exception happens in connect()/login() the
'disconnectedButResumable' boolean may (still) be set. Which causes
only one of the reader and writer threads to exit, typically the
reader thread, because shutdown() will bail out very early. This
leaves a dangling (writer) thread causing memory leaks and deadlocks
on a subsequent connect()/login().
as it is not neccessary and causes stalls and deadlocks with a
concurrent connect()/login() (which could be resolved interrupting the
connect()/login() calling thread):
"Smack Reader (0)" daemon prio=5 tid=21 Blocked
| group="main" sCount=1 dsCount=0 obj=0x12c84670 self=0x7e072ca200
| sysTid=14965 nice=0 cgrp=default sched=0/0 handle=0x7e06155450
| state=S schedstat=( 63430626 3034269 21 ) utm=5 stm=0 core=2 HZ=100
| stack=0x7e06053000-0x7e06055000 stackSize=1037KB
| held mutexes=
at org.jivesoftware.smack.tcp.XMPPTCPConnection.notifyConnectionError(XMPPTCPConnection.java:-1)
- waiting to lock <0x0ec6e6bb> (a org.jivesoftware.smack.tcp.XMPPTCPConnection) held by thread 22
at org.jivesoftware.smack.tcp.XMPPTCPConnection.access$3900(XMPPTCPConnection.java:154)
at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader.parsePackets(XMPPTCPConnection.java:1330)
at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader.access$900(XMPPTCPConnection.java:1064)
at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader$1.run(XMPPTCPConnection.java:1081)
at java.lang.Thread.run(Thread.java:761)
"connect" prio=5 tid=22 Waiting
| group="main" sCount=1 dsCount=0 obj=0x12c8e820 self=0x7e19bcd600
| sysTid=15047 nice=0 cgrp=default sched=0/0 handle=0x7e05ee4450
| state=S schedstat=( 14067083 7475835 14 ) utm=1 stm=0 core=0 HZ=100
| stack=0x7e05de2000-0x7e05de4000 stackSize=1037KB
| held mutexes=
at java.lang.Object.wait!(Native method)
- waiting on <0x0c058b14> (a java.lang.Object)
at java.lang.Thread.parkFor$(Thread.java:2127)
- locked <0x0c058b14> (a java.lang.Object)
at sun.misc.Unsafe.park(Unsafe.java:325)
at java.util.concurrent.locks.LockSupport.park(LockSupport.java:161)
at java.util.concurrent.locks.AbstractQueuedSynchronizer.parkAndCheckInterrupt(AbstractQueuedSynchronizer.java:840)
at java.util.concurrent.locks.AbstractQueuedSynchronizer.doAcquireSharedInterruptibly(AbstractQueuedSynchronizer.java:994)
at java.util.concurrent.locks.AbstractQueuedSynchronizer.acquireSharedInterruptibly(AbstractQueuedSynchronizer.java:1303)
at java.util.concurrent.Semaphore.acquire(Semaphore.java:446)
at org.jivesoftware.smack.tcp.XMPPTCPConnection.initConnection(XMPPTCPConnection.java:685)
at org.jivesoftware.smack.tcp.XMPPTCPConnection.connectInternal(XMPPTCPConnection.java:942)
at org.jivesoftware.smack.AbstractXMPPConnection.connect(AbstractXMPPConnection.java:417)
- locked <0x0ec6e6bb> (a org.jivesoftware.smack.tcp.XMPPTCPConnection)
at org.yaxim.androidclient.service.SmackableImp.connectAndLogin(SmackableImp.java:717)
- locked <0x0ec6e6bb> (a org.jivesoftware.smack.tcp.XMPPTCPConnection)
at org.yaxim.androidclient.service.SmackableImp.doConnect(SmackableImp.java:304)
at org.yaxim.androidclient.service.SmackableImp$5.run(SmackableImp.java:391)
If a stream resume fails, smack will re-send all queued stanzas after a
reconnect. However, it does not make sense to re-send them:
* IQs / IQ responses have probably timed out
* MUC messages and PMs will be rejected as you haven't rejoined yet
* regular messages should be amended with a <delay> element
This patch adds a StanzaDroppedListener interface to the
XMPPTCPConnection. If at least one StanzaDroppedListener is provided,
all queued messages will be drained into the StanzaDroppedListener(s).
Otherwise, the original behavior of attempting to transmit them will be
followed.
Discussion: https://discourse.igniterealtime.org/t/xep-0198-resume-failure-reconnect-resending-of-muc-messages/83510/3
Signed-off-by: Georg Lukas <georg@op-co.de>
to clean up the state build up by connect().
Related to SMACK-855 there is the possiblitiy of a stray (writer)
thread if, for example, tlsHandled.checkifSuccessOrWaitorThrow() in
XMPPTCPConnection.connectInternal() throws. This commit should prevent
that.
The inetAddressAndPort String is redundant since
a2743549b8, because we now construct the
InetSocketAddress earlier and can hence use it in the log statement.
The purpose of the "maybeCompressFeaturesReceived" synchronization
point is to wait for the stream features after authentication. This is
not really reflected by its name and furthermore its
checkIfSuccessOrWait() method was only invoked if compression was
enabled, but we always want to wait for the stream features after
authentication. Hence move the call before the isCompressionEnabled()
check and one layer up in the call stack.
Fixes SMACK-846.
Florian Schmaus
Georg Lukas
Jesus Fuentes
Paul Schaub