1
0
Fork 0
mirror of https://github.com/vanitasvitae/Smack.git synced 2024-12-23 02:58:00 +01:00

SMACK-417 If both PacketReader and PacketWriter fail at the same time, connectionClosedonError() is called two times

Refactored notifyConnectionError() and notifyReconnection() from PacketReader to XMPPConnection. Made PacketReader.done and PacketWriter.done volatile. Prevent duplicate connectionClosedonError() calls by making the method synchronzied and protected them with an enter guard: if (packetReader.done && packetWriter.done) return;

git-svn-id: http://svn.igniterealtime.org/svn/repos/smack/branches/smack_3_3_0@13568 b35dd754-fafc-0310-a699-88a17e54d16e
This commit is contained in:
Florian Schmaus 2013-03-18 19:58:48 +00:00 committed by flow
parent 6dcf8e0123
commit a55b54f20b
5 changed files with 65 additions and 61 deletions

View file

@ -48,7 +48,7 @@ class PacketReader {
private XMPPConnection connection; private XMPPConnection connection;
private XmlPullParser parser; private XmlPullParser parser;
private boolean done; volatile boolean done;
private String connectionID = null; private String connectionID = null;
private Semaphore connectionSemaphore; private Semaphore connectionSemaphore;
@ -155,48 +155,6 @@ class PacketReader {
connection.collectors.clear(); connection.collectors.clear();
} }
/**
* Sends out a notification that there was an error with the connection
* and closes the connection.
*
* @param e the exception that causes the connection close event.
*/
void notifyConnectionError(Exception e) {
done = true;
// Closes the connection temporary. A reconnection is possible
connection.shutdown(new Presence(Presence.Type.unavailable));
// Print the stack trace to help catch the problem
e.printStackTrace();
// Notify connection listeners of the error.
for (ConnectionListener listener : connection.getConnectionListeners()) {
try {
listener.connectionClosedOnError(e);
}
catch (Exception e2) {
// Catch and print any exception so we can recover
// from a faulty listener
e2.printStackTrace();
}
}
}
/**
* Sends a notification indicating that the connection was reconnected successfully.
*/
protected void notifyReconnection() {
// Notify connection listeners of the reconnection.
for (ConnectionListener listener : connection.getConnectionListeners()) {
try {
listener.reconnectionSuccessful();
}
catch (Exception e) {
// Catch and print any exception so we can recover
// from a faulty listener
e.printStackTrace();
}
}
}
/** /**
* Resets the parser using the latest connection's reader. Reseting the parser is necessary * Resets the parser using the latest connection's reader. Reseting the parser is necessary
* when the plain connection has been secured or when a new opening stream element is going * when the plain connection has been secured or when a new opening stream element is going
@ -332,7 +290,7 @@ class PacketReader {
if (!(done || connection.isSocketClosed())) { if (!(done || connection.isSocketClosed())) {
// Close the connection and notify connection listeners of the // Close the connection and notify connection listeners of the
// error. // error.
notifyConnectionError(e); connection.notifyConnectionError(e);
} }
} }
} }

View file

@ -44,7 +44,7 @@ class PacketWriter {
private Writer writer; private Writer writer;
private XMPPConnection connection; private XMPPConnection connection;
private final BlockingQueue<Packet> queue; private final BlockingQueue<Packet> queue;
private boolean done; volatile boolean done;
/** /**
* Timestamp when the last stanza was sent to the server. This information is used * Timestamp when the last stanza was sent to the server. This information is used
@ -245,7 +245,7 @@ class PacketWriter {
// packetReader could be set to null by an concurrent disconnect() call. // packetReader could be set to null by an concurrent disconnect() call.
// Therefore Prevent NPE exceptions by checking packetReader. // Therefore Prevent NPE exceptions by checking packetReader.
if (connection.packetReader != null) { if (connection.packetReader != null) {
connection.packetReader.notifyConnectionError(ioe); connection.notifyConnectionError(ioe);
} }
} }
} }

View file

@ -473,6 +473,10 @@ public class XMPPConnection extends Connection {
return; return;
} }
if (!isConnected()) {
return;
}
shutdown(unavailablePresence); shutdown(unavailablePresence);
if (roster != null) { if (roster != null) {
@ -483,9 +487,7 @@ public class XMPPConnection extends Connection {
wasAuthenticated = false; wasAuthenticated = false;
packetWriter.cleanup(); packetWriter.cleanup();
packetWriter = null;
packetReader.cleanup(); packetReader.cleanup();
packetReader = null;
} }
public void sendPacket(Packet packet) { public void sendPacket(Packet packet) {
@ -614,10 +616,8 @@ public class XMPPConnection extends Connection {
*/ */
private void initConnection() throws XMPPException { private void initConnection() throws XMPPException {
boolean isFirstInitialization = packetReader == null || packetWriter == null; boolean isFirstInitialization = packetReader == null || packetWriter == null;
if (!isFirstInitialization) { compressionHandler = null;
compressionHandler = null; serverAckdCompression = false;
serverAckdCompression = false;
}
// Set the reader and writer instance variables // Set the reader and writer instance variables
initReaderAndWriter(); initReaderAndWriter();
@ -661,7 +661,7 @@ public class XMPPConnection extends Connection {
} }
} }
else if (!wasAuthenticated) { else if (!wasAuthenticated) {
packetReader.notifyReconnection(); notifyReconnection();
} }
} }
@ -773,7 +773,7 @@ public class XMPPConnection extends Connection {
void startTLSReceived(boolean required) { void startTLSReceived(boolean required) {
if (required && config.getSecurityMode() == if (required && config.getSecurityMode() ==
ConnectionConfiguration.SecurityMode.disabled) { ConnectionConfiguration.SecurityMode.disabled) {
packetReader.notifyConnectionError(new IllegalStateException( notifyConnectionError(new IllegalStateException(
"TLS required by server but not allowed by connection configuration")); "TLS required by server but not allowed by connection configuration"));
return; return;
} }
@ -787,7 +787,7 @@ public class XMPPConnection extends Connection {
writer.flush(); writer.flush();
} }
catch (IOException e) { catch (IOException e) {
packetReader.notifyConnectionError(e); notifyConnectionError(e);
} }
} }
@ -977,7 +977,7 @@ public class XMPPConnection extends Connection {
writer.flush(); writer.flush();
} }
catch (IOException e) { catch (IOException e) {
packetReader.notifyConnectionError(e); notifyConnectionError(e);
} }
} }
@ -1041,7 +1041,7 @@ public class XMPPConnection extends Connection {
login(config.getUsername(), config.getPassword(), login(config.getUsername(), config.getPassword(),
config.getResource()); config.getResource());
} }
packetReader.notifyReconnection(); notifyReconnection();
} }
catch (XMPPException e) { catch (XMPPException e) {
e.printStackTrace(); e.printStackTrace();
@ -1059,4 +1059,50 @@ public class XMPPConnection extends Connection {
this.wasAuthenticated = wasAuthenticated; this.wasAuthenticated = wasAuthenticated;
} }
} }
/**
* Sends out a notification that there was an error with the connection
* and closes the connection. Also prints the stack trace of the given exception
*
* @param e the exception that causes the connection close event.
*/
synchronized void notifyConnectionError(Exception e) {
// Listeners were already notified of the exception, return right here.
if (packetReader.done && packetWriter.done) return;
packetReader.done = true;
packetWriter.done = true;
// Closes the connection temporary. A reconnection is possible
shutdown(new Presence(Presence.Type.unavailable));
// Print the stack trace to help catch the problem
e.printStackTrace();
// Notify connection listeners of the error.
for (ConnectionListener listener : getConnectionListeners()) {
try {
listener.connectionClosedOnError(e);
}
catch (Exception e2) {
// Catch and print any exception so we can recover
// from a faulty listener
e2.printStackTrace();
}
}
}
/**
* Sends a notification indicating that the connection was reconnected successfully.
*/
protected void notifyReconnection() {
// Notify connection listeners of the reconnection.
for (ConnectionListener listener : getConnectionListeners()) {
try {
listener.reconnectionSuccessful();
}
catch (Exception e) {
// Catch and print any exception so we can recover
// from a faulty listener
e.printStackTrace();
}
}
}
} }

