Fix deadlocks while connecting.

Replace an events loop at the communication service with single-shot
connection workers. This should help to avoid deadlock behaviour in situations
when user quits the connection to one server and tries to connect to another
one. The events loop required synchronization blocks which cause ANR in
the situation described above.

Change the server connection interface to contain a connecting stage
in a separate method. This is more just-in-case measure for situations
when the connection constructor shouldn't be blocking.

Change-Id: I941a4b67d965f6b1f76bc9975818e82aea12bf00

6 files changed, 147 insertions, 171 deletions

diff --git a/android/sdremote/src/org/libreoffice/impressremote/activity/SlideShowActivity.java b/android/sdremote/src/org/libreoffice/impressremote/activity/SlideShowActivity.java
index 19b7dbbbf1d2..2d611340f8a7 100644
--- a/android/sdremote/src/org/libreoffice/impressremote/activity/SlideShowActivity.java
+++ b/android/sdremote/src/org/libreoffice/impressremote/activity/SlideShowActivity.java
@@ -502,8 +502,6 @@ public class SlideShowActivity extends SherlockFragmentActivity implements Servi
 
         stopTimer();
 
-        // TODO: disconnect computer
-
         unbindService();
     }
 
diff --git a/android/sdremote/src/org/libreoffice/impressremote/communication/BluetoothServerConnection.java b/android/sdremote/src/org/libreoffice/impressremote/communication/BluetoothServerConnection.java
index fe5834d9a8a2..1056b7dfe116 100644
--- a/android/sdremote/src/org/libreoffice/impressremote/communication/BluetoothServerConnection.java
+++ b/android/sdremote/src/org/libreoffice/impressremote/communication/BluetoothServerConnection.java
@@ -14,10 +14,11 @@ import java.io.InputStream;
 import java.io.OutputStream;
 import java.util.UUID;
 
-import android.bluetooth.BluetoothAdapter;
 import android.bluetooth.BluetoothDevice;
 import android.bluetooth.BluetoothSocket;
 
