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

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();
     }
 

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
+            BluetoothDevice aBluetoothServer = BluetoothOperator.getAdapter()
-                .getDefaultAdapter().getRemoteDevice(aServer.getAddress());
+                .getRemoteDevice(aServer.getAddress());
 
-            BluetoothSocket aServerConnection = aBluetoothServer
+            return aBluetoothServer.createRfcommSocketToServiceRecord(
-                .createRfcommSocketToServiceRecord(
+                UUID.fromString(STANDARD_SPP_UUID));
-                    UUID.fromString(STANDARD_SPP_UUID));
-
-            aServerConnection.connect();
-
-            return aServerConnection;
         } catch (IOException e) {
-            throw new RuntimeException("Unable to connect to Bluetooth host.");
+            throw new RuntimeException("Unable to create server connection.");
+        }
+    }
+
+    @Override
+    public void open() {
+        try {
+            mServerConnection.connect();
+        } catch (IOException e) {
+            throw new RuntimeException("Unable to open server connection.");
+        }
+    }
+
+    @Override
+    public void close() {
+        try {
+            mServerConnection.close();
+        } catch (IOException e) {
+            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: */

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,92 +67,7 @@ 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();
-                        }
-                    }
-                }
-            }
-        }
-    }
-
-    private void closeConnection() {
-        mServerConnection.close();
-
-        mState = State.DISCONNECTED;
-    }
-
-    private void openConnection() {
-        mServerConnection = buildServerConnection();
-
-        mMessagesReceiver = new MessagesReceiver(mServerConnection, this);
-        mCommandsTransmitter = new CommandsTransmitter(mServerConnection);
-
-        if (PairingProvider.isPairingNecessary(mServerDesired)) {
-            pair();
-        }
-
-        mState = State.CONNECTED;
-    }
-
-    private ServerConnection buildServerConnection() {
-        switch (mServerDesired.getProtocol()) {
-            case TCP:
-                return new TcpServerConnection(mServerDesired);
-
-            case BLUETOOTH:
-                return new BluetoothServerConnection(mServerDesired);
-
-            default:
-                throw new RuntimeException("Unknown desired protocol.");
-        }
-    }
-
-    private void pair() {
-        String aPairingDeviceName = PairingProvider.getPairingDeviceName(this);
-        String aPairingPin = PairingProvider.getPairingPin(this, mServerDesired);
-
-        mCommandsTransmitter.pair(aPairingDeviceName, aPairingPin);
-    }
-
-    private void connectionFailed() {
-        mState = State.DISCONNECTED;
-
-        Intent aIntent = Intents.buildConnectionFailedIntent();
-        LocalBroadcastManager.getInstance(this).sendBroadcast(aIntent);
-    }
-
     public void startServersSearch() {
-        mState = State.SEARCHING;
-
         mServersManager.startServersSearch();
     }
 
@@ -183,43 +75,6 @@ public class CommunicationService extends Service implements Runnable, MessagesL
         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();
-            }
-
-        }
-        // TODO: connect
-    }
-
-    public void disconnect() {
-        synchronized (mConnectionVariableMutex) {
-            mStateDesired = State.DISCONNECTED;
-            synchronized (this) {
-                notify();
-            }
-        }
-    }
-
-    public CommandsTransmitter getTransmitter() {
-        return mCommandsTransmitter;
-    }
-
-    public SlideShow getSlideShow() {
-        return mSlideShow;
-    }
-
     public void addServer(String aAddress, String aName) {
         mServersManager.addTcpServer(aAddress, aName);
     }
@@ -228,9 +83,87 @@ public class CommunicationService extends Service implements Runnable, MessagesL
         mServersManager.removeServer(aServer);
     }
 
+    public List<Server> getServers() {
+        return mServersManager.getServers();
+    }
+
+    public void connectServer(Server aServer) {
+        mServer = aServer;
+
+        new Thread(this).start();
+    }
+
+    @Override
+    public void run() {
+        try {
+            disconnectServer();
+            connectServer();
+        }
+        catch (RuntimeException e) {
+            sendConnectionFailedMessage();
+        }
+    }
+
+    private void connectServer() {
+        mServerConnection = buildServerConnection();
+        mServerConnection.open();
+
+        mMessagesReceiver = new MessagesReceiver(mServerConnection, this);
+        mCommandsTransmitter = new CommandsTransmitter(mServerConnection);
+
+        if (PairingProvider.isPairingNecessary(mServer)) {
+            pair();
+        }
+    }
+
+    private ServerConnection buildServerConnection() {
+        switch (mServer.getProtocol()) {
+            case TCP:
+                return new TcpServerConnection(mServer);
+
+            case BLUETOOTH:
+                return new BluetoothServerConnection(mServer);
+
+            default:
+                throw new RuntimeException("Unknown desired protocol.");
+        }
+    }
+
+    private void pair() {
+        String aPairingDeviceName = PairingProvider.getPairingDeviceName(this);
+        String aPairingPin = PairingProvider.getPairingPin(this, mServer);
+
+        mCommandsTransmitter.pair(aPairingDeviceName, aPairingPin);
+    }
+
+    private void sendConnectionFailedMessage() {
+        Intent aIntent = Intents.buildConnectionFailedIntent();
+        LocalBroadcastManager.getInstance(this).sendBroadcast(aIntent);
+    }
+
+    public void disconnectServer() {
+        if (!isServerConnectionAvailable()) {
+            return;
+        }
+
+        mServerConnection.close();
+    }
+
+    private boolean isServerConnectionAvailable() {
+        return mServerConnection != null;
+    }
+
+    public CommandsTransmitter getTransmitter() {
+        return mCommandsTransmitter;
+    }
+
+    public SlideShow getSlideShow() {
+        return mSlideShow;
+    }
+
     @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();
     }
 }

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: */

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() {
-        try {
+        return new Socket();
-            String aServerAddress = aServer.getAddress();
+    }
-            int aServerPort = Protocol.Ports.CLIENT_CONNECTION;
 
-            return new Socket(aServerAddress, aServerPort);
+    @Override
-        } catch (UnknownHostException e) {
+    public void open() {
-            throw new RuntimeException("Unable to connect to unknown host.");
+        try {
+            mServerConnection.connect(buildServerAddress());
         } catch (IOException e) {
-            throw new RuntimeException("Unable to connect to host.");
+            throw new RuntimeException("Unable to open server connection.");
+        }
+    }
+
+    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 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: */

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;