vanitasvitae/Smack
1
0
Fork 0
mirror of https://github.com/vanitasvitae/Smack.git synced 2024-11-22 20:12:07 +01:00
Code Issues Releases Wiki Activity

Improve PacketReader startup()

Browse source 
Instead of using the connectionID, we now use a new boolean
lastFeaturesParsed to mark when startup() is able to continue or when a
NoResponseException should be thrown.

Fixes the problem when the connectionId was set and the wait() would
timeout, resulting in startup() believing that everything was ok, while
in fact the features where not yet received (or parsed). SMACK-558.
This commit is contained in:
Florian Schmaus 2014-04-26 19:00:58 +02:00
parent 76ce883ef3
commit a613d5f574
1 changed files with 19 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

33
tcp/src/main/java/org/jivesoftware/smack/PacketReader.java
View file

@ -53,9 +53,16 @@ class PacketReader {
private XMPPTCPConnection connection; private XMPPTCPConnection connection;
private XmlPullParser parser; private XmlPullParser parser;
volatile boolean done;
private String connectionID = null; /**
* Set to true if the last features stanza from the server has been parsed. A XMPP connection
* handshake can invoke multiple features stanzas, e.g. when TLS is activated a second feature
* stanza is send by the server. This is set to true once the last feature stanza has been
* parsed.
*/
private volatile boolean lastFeaturesParsed;
volatile boolean done;
protected PacketReader(final XMPPTCPConnection connection) { protected PacketReader(final XMPPTCPConnection connection) {
this.connection = connection; this.connection = connection;
@ -68,7 +75,7 @@ class PacketReader {
*/ */
protected void init() { protected void init() {
done = false; done = false;
connectionID = null; lastFeaturesParsed = false;
readerThread = new Thread() { readerThread = new Thread() {
public void run() { public void run() {
@ -83,10 +90,11 @@ class PacketReader {
/** /**
* Starts the packet reader thread and returns once a connection to the server * Starts the packet reader thread and returns once a connection to the server
* has been established. A connection will be attempted for a maximum of five * has been established or if the server's features could not be parsed within
* seconds. An XMPPException will be thrown if the connection fails. * the connection's PacketReplyTimeout.
*
* @throws NoResponseException if the server fails to send an opening stream back * @throws NoResponseException if the server fails to send an opening stream back
* within packetReplyTimeout*3. * within packetReplyTimeout.
*/ */
synchronized public void startup() throws NoResponseException { synchronized public void startup() throws NoResponseException {
readerThread.start(); readerThread.start();
@ -95,20 +103,16 @@ class PacketReader {
try { try {
// A waiting thread may be woken up before the wait time or a notify // A waiting thread may be woken up before the wait time or a notify
// (although this is a rare thing). Therefore, we continue waiting // (although this is a rare thing). Therefore, we continue waiting
// until either a connectionID has been set (and hence a notify was // until either the server's features have been parsed (and hence a notify was
// made) or the total wait time has elapsed. // made) or the total wait time has elapsed.
long waitTime = connection.getPacketReplyTimeout(); wait(connection.getPacketReplyTimeout());
wait(3 * waitTime);
} }
catch (InterruptedException ie) { catch (InterruptedException ie) {
// Ignore. // Ignore.
} }
if (connectionID == null) { if (!lastFeaturesParsed) {
throw new NoResponseException(); throw new NoResponseException();
} }
else {
connection.connectionID = connectionID;
}
} }
/** /**
@ -201,7 +205,7 @@ class PacketReader {
for (int i=0; i<parser.getAttributeCount(); i++) { for (int i=0; i<parser.getAttributeCount(); i++) {
if (parser.getAttributeName(i).equals("id")) { if (parser.getAttributeName(i).equals("id")) {
// Save the connectionID // Save the connectionID
connectionID = parser.getAttributeValue(i); connection.connectionID = parser.getAttributeValue(i);
} }
else if (parser.getAttributeName(i).equals("from")) { else if (parser.getAttributeName(i).equals("from")) {
// Use the server name that the server says that it is. // Use the server name that the server says that it is.
@ -376,6 +380,7 @@ class PacketReader {
if (!startTLSReceived || connection.getConfiguration().getSecurityMode() == if (!startTLSReceived || connection.getConfiguration().getSecurityMode() ==
ConnectionConfiguration.SecurityMode.disabled) ConnectionConfiguration.SecurityMode.disabled)
{ {
lastFeaturesParsed = true;
// This synchronized block prevents this thread from calling notify() before the other // This synchronized block prevents this thread from calling notify() before the other
// thread had called wait() (it would cause an Exception if wait() hadn't been called) // thread had called wait() (it would cause an Exception if wait() hadn't been called)
synchronized (this) { synchronized (this) {