View file

@ -44,7 +44,7 @@ public class ReconnectionTest extends SmackTestCase {
connection.addConnectionListener(listener); connection.addConnectionListener(listener);
// Simulates an error in the connection // Simulates an error in the connection
connection.packetReader.notifyConnectionError(new Exception("Simulated Error")); connection.notifyConnectionError(new Exception("Simulated Error"));
Thread.sleep(12000); Thread.sleep(12000);
// After 10 seconds, the reconnection manager must reestablishes the connection // After 10 seconds, the reconnection manager must reestablishes the connection
assertEquals("The ConnectionListener.connectionStablished() notification was not fired", assertEquals("The ConnectionListener.connectionStablished() notification was not fired",
@ -79,7 +79,7 @@ public class ReconnectionTest extends SmackTestCase {
connection.addConnectionListener(listener); connection.addConnectionListener(listener);
// Simulates an error in the connection // Simulates an error in the connection
connection.packetReader.notifyConnectionError(new Exception("Simulated Error")); connection.notifyConnectionError(new Exception("Simulated Error"));
Thread.sleep(12000); Thread.sleep(12000);
// After 10 seconds, the reconnection manager must reestablishes the connection // After 10 seconds, the reconnection manager must reestablishes the connection
assertEquals("The ConnectionListener.connectionStablished() notification was not fired", assertEquals("The ConnectionListener.connectionStablished() notification was not fired",
@ -103,7 +103,7 @@ public class ReconnectionTest extends SmackTestCase {
connection.addConnectionListener(listener); connection.addConnectionListener(listener);
// Produces a connection error // Produces a connection error
connection.packetReader.notifyConnectionError(new Exception("Simulated Error")); connection.notifyConnectionError(new Exception("Simulated Error"));
assertEquals( assertEquals(
"An error occurs but the ConnectionListener.connectionClosedOnError(e) was not notified", "An error occurs but the ConnectionListener.connectionClosedOnError(e) was not notified",
true, listener.connectionClosedOnError); true, listener.connectionClosedOnError);

View file

@ -689,7 +689,7 @@ public class RosterSmackTest extends SmackTestCase {
Thread.sleep(200); Thread.sleep(200);
// Break the connection // Break the connection
getConnection(0).packetReader.notifyConnectionError(new Exception("Simulated Error")); getConnection(0).notifyConnectionError(new Exception("Simulated Error"));
Presence presence = roster.getPresence(getBareJID(1)); Presence presence = roster.getPresence(getBareJID(1));
assertFalse("Unavailable presence not found for offline user", presence.isAvailable()); assertFalse("Unavailable presence not found for offline user", presence.isAvailable());