check if thread was interrupted before doing operations on socket

Daniel Gultsch created

Change summary

src/main/java/eu/siacs/conversations/services/XmppConnectionService.java |   3 
src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java            | 150 
2 files changed, 77 insertions(+), 76 deletions(-)

Detailed changes

src/main/java/eu/siacs/conversations/services/XmppConnectionService.java 🔗

@@ -2897,8 +2897,6 @@ public class XmppConnectionService extends Service {
 			if (connection == null) {
 				connection = createConnection(account);
 				account.setXmppConnection(connection);
-			} else {
-				connection.interrupt();
 			}
 			if (!account.isOptionSet(Account.OPTION_DISABLED)) {
 				if (!force) {
@@ -2907,6 +2905,7 @@ public class XmppConnectionService extends Service {
 				Thread thread = new Thread(connection);
 				connection.setInteractive(interactive);
 				connection.prepareNewConnection();
+				connection.interrupt();
 				thread.start();
 				scheduleWakeUpCall(Config.CONNECT_DISCO_TIMEOUT, account.getUuid().hashCode());
 			} else {

src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java 🔗

@@ -218,7 +218,10 @@ public class XmppConnection implements Runnable {
 		mXmppConnectionService = service;
 	}
 
-	protected void changeStatus(final Account.State nextStatus) {
+	protected synchronized void changeStatus(final Account.State nextStatus) {
+		if (Thread.currentThread().isInterrupted()) {
+			Log.d(Config.LOGTAG,account.getJid().toBareJid()+": not changing status to "+nextStatus+" because thread was interrupted");
+		}
 		if (account.getStatus() != nextStatus) {
 			if ((nextStatus == Account.State.OFFLINE)
 					&& (account.getStatus() != Account.State.CONNECTING)
@@ -262,6 +265,7 @@ public class XmppConnection implements Runnable {
 				break;
 		}
 		try {
+			Socket localSocket;
 			shouldAuthenticate = needsBinding = !account.isOptionSet(Account.OPTION_REGISTER);
 			tagReader = new XmlReader(wakeLock);
 			tagWriter = new TagWriter();
@@ -276,8 +280,15 @@ public class XmppConnection implements Runnable {
 					destination = account.getHostname();
 				}
 				Log.d(Config.LOGTAG, account.getJid().toBareJid() + ": connect to " + destination + " via Tor");
-				socket = SocksSocketFactory.createSocketOverTor(destination, account.getPort());
-				startXmpp();
+				localSocket = SocksSocketFactory.createSocketOverTor(destination, account.getPort());
+				try {
+					startXmpp(localSocket);
+				} catch (InterruptedException e) {
+					Log.d(Config.LOGTAG,account.getJid().toBareJid()+": thread was interrupted before beginning stream");
+					return;
+				} catch (Exception e) {
+					throw new IOException(e.getMessage());
+				}
 			} else if (extended && account.getHostname() != null && !account.getHostname().isEmpty()) {
 
 				InetSocketAddress address = new InetSocketAddress(account.getHostname(), account.getPort());
@@ -288,33 +299,47 @@ public class XmppConnection implements Runnable {
 					if (features.encryptionEnabled) {
 						try {
 							final TlsFactoryVerifier tlsFactoryVerifier = getTlsFactoryVerifier();
-							socket = tlsFactoryVerifier.factory.createSocket();
-							socket.connect(address, Config.SOCKET_TIMEOUT * 1000);
-							final SSLSession session = ((SSLSocket) socket).getSession();
+							localSocket = tlsFactoryVerifier.factory.createSocket();
+							localSocket.connect(address, Config.SOCKET_TIMEOUT * 1000);
+							final SSLSession session = ((SSLSocket) localSocket).getSession();
 							if (!tlsFactoryVerifier.verifier.verify(account.getServer().getDomainpart(), session)) {
 								Log.d(Config.LOGTAG, account.getJid().toBareJid() + ": TLS certificate verification failed");
 								throw new SecurityException();
 							}
 						} catch (KeyManagementException e) {
 							features.encryptionEnabled = false;
-							socket = new Socket();
+							localSocket = new Socket();
 						}
 					} else {
-						socket = new Socket();
-						socket.connect(address, Config.SOCKET_TIMEOUT * 1000);
+						localSocket = new Socket();
+						localSocket.connect(address, Config.SOCKET_TIMEOUT * 1000);
 					}
 				} catch (IOException e) {
 					throw new UnknownHostException();
 				}
-				startXmpp();
+				try {
+					startXmpp(localSocket);
+				} catch (InterruptedException e) {
+					Log.d(Config.LOGTAG,account.getJid().toBareJid()+": thread was interrupted before beginning stream");
+					return;
+				} catch (Exception e) {
+					throw new IOException(e.getMessage());
+				}
 			} else if (DNSHelper.isIp(account.getServer().toString())) {
-				socket = new Socket();
+				localSocket = new Socket();
 				try {
-					socket.connect(new InetSocketAddress(account.getServer().toString(), 5222), Config.SOCKET_TIMEOUT * 1000);
+					localSocket.connect(new InetSocketAddress(account.getServer().toString(), 5222), Config.SOCKET_TIMEOUT * 1000);
 				} catch (IOException e) {
 					throw new UnknownHostException();
 				}
-				startXmpp();
+				try {
+					startXmpp(localSocket);
+				} catch (InterruptedException e) {
+					Log.d(Config.LOGTAG,account.getJid().toBareJid()+": thread was interrupted before beginning stream");
+					return;
+				} catch (Exception e) {
+					throw new IOException(e.getMessage());
+				}
 			} else {
 				final Bundle result = DNSHelper.getSRVRecord(account.getServer(), mXmppConnectionService);
 				final ArrayList<Parcelable> values = result.getParcelableArrayList("values");
@@ -350,32 +375,34 @@ public class XmppConnection implements Runnable {
 						}
 
 						if (!features.encryptionEnabled) {
-							socket = new Socket();
-							socket.connect(addr, Config.SOCKET_TIMEOUT * 1000);
+							localSocket = new Socket();
+							localSocket.connect(addr, Config.SOCKET_TIMEOUT * 1000);
 						} else {
 							final TlsFactoryVerifier tlsFactoryVerifier = getTlsFactoryVerifier();
-							socket = tlsFactoryVerifier.factory.createSocket();
+							localSocket = tlsFactoryVerifier.factory.createSocket();
 
-							if (socket == null) {
+							if (localSocket == null) {
 								throw new IOException("could not initialize ssl socket");
 							}
 
-							SSLSocketHelper.setSecurity((SSLSocket) socket);
-							SSLSocketHelper.setSNIHost(tlsFactoryVerifier.factory, (SSLSocket) socket, account.getServer().getDomainpart());
-							SSLSocketHelper.setAlpnProtocol(tlsFactoryVerifier.factory, (SSLSocket) socket, "xmpp-client");
+							SSLSocketHelper.setSecurity((SSLSocket) localSocket);
+							SSLSocketHelper.setSNIHost(tlsFactoryVerifier.factory, (SSLSocket) localSocket, account.getServer().getDomainpart());
+							SSLSocketHelper.setAlpnProtocol(tlsFactoryVerifier.factory, (SSLSocket) localSocket, "xmpp-client");
 
-							socket.connect(addr, Config.SOCKET_TIMEOUT * 1000);
+							localSocket.connect(addr, Config.SOCKET_TIMEOUT * 1000);
 
-							if (!tlsFactoryVerifier.verifier.verify(account.getServer().getDomainpart(), ((SSLSocket) socket).getSession())) {
+							if (!tlsFactoryVerifier.verifier.verify(account.getServer().getDomainpart(), ((SSLSocket) localSocket).getSession())) {
 								Log.d(Config.LOGTAG, account.getJid().toBareJid() + ": TLS certificate verification failed");
 								throw new SecurityException();
 							}
 						}
-
-						if (startXmpp())
+						if (startXmpp(localSocket))
 							break; // successfully connected to server that speaks xmpp
 					} catch (final SecurityException e) {
 						throw e;
+					} catch (InterruptedException e) {
+						Log.d(Config.LOGTAG,account.getJid().toBareJid()+": thread was interrupted before beginning stream");
+						return;
 					} catch (final Throwable e) {
 						Log.d(Config.LOGTAG, account.getJid().toBareJid().toString() + ": " + e.getMessage() + "(" + e.getClass().getName() + ")");
 						if (!iterator.hasNext()) {
@@ -385,8 +412,10 @@ public class XmppConnection implements Runnable {
 				}
 			}
 			processStream();
-		} catch (final java.lang.SecurityException e) {
+		}  catch (final java.lang.SecurityException e) {
 			this.changeStatus(Account.State.MISSING_INTERNET_PERMISSION);
+		} catch (final RegistrationNotSupportedException e) {
+			this.changeStatus(Account.State.REGISTRATION_NOT_SUPPORTED);
 		} catch (final IncompatibleServerException e) {
 			this.changeStatus(Account.State.INCOMPATIBLE_SERVER);
 		} catch (final SecurityException e) {
@@ -410,12 +439,16 @@ public class XmppConnection implements Runnable {
 			this.changeStatus(Account.State.OFFLINE);
 			this.attempt = Math.max(0, this.attempt - 1);
 		} finally {
-			forceCloseSocket();
-			if (wakeLock.isHeld()) {
-				try {
-					wakeLock.release();
-				} catch (final RuntimeException ignored) {
+			if (!Thread.currentThread().isInterrupted()) {
+				forceCloseSocket();
+				if (wakeLock.isHeld()) {
+					try {
+						wakeLock.release();
+					} catch (final RuntimeException ignored) {
+					}
 				}
+			} else {
+				Log.d(Config.LOGTAG,account.getJid().toBareJid()+": not force closing socket and releasing wake lock because thread was interrupted");
 			}
 		}
 	}
@@ -423,27 +456,18 @@ public class XmppConnection implements Runnable {
 	/**
 	 * Starts xmpp protocol, call after connecting to socket
 	 * @return true if server returns with valid xmpp, false otherwise
-	 * @throws IOException Unknown tag on connect
-	 * @throws XmlPullParserException Bad Xml
-	 * @throws NoSuchAlgorithmException Other error
      */
-	private boolean startXmpp() throws IOException, XmlPullParserException, NoSuchAlgorithmException {
+	private boolean startXmpp(Socket socket) throws Exception {
+		if (Thread.currentThread().isInterrupted()) {
+			throw new InterruptedException();
+		}
+		this.socket = socket;
 		tagWriter.setOutputStream(socket.getOutputStream());
 		tagReader.setInputStream(socket.getInputStream());
 		tagWriter.beginDocument();
 		sendStartStream();
-		Tag nextTag;
-		while ((nextTag = tagReader.readTag()) != null) {
-			if (nextTag.isStart("stream")) {
-				return true;
-			} else {
-				throw new IOException("unknown tag on connect");
-			}
-		}
-		if (socket.isConnected()) {
-			socket.close();
-		}
-		return false;
+		final Tag tag = tagReader.readTag();
+		return tag != null && tag.isStart("stream");
 	}
 
 	private static class TlsFactoryVerifier {
@@ -812,10 +836,8 @@ public class XmppConnection implements Runnable {
 			} else {
 				throw new IncompatibleServerException();
 			}
-		} else if (!this.streamFeatures.hasChild("register")
-				&& account.isOptionSet(Account.OPTION_REGISTER)) {
-			forceCloseSocket();
-			changeStatus(Account.State.REGISTRATION_NOT_SUPPORTED);
+		} else if (!this.streamFeatures.hasChild("register") && account.isOptionSet(Account.OPTION_REGISTER)) {
+			throw new RegistrationNotSupportedException();
 		} else if (this.streamFeatures.hasChild("mechanisms")
 				&& shouldAuthenticate
 				&& (features.encryptionEnabled || Config.ALLOW_NON_TLS_CONNECTIONS)) {
@@ -1359,30 +1381,6 @@ public class XmppConnection implements Runnable {
 		}
 	}
 
-	public void waitForPush() {
-		if (tagWriter.isActive()) {
-			tagWriter.finish();
-			new Thread(new Runnable() {
-				@Override
-				public void run() {
-					try {
-						while(!tagWriter.finished()) {
-							Thread.sleep(10);
-						}
-						socket.close();
-						Log.d(Config.LOGTAG,account.getJid().toBareJid()+": closed tcp without closing stream");
-						changeStatus(Account.State.OFFLINE);
-					} catch (IOException | InterruptedException e) {
-						Log.d(Config.LOGTAG,account.getJid().toBareJid()+": error while closing socket for waitForPush()");
-					}
-				}
-			}).start();
-		} else {
-			forceCloseSocket();
-			Log.d(Config.LOGTAG, account.getJid().toBareJid() + ": closed tcp without closing stream (no waiting)");
-		}
-	}
-
 	private void forceCloseSocket() {
 		if (socket != null) {
 			try {
@@ -1569,6 +1567,10 @@ public class XmppConnection implements Runnable {
 
 	}
 
+	private class RegistrationNotSupportedException extends IOException {
+
+	}
+
 	public enum Identity {
 		FACEBOOK,
 		SLACK,