[.NET] Found issues and fixes

[at]

Hi. I have renamed this topic, as we post here information about different issues. I have corrected a bit SendXtMessage method in SmartFoxClient class. If I need send message without any parameters I want just to pass null as paramObj. But it causes NullReferenceException. So I added this string in the beginning of the method:

Code: Select all

if (paramObj==null) paramObj = new Hashtable();

The changed part of the method looks like this:

Code: Select all

public void SendXtMessage(string xtName, string cmd, Hashtable paramObj, string type, int roomId)
        {
            if (roomId == -1)
                roomId = activeRoomId;
               
            if (paramObj==null) paramObj = new Hashtable();

...


Now I can send messages without parameters just passing null as paramObj.

[ThomasLund]

Nice one. Adds some robustness - not critical for a beta 2 release, so will add that to the beta 3 todo list instead.

(Want Marco get the beta 2 out, so we can get more eyes on it instead of beta 1!! Hehe)

-----

Edit - made it into the beta 2 SVN anyways due to a waiting source commit that was going into beta 2.

[at]

First of all, thanks ThomasLund for the great work! Beta 2 is really cool! So let's continue to report found problems here to approach the day of Beta 3 release

In the new version I found a serious issue with writing data to sockets. When I start my Unity-based game I receive randomly many exceptions of type ObjectDisposedException in the WriteToSocket method. Here the part of code.

Code: Select all

               
                StreamWriter writer = new StreamWriter(socketConnection.GetStream());
                writer.Write(msg + (char)0);
                writer.Flush();


I always got the mentioned exception on the call of writer.Flush()

I don't know exactly the reason, but it seems that NetworkStream object that is received by socketConnection.GetStream() method are being somehow disposed by the GC. Maybe the reason is IDisposable interface implemented in this object.

I have solved this problem by making the NetworkStream a private field of the SmartFoxClient class. It's a solution that I found on .NET-related forums when people faced similar problems. I have also changed the code of HandleSocketData() method to make it working with common NetworkStream field, not with a local variable.

Here the diff

diff -c SmartFoxClient.cs new/SmartFoxClient.cs

Code: Select all

*** SmartFoxClient.cs   Tue Nov 11 14:21:51 2008
--- new/SmartFoxClient.cs   Tue Nov 11 14:21:56 2008
***************
*** 214,219 ****
--- 214,220 ----
 
          private Hashtable messageHandlers = new Hashtable();
          private TcpClient socketConnection;
+         private NetworkStream networkStream;
          const int READ_BUFFER_SIZE = 4096;
          private byte[] byteBuffer = new byte[READ_BUFFER_SIZE];
          private string messageBuffer = "";
***************
*** 1051,1056 ****
--- 1052,1058 ----
              {
                  socketConnection = new TcpClient(ipAddress, port);
                  connected = true;
+                 networkStream = socketConnection.GetStream();
                  thrSocketReader = new Thread(HandleSocketData);
                  thrSocketReader.Start();
                  HandleSocketConnection(this, new EventArgs());
***************
*** 3215,3221 ****
 
              try
              {
!                 StreamWriter writer = new StreamWriter(socketConnection.GetStream());
                  writer.Write(msg + (char)0);
                  writer.Flush();
              }
--- 3217,3223 ----
 
              try
              {
!                 StreamWriter writer = new StreamWriter(networkStream);
                  writer.Write(msg + (char)0);
                  writer.Flush();
              }
***************
*** 3560,3566 ****
              {
                  while (true)
                  {
!                     bytesRead = socketConnection.GetStream().Read(byteBuffer, 0, READ_BUFFER_SIZE);
                      if (bytesRead < 1)
                      {
                          // If we are already in a disconnect loop, then dont try to do another disconnection
--- 3562,3568 ----
              {
                  while (true)
                  {
!                     bytesRead = networkStream.Read(byteBuffer, 0, READ_BUFFER_SIZE);
                      if (bytesRead < 1)
                      {
                          // If we are already in a disconnect loop, then dont try to do another disconnection


Another minor issue - Firefox in OSX crashes on attempt to close the browser after Unity web-application has been using SFS.

[ThomasLund]

Hmmm - havent seen that one here. But good with a fix!

I will very soon start up for the (final?) beta and get the last issues nailed.

The main culprit seems to be failover to http - that still uses regular async instead of threads, and thus will crash Firefox and similar on shutdown.

Please hammer away on this all you can!

[ThomasLund]

HTTP / bluebox failover seems to work now for at least standalone games.

Replace the HttpConnection Send and HttpSend + receive methods with this:

Code: Select all

public void Send(string message)
        {
            if (connected || (!connected && message == HANDSHAKE) || (!connected && message == "poll"))
            {
                sfs.DebugMessage("[ Send ]: " + message + "\n");

                ThreadStart starter = delegate { HttpSend(message); };
                Thread t = new Thread(new ThreadStart(starter));
                t.IsBackground = true;
                t.Start();
            }
        }

        private void HttpSend(string message)
        {
            try
            {
                WebRequest request = HttpWebRequest.Create(webUrl);
                request.Method = "POST";
                request.ContentType = "application/x-www-form-urlencoded";

                string data = paramName + "=" + codec.Encode(this.sessionId, message);
                byte[] dataBytes = Encoding.ASCII.GetBytes(data);
                request.ContentLength = dataBytes.Length;
                request.GetRequestStream().Write(dataBytes, 0, dataBytes.Length);

                // Get and read the response.
                StreamReader reader = new StreamReader(request.GetResponse().GetResponseStream());
                string responseFromServer = reader.ReadToEnd();

                // Cleanup
                reader.Close();

                // Data is read now - lets process it unless the length is 0 (nothing recieved)
                // Handle handshake
                if (responseFromServer.Length != 0 && responseFromServer.Substring(0, 1) == HANDSHAKE_TOKEN)
                {
                    if (sessionId == null)
                    {
                        sessionId = codec.Decode(responseFromServer);
                        connected = true;

                        Hashtable parameters = new Hashtable();
                        parameters.Add("sessionId", this.sessionId);
                        parameters.Add("success", true);

                        if (OnHttpConnectCallback != null)
                            OnHttpConnectCallback(new HttpEvent(HttpEvent.onHttpConnect, parameters));

                    }
                    else
                    {
                        sfs.DebugMessage("**ERROR** SessionId is being rewritten");
                    }
                }

                // Handle data
                else
                {
                    // fire disconnection
                    if (responseFromServer.IndexOf(CONN_LOST) == 0)
                    {
                        if (OnHttpCloseCallback != null)
                            OnHttpCloseCallback(new HttpEvent(HttpEvent.onHttpClose, null));
                    }

                    // fire onHttpData
                    else
                    {
                        Hashtable parameters = new Hashtable();
                        parameters.Add("data", responseFromServer);
                        if (OnHttpDataCallback != null)
                            OnHttpDataCallback(new HttpEvent(HttpEvent.onHttpData, parameters));
                    }
                }
            }
            catch (Exception e)
            {
                Hashtable parameters = new Hashtable();
                parameters.Add("exception", e);
                if (OnHttpErrorCallback != null)
                    OnHttpErrorCallback(new HttpEvent(HttpEvent.onHttpError, parameters));
            }
        }


There is still a few problems left with this. If a Unity player has fallen back into http mode, then exiting it will crash the player. Something not being cleaned up properly. Will track that down. But the fallback mechanism itself now seems to work correctly.

There is also a problem with browser games. Closing e.g. a game in a tab, then reopening it and retrying (without shutting down browser) wont let it reconnect. Something static lingering around there escaping cleanup.

Will pick up pace now for beta 3 to get out the door, so we can finish this all up and move ahead from here

[ThomasLund]

OK - time to give things a "name".

There is a "reconnect issue". That is when in the webplayer you close a tab with a Unity player, open again and try to establish the connection again.

Then there is a "crash on close". After having run http communication, then browser crashes on exit (as well as the stand alone player).

On the reconnect issue I've tried printing out the caught exception - and wow thats a strange one.

[SFS DEBUG] Got this http exception: System.NotSupportedException: http://localhost:8080/bluebox/httpbox.do
at System.Net.WebRequest.GetCreator (System.String prefix) [0x00000]
at System.Net.WebRequest.Create (System.Uri requestUri) [0x00000]
at System.Net.WebRequest.Create (System.String requestUriString) [0x00000]
at SmartFoxClientAPI.Http.HttpConnection.HttpSend (System.String message) [0x00000]

Googling this doesnt give me any clue at all. Anyone ever seen one of these before and know what it means? Besides the obvious MSDN explaination. Obviously a WebRequest create from string works - so why does it throw an NotSupported exception the second time?


On the crash on close I think I have found the problem. WebRequest internally uses the async connection stuff - ARGH! This means that I cannot use the WebRequest class for http, as this will trigger the Unity/mono close crash bug that also haunted the socket connections in beta 1.

I got to think a bit about this, but it will basically mean that the only option here is to make a Unity specific failover using the Unity WWW class instead - which renders the API unusable for non-Unity clients. Bah

Another way to solve this would be to implement my own http class using bare sockets. Would be a portable solution, and HTTP protocol isnt that hard to code. Hmmm

[ThomasLund]

OK - I *think* I got http failover working now. Will during today run a few local tests here and then put up a snapshot build+sources for testing. Especially ppl with Windows machines should give this is go.

Had to code my own http client over raw sockets + make my own threaded pull mechanism. Using anything in the .NET platform that uses threads internally seems to make a crash on exit. Bah! At the moment I am cursing Unity for not upgrading to a less buggy mono version

If http failover now fully works (it does here on my Mac), then whats left is some of the browser issues with reconnecting and reloading a page - something not being properly cleaned up/restarted.

[ThomasLund]

Source code drop is available here:

http://www.fullcontrol.dk/downloads/Sma ... 081130.zip

There is also a build debug dll in bin/debug

Please give this a try on your projects to see if the HTTP failover works for everyone. It should also NOT crash on exiting the player.

As detailed above, it will still crash in the browser if you switch away from it (close tab, reload, restart) without shutting down the browser.

[ThomasLund]

Been looking a bit more at the reconnect issue, and have a hunch that its a socket problem.

Looking at the SFS console, you actually never see a real socket disconnection happen. Using netstat also shows the socket in TIME_WAIT.

I've now tried to play around with the Disconnect method a bit, and not really gotten around this. Seems no matter what I try (even stand alone player) I cannot get the socket disconnected message on the SFS console (not sure either that this can be used as a proper tool for debugging this issue).

Been trying to do things like:

thrConnect.Abort();
thrSocketReader.Abort();
networkStream.Close();
socketConnection.GetStream().Close();
socketConnection.Close();
socketConnection = null;
networkStream = null;

but it just doesnt trigger what I want *sigh*

Anyone here know some tricks on how to close the TcpClient so it actually does a total and hard disconnect that triggers a socket disconnection on the server side?

BTW - there is a subtle bug in Disconnect.

Change this

Code: Select all

                if (!connected)
                {


to this

Code: Select all

                if (connected)
                {


[at]

Been looking a bit more at the reconnect issue, and have a hunch that its a socket problem.

Looking at the SFS console, you actually never see a real socket disconnection happen. Using netstat also shows the socket in TIME_WAIT.

Hi, Thomas

On my sfs console I always see such messages on disconnection:
User [ java.nio.channels.SocketChannel[closed] ] removed

So, it seems like there is no problem with correct socket closing on the server side. But concerning the "reconnect issue" my collegues and me haven't still found a simple solution for it. No we are thinking of writing some javascript, that would send a message to Unity application before closing or navigating to another page.

[at]

An interesting thing happened: when I started using the latest SFS client from 2008-11-30 (I had been using beta2 before), I also stopped getting messages
User [ java.nio.channels.SocketChannel[closed] ] removed on disconnection.

But when I changed

Code: Select all

if (!connected)
                {

to

Code: Select all

if (connected)
                {

as you had mentioned before, the disconnection messages came back

[at]

A little change in SmartFoxClient class.

I found a few parts of such code:

Code: Select all

// If we are already in a disconnect loop, then dont try to do another disconnection
                        if (!connected)
                        {
                            DebugMessage("Disconnect due to lost socket connection");
                            Disconnect();
                        }


It's much better to use lock operator in Disconnect method to ensure that it won't be called if we are already in disconnect loop.

So I changed Disconnect method like this:

Code: Select all


Object disconnectionLocker = new Object();

...

public void Disconnect()
        {
           lock (disconnectionLocker) {
               if (!isHttpMode)
               {
                   if (connected)
                   {
                       try
                       {
                           socketConnection.Client.Shutdown(SocketShutdown.Both);
                           socketConnection.Close();
                       }
                       catch (Exception e)
                       {
                           DebugMessage("Disconnect Exception: " + e.ToString());
                       }
                       connected = false;
                   }
               }
               else
                   httpConnection.Close();
   
               // dispatch event
               HandleSocketDisconnection();
           }
        }


So we can just write Disconnect(); without performing any cheking because if it will be called once, the next call will end on condition connected == false.

Generally it's always nice using locks in C# to ensure that a cross-threaded method is not called at this time, not properties.

Here the diff:

diff -c SmartFoxClient.cs new/SmartFoxClient.cs

Code: Select all

*** SmartFoxClient.cs   Mon Dec  1 12:01:09 2008
--- new/SmartFoxClient.cs   Mon Dec  1 12:00:55 2008
***************
*** 1064,1069 ****
--- 1064,1071 ----
              }
              connecting = false;
          }
+         
+         Object disconnectionLocker = new Object();
 
          /**
           * <summary>
***************
*** 1088,1114 ****
           */
          public void Disconnect()
          {
!             if (!isHttpMode)
!             {
!                 if (!connected)
!                 {
!                     try
!                     {
!                         socketConnection.Client.Shutdown(SocketShutdown.Both);
!                         socketConnection.Close();
!                     }
!                     catch (Exception e)
!                     {
!                         DebugMessage("Disconnect Exception: " + e.ToString());
!                     }
!                     connected = false;
!                 }
!             }
!             else
!                 httpConnection.Close();
!
!             // dispatch event
!             HandleSocketDisconnection();
          }
 
          /**
--- 1090,1118 ----
           */
          public void Disconnect()
          {
!            lock (disconnectionLocker) {
!                if (!isHttpMode)
!                {
!                    if (connected)
!                    {
!                        try
!                        {
!                            socketConnection.Client.Shutdown(SocketShutdown.Both);
!                            socketConnection.Close();
!                        }
!                        catch (Exception e)
!                        {
!                            DebugMessage("Disconnect Exception: " + e.ToString());
!                        }
!                        connected = false;
!                    }
!                }
!                else
!                    httpConnection.Close();
!    
!                // dispatch event
!                HandleSocketDisconnection();
!            }
          }
 
          /**
***************
*** 3570,3581 ****
                      bytesRead = networkStream.Read(byteBuffer, 0, READ_BUFFER_SIZE);
                      if (bytesRead < 1)
                      {
!                         // If we are already in a disconnect loop, then dont try to do another disconnection

!                         if (!connected)
!                         {
!                             DebugMessage("Disconnect due to lost socket connection");
!                             Disconnect();
!                         }
                          return;
                      }
                      // Add the received byte message to the messageBuffer, so we can cut up that one                 
--- 3574,3581 ----
                      bytesRead = networkStream.Read(byteBuffer, 0, READ_BUFFER_SIZE);
                      if (bytesRead < 1)
                      {
!                         DebugMessage("Disconnect due to lost socket connection");
!                         Disconnect();
                          return;
                      }
                      // Add the received byte message to the messageBuffer, so we can cut up that one                 
***************
*** 3624,3635 ****
              }
              catch (Exception e)
              {
!                 // If we are already in a disconnect loop, then dont try to do another disconnection
!                 if (!connected)
!                 {
!                     DebugMessage("Disconnect due to: " + e.Message);
!                     Disconnect();
!                 }
              }
          }
 
--- 3624,3631 ----
              }
              catch (Exception e)
              {
!                 DebugMessage("Disconnect due to: " + e.Message);
!                 Disconnect();
              }
          }
 
***************
*** 4018,4024 ****
 
          /**
           */
!         protected virtual void Dispose(bool disposing)
       {
         if (!this.isDisposed) // only dispose once!
         {
--- 4014,4020 ----
 
          /**
           */
!       protected virtual void Dispose(bool disposing)
       {
         if (!this.isDisposed) // only dispose once!
         {
***************
*** 4026,4036 ****
           {
               // Dispose managed resources here
           }
!          // perform cleanup for unmanaged resources here
!           if (connected)
!           {
!               Disconnect();
!           }
            this.isDisposed = true;
          }
       }
--- 4022,4028 ----
           {
               // Dispose managed resources here
           }
!          Disconnect();
            this.isDisposed = true;
          }
       }


[ThomasLund]

Great - will get that in locally and test later today.

Could you please run a quick test on the http fallback too - see if it works fully in your end?!?!

Its not fast, but it works for me.

[ThomasLund]

OK - I think this is it now. Found the problem with the reload issue - but what an annoying little bugger.

It is a problem in the mono version that Unity uses on OSX. This means that there is nothing I can do.

Fortunately there is a workaround that I implemented, but it also adds a restriction to the SFS API usage - you can only use IP numbers for host in your connection. You cannot use hostnames.

This is maybe a minor thing for most ppl, but it has some irritating implications in some loadbalancing/cluster scenarios. But there is not much I can do about it until Unity uopgrades to a more recent mono version.

Here locally in my build I have now a non-crashing, fully featured version that runs with http fallback and sockets - in safari+firefox webplayer as well as standalone - tested on OSX, but soon also Windows.

Will do some more tests tomorrow, and then do another codedrop for anyone interested!

/Thomas

[Lapo]

This is maybe a minor thing for most ppl, but it has some irritating implications in some loadbalancing/cluster scenarios.

Just wanted to drop a small note: actually it's not a big problem with clustering. In our whitepaper for the Terracotta integration we exclusively use IP addresses. The load balancing is actually performed on the server side which returns the right IP for the client socket connection.

For those interested to read the whole thing -> http://www.smartfoxserver.com/clustering/