+import org.libreoffice.impressremote.util.BluetoothOperator;
+
 class BluetoothServerConnection implements ServerConnection {
     // Standard UUID for the Serial Port Profile.
     // https://www.bluetooth.org/en-us/specification/assigned-numbers-overview/service-discovery
@@ -31,18 +32,31 @@ class BluetoothServerConnection implements ServerConnection {
 
     private BluetoothSocket buildServerConnection(Server aServer) {
         try {
-            BluetoothDevice aBluetoothServer = BluetoothAdapter
-                .getDefaultAdapter().getRemoteDevice(aServer.getAddress());
+            BluetoothDevice aBluetoothServer = BluetoothOperator.getAdapter()
+                .getRemoteDevice(aServer.getAddress());
 
-            BluetoothSocket aServerConnection = aBluetoothServer
-                .createRfcommSocketToServiceRecord(
-                    UUID.fromString(STANDARD_SPP_UUID));
+            return aBluetoothServer.createRfcommSocketToServiceRecord(
+                UUID.fromString(STANDARD_SPP_UUID));
+        } catch (IOException e) {
+            throw new RuntimeException("Unable to create server connection.");
+        }
+    }
 
-            aServerConnection.connect();
+    @Override
+    public void open() {
+        try {
+            mServerConnection.connect();
+        } catch (IOException e) {
+            throw new RuntimeException("Unable to open server connection.");
+        }
+    }
 
-            return aServerConnection;
+    @Override
+    public void close() {
+        try {
+            mServerConnection.close();
         } catch (IOException e) {
-            throw new RuntimeException("Unable to connect to Bluetooth host.");
+            throw new RuntimeException("Unable to close server connection.");
         }
     }
 
@@ -63,15 +77,6 @@ class BluetoothServerConnection implements ServerConnection {
             throw new RuntimeException("Unable to open commands stream.");
         }
     }
-
-    @Override
-    public void close() {
-        try {
-            mServerConnection.close();
-        } catch (IOException e) {
-            throw new RuntimeException("Unable to close server connection.");
-        }
-    }
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/android/sdremote/src/org/libreoffice/impressremote/communication/CommunicationService.java b/android/sdremote/src/org/libreoffice/impressremote/communication/CommunicationService.java
index d7915c3a9d57..b4285f100cf3 100644
--- a/android/sdremote/src/org/libreoffice/impressremote/communication/CommunicationService.java
+++ b/android/sdremote/src/org/libreoffice/impressremote/communication/CommunicationService.java
@@ -20,43 +20,23 @@ import org.libreoffice.impressremote.util.BluetoothOperator;
 import org.libreoffice.impressremote.util.Intents;
 
 public class CommunicationService extends Service implements Runnable, MessagesListener, Timer.TimerListener {
-    public static enum State {
-        DISCONNECTED, SEARCHING, CONNECTING, CONNECTED
-    }
-
-    /**
-     * Used to protect all writes to mState, mStateDesired, and mServerDesired.
-     */
-    private final Object mConnectionVariableMutex = new Object();
-
-    private State mState;
-    private State mStateDesired;
-
-    private Server mServerDesired;
-
     private IBinder mBinder;
 
     private ServersManager mServersManager;
 
-    private ServerConnection mServerConnection;
-
-    private MessagesReceiver mMessagesReceiver;
-    private CommandsTransmitter mCommandsTransmitter;
-
     private BluetoothOperator.State mBluetoothState;
 
     private Timer mTimer;
     private SlideShow mSlideShow;
 
-    private Thread mThread;
+    private Server mServer;
+    private ServerConnection mServerConnection;
+
+    private MessagesReceiver mMessagesReceiver;
+    private CommandsTransmitter mCommandsTransmitter;
 
     @Override
     public void onCreate() {
-        mState = State.DISCONNECTED;
-        mStateDesired = State.DISCONNECTED;
-
-        mServerDesired = null;
-
         mBinder = new CBinder();
 
         mServersManager = new ServersManager(this);
@@ -66,9 +46,6 @@ public class CommunicationService extends Service implements Runnable, MessagesL
 
         mTimer = new Timer(this);
         mSlideShow = new SlideShow(mTimer);
-
-        mThread = new Thread(this);
-        mThread.start();
     }
 
     public class CBinder extends Binder {
@@ -90,69 +67,62 @@ public class CommunicationService extends Service implements Runnable, MessagesL
         return mBinder;
     }
 
-    @Override
-    public void run() {
-        synchronized (this) {
-            while (true) {
-                // Condition
-                try {
-                    wait();
-                } catch (InterruptedException e) {
-                    // We have finished
-                    return;
-                }
-
-                // Work
-                synchronized (mConnectionVariableMutex) {
-                    if ((mStateDesired == State.CONNECTED) && (mState == State.CONNECTED)) {
-                        closeConnection();
-                    }
-
-                    if ((mStateDesired == State.DISCONNECTED) && (mState == State.CONNECTED)) {
-                        closeConnection();
-                    }
-
-                    if (mStateDesired == State.CONNECTED) {
-                        mState = State.CONNECTING;
-
-                        try {
-                            openConnection();
-                        }
-                        catch (RuntimeException e) {
-                            connectionFailed();
-                        }
-                    }
-                }
-            }
-        }
+    public void startServersSearch() {
+        mServersManager.startServersSearch();
     }
 
-    private void closeConnection() {
-        mServerConnection.close();
+    public void stopServersSearch() {
+        mServersManager.stopServersSearch();
+    }
+
+    public void addServer(String aAddress, String aName) {
+        mServersManager.addTcpServer(aAddress, aName);
+    }
+
+    public void removeServer(Server aServer) {
+        mServersManager.removeServer(aServer);
+    }
+
+    public List<Server> getServers() {
+        return mServersManager.getServers();
+    }
 
-        mState = State.DISCONNECTED;
+    public void connectServer(Server aServer) {
+        mServer = aServer;
+
+        new Thread(this).start();
+    }
+
+    @Override
+    public void run() {
+        try {
+            disconnectServer();
+            connectServer();
+        }
+        catch (RuntimeException e) {
+            sendConnectionFailedMessage();
+        }
     }
 
-    private void openConnection() {
+    private void connectServer() {
         mServerConnection = buildServerConnection();
+        mServerConnection.open();
 
         mMessagesReceiver = new MessagesReceiver(mServerConnection, this);
         mCommandsTransmitter = new CommandsTransmitter(mServerConnection);
 
-        if (PairingProvider.isPairingNecessary(mServerDesired)) {
+        if (PairingProvider.isPairingNecessary(mServer)) {
             pair();
         }
-
-        mState = State.CONNECTED;
     }
 
     private ServerConnection buildServerConnection() {
-        switch (mServerDesired.getProtocol()) {
+        switch (mServer.getProtocol()) {
             case TCP:
-                return new TcpServerConnection(mServerDesired);
+                return new TcpServerConnection(mServer);
 
             case BLUETOOTH:
-                return new BluetoothServerConnection(mServerDesired);
+                return new BluetoothServerConnection(mServer);
 
             default:
                 throw new RuntimeException("Unknown desired protocol.");
@@ -161,55 +131,26 @@ public class CommunicationService extends Service implements Runnable, MessagesL
 
     private void pair() {
         String aPairingDeviceName = PairingProvider.getPairingDeviceName(this);
-        String aPairingPin = PairingProvider.getPairingPin(this, mServerDesired);
+        String aPairingPin = PairingProvider.getPairingPin(this, mServer);
 
         mCommandsTransmitter.pair(aPairingDeviceName, aPairingPin);
     }
 
-    private void connectionFailed() {
-        mState = State.DISCONNECTED;
-
+    private void sendConnectionFailedMessage() {
         Intent aIntent = Intents.buildConnectionFailedIntent();
         LocalBroadcastManager.getInstance(this).sendBroadcast(aIntent);
     }
 
-    public void startServersSearch() {
-        mState = State.SEARCHING;
-
-        mServersManager.startServersSearch();
-    }
-
-    public void stopServersSearch() {
-        mServersManager.stopServersSearch();
-    }
-
-    public List<Server> getServers() {
-        return mServersManager.getServers();
-    }
-
-    public void connectTo(Server aServer) {
-        synchronized (mConnectionVariableMutex) {
-            if (mState == State.SEARCHING) {
-                mServersManager.stopServersSearch();
-                mState = State.DISCONNECTED;
-            }
-            mServerDesired = aServer;
-            mStateDesired = State.CONNECTED;
-            synchronized (this) {
-                notify();
-            }
-
+    public void disconnectServer() {
+        if (!isServerConnectionAvailable()) {
+            return;
         }
-        // TODO: connect
+
+        mServerConnection.close();
     }
 
-    public void disconnect() {
-        synchronized (mConnectionVariableMutex) {
-            mStateDesired = State.DISCONNECTED;
-            synchronized (this) {
-                notify();
-            }
-        }
+    private boolean isServerConnectionAvailable() {
+        return mServerConnection != null;
     }
 
     public CommandsTransmitter getTransmitter() {
@@ -220,17 +161,9 @@ public class CommunicationService extends Service implements Runnable, MessagesL
         return mSlideShow;
     }
 
-    public void addServer(String aAddress, String aName) {
-        mServersManager.addTcpServer(aAddress, aName);
-    }
-
-    public void removeServer(Server aServer) {
-        mServersManager.removeServer(aServer);
-    }
-
     @Override
     public void onPinValidation() {
-        String aPin = PairingProvider.getPairingPin(this, mServerDesired);
+        String aPin = PairingProvider.getPairingPin(this, mServer);
 
         Intent aIntent = Intents.buildPairingValidationIntent(aPin);
         LocalBroadcastManager.getInstance(this).sendBroadcast(aIntent);
@@ -294,11 +227,9 @@ public class CommunicationService extends Service implements Runnable, MessagesL
     @Override
     public void onDestroy() {
         stopServersSearch();
+        disconnectServer();
 
         restoreBluetoothState();
-
-        mThread.interrupt();
-        mThread = null;
     }
 
     private void restoreBluetoothState() {
@@ -310,6 +241,10 @@ public class CommunicationService extends Service implements Runnable, MessagesL
             return;
         }
 
+        disableBluetooth();
+    }
+
+    private void disableBluetooth() {
         BluetoothOperator.disable();
     }
 }
diff --git a/android/sdremote/src/org/libreoffice/impressremote/communication/ServerConnection.java b/android/sdremote/src/org/libreoffice/impressremote/communication/ServerConnection.java
index e0bb49d40562..c9179d2ef209 100644
--- a/android/sdremote/src/org/libreoffice/impressremote/communication/ServerConnection.java
+++ b/android/sdremote/src/org/libreoffice/impressremote/communication/ServerConnection.java
@@ -12,11 +12,13 @@ import java.io.InputStream;
 import java.io.OutputStream;
 
 interface ServerConnection {
+    public void open();
+
+    public void close();
+
     public InputStream buildMessagesStream();
 
     public OutputStream buildCommandsStream();
-
-    public void close();
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/android/sdremote/src/org/libreoffice/impressremote/communication/TcpServerConnection.java b/android/sdremote/src/org/libreoffice/impressremote/communication/TcpServerConnection.java
index a37d353221b7..fa18079f3d86 100644
--- a/android/sdremote/src/org/libreoffice/impressremote/communication/TcpServerConnection.java
+++ b/android/sdremote/src/org/libreoffice/impressremote/communication/TcpServerConnection.java
@@ -11,26 +11,45 @@ package org.libreoffice.impressremote.communication;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.net.InetSocketAddress;
 import java.net.Socket;
-import java.net.UnknownHostException;
+import java.net.SocketAddress;
 
 class TcpServerConnection implements ServerConnection {
+    private final Server mServer;
     private final Socket mServerConnection;
 
     public TcpServerConnection(Server aServer) {
-        mServerConnection = buildServerConnection(aServer);
+        mServer = aServer;
+        mServerConnection = buildServerConnection();
     }
 
-    private Socket buildServerConnection(Server aServer) {
+    private Socket buildServerConnection() {
+        return new Socket();
+    }
+
+    @Override
+    public void open() {
         try {
-            String aServerAddress = aServer.getAddress();
-            int aServerPort = Protocol.Ports.CLIENT_CONNECTION;
+            mServerConnection.connect(buildServerAddress());
+        } catch (IOException e) {
+            throw new RuntimeException("Unable to open server connection.");
+        }
+    }
 
-            return new Socket(aServerAddress, aServerPort);
-        } catch (UnknownHostException e) {
-            throw new RuntimeException("Unable to connect to unknown host.");
+    private SocketAddress buildServerAddress() {
+        String aServerAddress = mServer.getAddress();
+        int aServerPort = Protocol.Ports.CLIENT_CONNECTION;
+
+        return new InetSocketAddress(aServerAddress, aServerPort);
+    }
+
+    @Override
+    public void close() {
+        try {
+            mServerConnection.close();
         } catch (IOException e) {
-            throw new RuntimeException("Unable to connect to host.");
+            throw new RuntimeException("Unable to close server connection.");
         }
     }
 
@@ -51,15 +70,6 @@ class TcpServerConnection implements ServerConnection {
             throw new RuntimeException("Unable to open commands stream.");
         }
     }
-
-    @Override
-    public void close() {
-        try {
-            mServerConnection.close();
-        } catch (IOException e) {
-            throw new RuntimeException("Unable to close server connection.");
-        }
-    }
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/android/sdremote/src/org/libreoffice/impressremote/fragment/ComputerConnectionFragment.java b/android/sdremote/src/org/libreoffice/impressremote/fragment/ComputerConnectionFragment.java
index f72944ab1a2a..ec0eeb5d1ff5 100644
--- a/android/sdremote/src/org/libreoffice/impressremote/fragment/ComputerConnectionFragment.java
+++ b/android/sdremote/src/org/libreoffice/impressremote/fragment/ComputerConnectionFragment.java
@@ -37,7 +37,11 @@ import org.libreoffice.impressremote.communication.Server;
 import org.libreoffice.impressremote.util.SavedStates;
 
 public class ComputerConnectionFragment extends SherlockFragment implements ServiceConnection {
-    private Server mComputer;
+    public static enum Result {
+        CONNECTED, NOT_CONNECTED
+    }
+
+    private Result mResult = Result.NOT_CONNECTED;
 
     private CommunicationService mCommunicationService;
     private BroadcastReceiver mIntentsReceiver;
@@ -62,8 +66,6 @@ public class ComputerConnectionFragment extends SherlockFragment implements Serv
     public void onCreate(Bundle aSavedInstance) {
         super.onCreate(aSavedInstance);
 
-        mComputer = getArguments().getParcelable(Fragments.Arguments.COMPUTER);
-
         setUpActionBarMenu();
     }
 
@@ -136,10 +138,10 @@ public class ComputerConnectionFragment extends SherlockFragment implements Serv
         CommunicationService.CBinder aServiceBinder = (CommunicationService.CBinder) aBinder;
         mCommunicationService = aServiceBinder.getService();
 
-        connectToComputer();
+        connectComputer();
     }
 
-    private void connectToComputer() {
+    private void connectComputer() {
         if (!isServiceBound()) {
             return;
         }
@@ -148,7 +150,7 @@ public class ComputerConnectionFragment extends SherlockFragment implements Serv
             return;
         }
 
-        mCommunicationService.connectTo(mComputer);
+        mCommunicationService.connectServer(getComputer());
     }
 
     private boolean isServiceBound() {
@@ -163,6 +165,10 @@ public class ComputerConnectionFragment extends SherlockFragment implements Serv
         return (ProgressBar) getView().findViewById(R.id.progress_bar);
     }
 
+    private Server getComputer() {
+        return getArguments().getParcelable(Fragments.Arguments.COMPUTER);
+    }
+
     @Override
     public void onServiceDisconnected(ComponentName aComponentName) {
         mCommunicationService = null;
@@ -247,6 +253,8 @@ public class ComputerConnectionFragment extends SherlockFragment implements Serv
     }
 
     private void setUpPresentation() {
+        mResult = Result.CONNECTED;
+
         Intent aIntent = Intents.buildSlideShowIntent(getActivity());
         startActivity(aIntent);
 
@@ -261,7 +269,7 @@ public class ComputerConnectionFragment extends SherlockFragment implements Serv
     }
 
     private String buildSecondaryErrorMessage() {
-        switch (mComputer.getProtocol()) {
+        switch (getComputer().getProtocol()) {
             case BLUETOOTH:
                 return getString(R.string.message_impress_pairing_check);
 
@@ -311,7 +319,7 @@ public class ComputerConnectionFragment extends SherlockFragment implements Serv
         switch (aMenuItem.getItemId()) {
             case R.id.menu_reconnect:
                 showProgressBar();
-                connectToComputer();
+                connectComputer();
                 refreshActionBarMenu();
                 return true;
 
@@ -374,9 +382,27 @@ public class ComputerConnectionFragment extends SherlockFragment implements Serv
     public void onDestroy() {
         super.onDestroy();
 
+        disconnectComputer();
+
         unbindService();
     }
 
+    private void disconnectComputer() {
+        if (!isServiceBound()) {
+            return;
+        }
+
+        if (!isDisconnectRequired()) {
+            return;
+        }
+
+        mCommunicationService.disconnectServer();
+    }
+
+    private boolean isDisconnectRequired() {
+        return mResult == Result.NOT_CONNECTED;
+    }
+
     private void unbindService() {
         if (!isServiceBound()) {
             return;