From 6cb691592db9eb9f6bd40901c503777b25e42bbf Mon Sep 17 00:00:00 2001
From: Paul Speed <pspeed42@users.noreply.github.com>
Date: Sat, 26 Dec 2015 17:56:31 -0500
Subject: [PATCH] Fixed two potential race conditions in the DefaultClient. In
 one case, closing a client while it was already closing on another thread
 (say because the server is shutting down and you are exiting at the same
 time) would cause an NPE if you caught it just right.  Now the thing checking
 and setting the connection state is synchronized to avoid the race.

The other more subtle one was caused by sending out the 'connected'
event before the services were all started.  It's quite common for
application code to start doing stuff when the 'connected' event comes
through like sending messages and stuff.  If the services hadn't been
fully started then even the serializers might not be registered yet...
and that = bad.

Now the client doesn't send the 'connected' event until the services
are started.  This should be safe and one could argue that it's more
'correct' but there is some small chance that it screws up certain
use-cases.  However, if a real use-case comes up that's not solved by
a service then we can always add some kind of prestarted event.
---
 .../com/jme3/network/base/DefaultClient.java  | 53 +++++++++++--------
 1 file changed, 31 insertions(+), 22 deletions(-)

diff --git a/jme3-networking/src/main/java/com/jme3/network/base/DefaultClient.java b/jme3-networking/src/main/java/com/jme3/network/base/DefaultClient.java
index 3e76f8be7..94a951e52 100644
--- a/jme3-networking/src/main/java/com/jme3/network/base/DefaultClient.java
+++ b/jme3-networking/src/main/java/com/jme3/network/base/DefaultClient.java
@@ -301,35 +301,38 @@ public class DefaultClient implements Client
 
     protected void closeConnections( DisconnectInfo info )
     {
-        if( !isRunning )
-            return;
+        synchronized(this) {
+            if( !isRunning )
+                return;
 
-        if( services.isStarted() ) {
-            // Let the services get a chance to stop before we
-            // kill the connection.
-            services.stop();
-        }
+            if( services.isStarted() ) {
+                // Let the services get a chance to stop before we
+                // kill the connection.
+                services.stop();
+            }
         
-        // Send a close message
+            // Send a close message
     
-        // Tell the thread it's ok to die
-        for( ConnectorAdapter ca : channels ) {
-            if( ca == null )
-                continue;
-            ca.close();
-        }
+            // Tell the thread it's ok to die
+            for( ConnectorAdapter ca : channels ) {
+                if( ca == null )
+                    continue;
+                ca.close();
+            }
         
-        // Wait for the threads?
+            // Wait for the threads?
 
-        // Just in case we never fully connected
-        connecting.countDown();
+            // Just in case we never fully connected
+            connecting.countDown();
         
-        fireDisconnected(info);
+            isRunning = false;
         
-        isRunning = false;
+            // Terminate the services
+            services.terminate();            
+        }
         
-        // Terminate the services
-        services.terminate();            
+        // Make sure we aren't synched while firing events
+        fireDisconnected(info);        
     }         
 
     @Override
@@ -462,11 +465,17 @@ public class DefaultClient implements Client
                 this.id = (int)crm.getId();
                 log.log( Level.FINE, "Connection established, id:{0}.", this.id );
                 connecting.countDown();
-                fireConnected();
+                //fireConnected();
             } else {
                 // Else it's a message letting us know that the 
                 // hosted services have been started
                 startServices();
+ 
+                // Delay firing 'connected' until the services have all
+                // been started to avoid odd race conditions.  If there is some
+                // need to get some kind of event before the services have been
+                // started then we should create a new event step.               
+                fireConnected();
             }
             return;
         } else if( m instanceof ChannelInfoMessage ) {