From 3bf2876e0931a064587ca5e560b5ccde18977b86 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Sat, 19 Nov 2016 10:44:40 +0100 Subject: [PATCH] check if thread was interrupted before doing operations on socket --- .../services/XmppConnectionService.java | 3 +- .../conversations/xmpp/XmppConnection.java | 152 +++++++++--------- 2 files changed, 78 insertions(+), 77 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java b/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java index 10155a147..0b33aa0cd 100644 --- a/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java +++ b/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 { diff --git a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java index 86c9a13ef..a1bd5ccfa 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java +++ b/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(); - } else if (DNSHelper.isIp(account.getServer().toString())) { - socket = new Socket(); try { - socket.connect(new InetSocketAddress(account.getServer().toString(), 5222), Config.SOCKET_TIMEOUT * 1000); + 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())) { + localSocket = new Socket(); + try { + 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 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,