0

Here is my code:

public void ConnectToWorldServer()
{
    if (socketReady)
    {
        return;
    }
    //Default host and port values;
    string host = ClientWorldServer.ServerIP;
    int port = ClientWorldServer.TCPPort;

    //ClientLoginServer ClientLoginServer = new ClientLoginServer();


    try
    {

        socket = new TcpClient(host, port);
        stream = socket.GetStream();
        socket.NoDelay = true;
        writer = new StreamWriter(stream);
        reader = new StreamReader(stream);
        socketReady = true;
        //Preserve the connection to worldserver thrue scenes
        UnityThread.executeInUpdate(() =>
        {
            DontDestroyOnLoad(worldserverConnection);
        });

        // Start listening for connections.
        while (true)
        {
            if (socketReady)
            {
                if (stream.DataAvailable)
                {
                    string sdata = reader.ReadLine();
                    if (sdata != null)
                    {

                        Task<JsonData> jsonConvert = Task<JsonData>.Factory.StartNew(() => convertJson(sdata));
                        UnityThread.executeInUpdate(() =>
                        {
                            OnIncomingData(jsonConvert.Result);
                        });
                    }
                }
            }
        }
    }
    catch (Exception e)
    {
        Debug.Log("Socket error : " + e.Message);
    }

}

private JsonData convertJson(string data)
{
    return JsonConvert.DeserializeObject<JsonData>(data);    
}

What I am wondering now is does this part of the code:

UnityThread.executeInUpdate(() =>
{
    OnIncomingData(jsonConvert.Result);
});

block until this task returns back a result:

Task<JsonData> jsonConvert = Task<JsonData>.Factory.StartNew(() => convertJson(sdata));

I am really not that familiar with Tasks. My goal is to run the json conversion and then execute OnIncomingData(jsonConvert.Result);.

I think my code is not doing that. Why?

Paul Wheeler
  • 18,988
  • 3
  • 28
  • 41
Venelin
  • 2,905
  • 7
  • 53
  • 117
  • 2
    Exception handling is a pet peeve of mine, and yours is bad. You catch fatal exceptions and only log the message. There are two articles I link often on the mater. When you get some time, they are worth a read: https://blogs.msdn.microsoft.com/ericlippert/2008/09/10/vexing-exceptions/ | https://www.codeproject.com/Articles/9538/Exception-Handling-Best-Practices-in-NET – Christopher Nov 27 '19 at 21:09
  • 2
    Upon closer inspection, you also do not seem to clean up your Unmanaged Resources. `socket` and `stream ` are the standout variable with that issue. – Christopher Nov 27 '19 at 21:11
  • Side note: parsing one line of JSON on separate thread will likely be more expensive than just doing it on the same thread... but ohhh well threads need to be used to just be used. :) – Alexei Levenkov Nov 27 '19 at 21:50
  • @AlexeiLevenkov it is a task not a thread. – Venelin Nov 27 '19 at 21:52
  • @VenelinVasilev You mean tasks don't need thread to run on or something else? In regular framework default scheduler will run task on separate thread in threadpool with all related extra synchronization overhead (in additional to scheduling overhead)... I don't know if Unity3d does the same or uses a scheduler that runs all tasks on the main thread but still there is non-trivial scheduling overhead ... – Alexei Levenkov Nov 27 '19 at 22:18
  • @AlexeiLevenkov I think it pretty much depends on the size of the JSON. In Unity you want to max out the framerate .. so push on a thread whatever you can! If this causes the overall task to take some milliseconds longer it is more acceptable then having one or multiple frames taking longer ;) – derHugo Nov 28 '19 at 06:33

2 Answers2

1

When a thread invokes Task.Result it will block until the Task completes, either by returning a value, throwing an exception, or being canceled. From the documentation:

Accessing the property's get accessor blocks the calling thread until the asynchronous operation is complete; it is equivalent to calling the Wait method.

So, to be clear, calling Task<JsonData>.Factory.StartNew creates a Task (which represents some computation to be executed), and schedules it for execution (when it gets executed and on what thread is up to the default TaskScheduler, but StartNew should return immediately). Your call to UnityThread.executeInUpdate will then happen without waiting for the Task you created to complete. At the point where UnityThread calls the anonymous function you passed to executeInUpdate that thread will block until the Task completes. I'm not familiar with UnityThread.executeInUpdate so I cannot tell you whether it will block until that callback completes or not.

One thing to be aware of is that depending on how Unity works with threads, it is possible to create a deadlock by accessing the Result property. In some cases a Task will attempt to use a specific context to execute, and if you cause that context to block waiting for the Task to complete, it will never get a chance to run: https://blog.stephencleary.com/2012/07/dont-block-on-async-code.html

Paul Wheeler
  • 18,988
  • 3
  • 28
  • 41
1

If you want to wait for the result then what is the point of using the Task. The right way of doing thing asynchronously is making your function async all the way.

public async void ConnectToWorldServer()
{
   .....
   .....
// Here await will put this method call on a queue to finish later and will return from this method. 
         Task<JsonData> jsonConvert = await Task<JsonData>.Factory.StartNew(() => convertJson(sdata));
// After the task is finished, it will resume to this method here to execute next statement. 
         UnityThread.executeInUpdate(() =>
         {
            OnIncomingData(jsonConvert.Result);
         });
   .....
   .....
}
vendettamit
  • 14,315
  • 2
  • 32
  • 54
  • I guess you are referring to [this](https://stackoverflow.com/questions/41330771/use-unity-api-from-another-thread-or-call-a-function-in-the-main-thread)? Would be good to include it in the answer since `UnityThread` is not a built-in type... – derHugo Nov 28 '19 at 06:36
  • I used the code from OP's question. The answer is to point out the correct usage of asynchronous code. – vendettamit Nov 29 '19 at 16:08