Fix concurrency problems.

– GameView should only draw on canvas from UI thread.
– ClientInterfaceBridge should not delete state after disconnect
  (Because an update may be still waiting.)
– Send a Bundle in JOIN_NETWORK request to SameService.

5 files changed, 45 insertions, 41 deletions

diff --git a/same-android/src/main/java/com/orbekk/same/GameView.java b/same-android/src/main/java/com/orbekk/same/GameView.java
index f3d018d..31ac27c 100644
--- a/same-android/src/main/java/com/orbekk/same/GameView.java
+++ b/same-android/src/main/java/com/orbekk/same/GameView.java
@@ -1,22 +1,16 @@
 package com.orbekk.same;
 
+import java.util.concurrent.atomic.AtomicBoolean;
+
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.orbekk.same.State.Component;
-import com.orbekk.same.ClientService;
-import com.orbekk.same.Client;
-import com.orbekk.same.UpdateConflict;
-
 import android.content.Context;
 import android.graphics.Canvas;
 import android.graphics.Paint;
 import android.view.MotionEvent;
 import android.view.SurfaceHolder;
 import android.view.SurfaceView;
-import android.widget.Toast;
-
-import java.util.concurrent.atomic.AtomicReference;
 
 public class GameView extends SurfaceView implements SurfaceHolder.Callback {
     private Logger logger = LoggerFactory.getLogger(getClass());
@@ -44,6 +38,7 @@ public class GameView extends SurfaceView implements SurfaceHolder.Callback {
         private Paint background;
         private Variable<Player> player;
         private VariableUpdaterTask<Player> updater;
+        private AtomicBoolean shouldRedraw = new AtomicBoolean(true);
         
         private Paint color = new Paint();
         
@@ -87,17 +82,36 @@ public class GameView extends SurfaceView implements SurfaceHolder.Callback {
         }
         
         @Override public void run() {
-            Canvas c = null;
-            try {
-                c = holder.lockCanvas();
-                synchronized(holder) {
-                    doDraw(c);
+            while (true) {
+                Canvas c = null;
+                try {
+                    c = holder.lockCanvas();
+                    synchronized(holder) {
+                        doDraw(c);
+                    }
+                } finally {
+                    holder.unlockCanvasAndPost(c);
+                }
+                synchronized (this) {
+                    if (Thread.interrupted()) {
+                        break;
+                    }
+                    try {
+                        while (!shouldRedraw.get()) {
+                            wait();
+                        }
+                    } catch (InterruptedException e) {
+                        break;
+                    }
                 }
-            } finally {
-                holder.unlockCanvasAndPost(c);
             }
         }
         
+        private synchronized void setShouldRedraw() {
+            shouldRedraw.set(true);
+            notifyAll();
+        }
+        
         private synchronized void setPosition(final float x, final float y) {
             if (player.get() == null || player.get().posX != x ||
                     player.get().posY != y) {
@@ -107,10 +121,10 @@ public class GameView extends SurfaceView implements SurfaceHolder.Callback {
         }
 
         @Override
-        public void valueChanged(Variable<Player> unused) {
+        public synchronized void valueChanged(Variable<Player> unused) {
             logger.info("Variable updated.");
             player.update();
-            run();
+            setShouldRedraw();
         }
     }
     
@@ -151,7 +165,7 @@ public class GameView extends SurfaceView implements SurfaceHolder.Callback {
     @Override
     public void surfaceDestroyed(SurfaceHolder holder) {
         logger.info("SurfaceDestroyed()");
-        // TODO: Stop thread.
+        thread.interrupt();
     }
     
     @Override
diff --git a/same-android/src/main/java/com/orbekk/same/SameControllerActivity.java b/same-android/src/main/java/com/orbekk/same/SameControllerActivity.java
index 59ec6e2..2fb0d25 100644
--- a/same-android/src/main/java/com/orbekk/same/SameControllerActivity.java
+++ b/same-android/src/main/java/com/orbekk/same/SameControllerActivity.java
@@ -106,8 +106,8 @@ public class SameControllerActivity extends Activity {
     
     private void joinNetwork(String masterUrl) {
         logger.info("joinNetwork({})", masterUrl);
-        Message message = Message.obtain(null, SameService.JOIN_NETWORK,
-            masterUrl);
+        Message message = Message.obtain(null, SameService.JOIN_NETWORK);
+        message.getData().putString("masterUrl", masterUrl);
         try {
             sameService.send(message);
         } catch (RemoteException e) {
diff --git a/same-android/src/main/java/com/orbekk/same/SameService.java b/same-android/src/main/java/com/orbekk/same/SameService.java
index b7ba0ca..f99eb68 100644
--- a/same-android/src/main/java/com/orbekk/same/SameService.java
+++ b/same-android/src/main/java/com/orbekk/same/SameService.java
@@ -24,9 +24,12 @@ import com.orbekk.same.config.Configuration;
 import com.orbekk.util.DelayedOperation;
 
 public class SameService extends Service {
-    public final static int DISPLAY_MESSAGE = 1;
     public final static int SEARCH_NETWORKS = 2;
     public final static int CREATE_NETWORK = 3;
+    
+    /**
+     * masterUrl: getData().getString("masterUrl")
+     */
     public final static int JOIN_NETWORK = 4;
     public final static int ADD_STATE_RECEIVER = 5;
     public final static int REMOVE_STATE_RECEIVER = 6;
@@ -87,21 +90,6 @@ public class SameService extends Service {
     class InterfaceHandler extends Handler {
         @Override public void handleMessage(Message message) {
             switch (message.what) {
-                case DISPLAY_MESSAGE:
-                    Toast.makeText(SameService.this,
-                        (String)message.obj, Toast.LENGTH_SHORT)
-                            .show();
-                    Messenger responseService = message.replyTo;
-                    if (responseService != null) {
-                        Message response = Message.obtain(null, DISPLAY_MESSAGE);
-                        response.obj = "Response from SameService";
-                        try {
-                            responseService.send(response);
-                        } catch (RemoteException e) {
-                            logger.error("Failed to respond.", e);
-                        }
-                    }
-                    break;
                 case SEARCH_NETWORKS:
                     logger.info("SEARCH_NETWORKS");
                     sameController.searchNetworks();
@@ -112,7 +100,7 @@ public class SameService extends Service {
                     break;
                 case JOIN_NETWORK:
                     logger.info("JOIN_NETWORK");
-                    String masterUrl = (String)message.obj;
+                    String masterUrl = message.getData().getString("masterUrl");
                     sameController.getClient().joinNetwork(masterUrl);
                     break;
                 case ADD_STATE_RECEIVER:
diff --git a/same-android/src/main/java/com/orbekk/same/StateViewerActivity.java b/same-android/src/main/java/com/orbekk/same/StateViewerActivity.java
index 5b8fd41..a6aa3ee 100644
--- a/same-android/src/main/java/com/orbekk/same/StateViewerActivity.java
+++ b/same-android/src/main/java/com/orbekk/same/StateViewerActivity.java
@@ -51,6 +51,7 @@ public class StateViewerActivity extends Activity {
         super.onStop();
         client.removeStateListener(stateListener);
         client.disconnect();
+        client = null;
     }
     
 }
diff --git a/same-android/src/main/java/com/orbekk/same/android/ClientInterfaceBridge.java b/same-android/src/main/java/com/orbekk/same/android/ClientInterfaceBridge.java
index 8ed38a7..72dfdb7 100644
--- a/same-android/src/main/java/com/orbekk/same/android/ClientInterfaceBridge.java
+++ b/same-android/src/main/java/com/orbekk/same/android/ClientInterfaceBridge.java
@@ -36,7 +36,7 @@ public class ClientInterfaceBridge implements ClientInterface {
     private int nextOperationNumber = 0;
     
     class ResponseHandler extends Handler {
-        @Override public void handleMessage(Message message) {
+        @Override public synchronized void handleMessage(Message message) {
             if (serviceMessenger == null) {
                 logger.warn("Ignoring message to disabled ResponseHandler.");
                 return;
@@ -119,7 +119,7 @@ public class ClientInterfaceBridge implements ClientInterface {
         this.context = context;
     }
     
-    public void connect() {
+    public synchronized void connect() {
         state = new State(".Temporary");
         Intent intent = new Intent(context, SameService.class);
         context.bindService(intent, serviceConnection, Context.BIND_AUTO_CREATE);
@@ -135,11 +135,12 @@ public class ClientInterfaceBridge implements ClientInterface {
         }
     }
     
-    public void disconnect() {
+    public synchronized void disconnect() {
         if (serviceMessenger != null) {
             disconnectFromService();
+        }
+        if (serviceConnection != null) {
             context.unbindService(serviceConnection);
-            state = null;
         }
     }