Does Calling a function that in turn calls a coroutine block the thread until a result is returned?

Unity Engine
,
1

Hey all,

I’m trying to wrap my head around Unity’s coroutine system and I might be missing something here.

I am creating an SDK that interacts with a restful web service and I want to abstract away as much complexity from the end-user (developer) as possible.

My implementation works, but it just doesn’t sit right with me so I would value your input.

I have a generic GET function that calls a coroutine and returns the result via call back like this:

public ResponseResult Get(string path)
        {
            ResponseResult result = new ResponseResult();
            
            StartCoroutine(Get_Coroutine(path, tmp =>
            {
                result = tmp;
            }));

                return (result);
            }
        }

        static IEnumerator Get_Coroutine(string path, System.Action<ResponseResult> callback = null)
        {
            using (UnityWebRequest request = UnityWebRequest.Get(Settings.URI + path))
            {
                ResponseResult tmp = new ResponseResult();

                request.SetRequestHeader("Content-type", "application/json");
                 request.SendWebRequest();
                do { } while (!request.isDone); [COLOR=#0080ff]// This makes me cringe!![/COLOR]
                tmp.Json = request.downloadHandler.text;
                tmp.Error = request.error;
                tmp.ResponseCode = request.responseCode;
                tmp.Result = request.result.ToString();
                callback(tmp);
                yield return new WaitForEndOfFrame();[COLOR=#0080ff] // Found this in some code online for using callbacks in coroutine - pretty sure it should just return null though. [/COLOR]
            }
        }

Then in order to allow the user to just do a simple call to 'get whatever object they want I have simple methods like this:

public (Locale locale, ResponseResult responseResult) GetLocale ()
        {

            var result = Get("locale");
               
             Locale locale = JsonUtility.FromJson<Locale>(result.Json);

            return (locale, result);
        }

The above example exposes a method to return a serialized ‘Locale’ object from the Json body of the webrequest. It also returns the responseResult object (tuple) to allow custom error handling not covered by the global error handler.

So firstly, I’m really uncomfortable having to use do { } while (!request.isDone); in GetCoroutine, but what I have read seems to indicate that my usage is correct and this is not blocking code. It just feels like a huge code smell to me. If I leave it out, the coroutine often returns without completing, resulting in no data.

More importantly though, does GetLocale () calling Get(string path) without doing so in its own coroutine cause thread blocks?

What about when the end-user calls GetLocale ()? The same question, does he need to call it in a coroutine?

I am really hoping to abstract away all complexity so would really like to avoid making the end-user use coroutines if possible.

Thanks for your time.

Dave

2

Don’t do that. Just yield the request.SendWebRequest();

Coroutines aren’t every much… Coroutines in a nutshell:

https://discussions.unity.com/t/825667/6

https://discussions.unity.com/t/749264/9

This construct:

Will immediately return.

You need to lift your result-setting delegate up and out of there, provide a way for Get to accept such a delegate.

3

Thanks for the response Kurt. I’ll take a look at those two links.

Appreciate it.

4

Huh … your while(!done) {} should freeze Unity, every time. Unlike threads, Coroutines don’t get a context switch – Unity will never say “well, we’ve done that loop long enough, let’s run something else”. Infinite loops freeze you every time, anywhere. Coroutines need to explicitly give up control (for 1+ frames) with a yield command: the standard coroutine busy-wait is while(!done) yield return null; (the last thing is the intended shortcut for WaitForNextFrame).

5

I’ve seen this pattern before. I think a started network request can still bang the .isDone property from the network thread, so the primary downside is that your game locks until the request completes or times out.

6

Ah … found a mention on StackOverflow (after not finding it in the Unity manual). A comment – not even an Answer – says that yes, functions in UnityWebRequest run in their own threads.

7

Yeah, the coroutine is not locking up the game even with the ‘while’ in there. It just feels very very wrong!

The loop is actually not infinite there is a built-in timeout in UnityWebRequest, but even still there seems to be no lockup at all even for the couple of seconds the request takes on a successful call.

These coroutines are starting to make a bit more sense to me but I’m still not quite grasping the whole picture, I think.
I’ll play around with some of the concepts in Kurts’ linked posts tomorrow and hopefully, I’ll trigger a lightbulb moment.

Thanks for all the responses.

8

I found this method under Unity’s custom yield documentation: WaitUntil Unity - Scripting API: WaitUntil

Perhaps that would be a better way of handling this?

9

Actually that is wrong ^^. The coroutine would look up because you have an empty while loop that busy spins until the request has finished. You do not yield the execution so you’re stuck in that while loop synchonously. This makes the whole point of a coroutine pointless. You only yield at the very end and nothing else happens after that, so it’s pointless for it to be a coroutine.

You can not somehow magically make a process that takes a relatively long time synschonous without lockup up the whole process. If you want the rest of the application to continue running, you have to work with callbacks. You have a callback in your coroutine but you just use it to synchronously passing the value back. Your whole approach will not work, no matter how you twist it.

We don’t know how or where you want to use your GetLocale method. However that code would have to change as well to actually work with an asynchonous method. Either through callbacks or async / await. However for that it’s necessary to know how the GetLocale method is used.

10

Morning all.

So I’ve finally figured this all out (I think).

The short answer to the original question is that in order to call a function that implements a coroutine, the calling function also needs to implement a coroutine or some form of delegate mechanism that gets called back once all is completed. Pretty obvious once you get a grasp on how coroutines actually work, but as I said I was really hoping to abstract that away, but that’s just not how it works.

Curiously though, there seems to be a lot of strong feelings (my own included) around the use of the while loop. My logic is screaming NO!, but the reality is that this is not locking up the game. To clarify, if I call the coroutine (without chaining the call from my ‘abstraction methods’), ie, just by calling it with the standard StartCoroutine, even with forcing an open-ended loop that can never complete, the UI remains responsive and the game continues. It really looks like this is running in another thread somewhere, which I have seen hints at on various posts around the web and was also mentioned by @Owen-Reynolds .

However, once I call the method that starts the coroutine without the calling one implementing a delegate, then the game locks up like it ate too much pizza! This is kind of obvious since the main game loop is now waiting for an answer.

Regardless, my implementation was flawed and the loop was a hack to fix something I didn’t understand. So the code smell I was getting was still very valid.

Thank you all for your time and input.

Dave

1 Like
11

Many coroutines just quit when done. For example, thy spawn 5 flowers over 5 seconds and and simply stop. For one’s that need to set a “done” flag or give a result, they have full access to globals. Or, most are non-static, giving them access to class variables. They can simply set the non-local madeAllFlowers=true;. Passing in actionOnStop = ()=>{madeAllFlowers=true;} is cool, but overly complicates a typical coroutine.

Unity mainly uses them as an easy way to do something slowly over a few frames. while(not full color) { increase color; yield return null; } is a typical use. Making a web request could be handled by a coroutine, but adding if(webReqVar!=null && webReqVar.isDone) to Update would probably work.

12

I had thought about going that route, but unfortunately there is a very real possibility that the end user has multiple simultaneous requests to different endpoints on the api from within the same script. Determining which one completed could get unwieldy fast.

I’ve come to terms with not being able to do without delegates, I’ll now focus my energies making the implementation for the end user as painless as possible.
The SDK will be for a very specific REST API, so I will give them a very good sample of working calls for them to copy and paste where required.

Thanks again.

13

If you really want to make it easy for users, wrap it up in some self-contained objects (one for each API endpoint) with factories to make them, which ensures they have everything they need to “do business.”

I almost never scatter AddComponent() around my codebases because it makes a mess. Rather I use the above pattern and keep it all centralized in the object that cares about that . Here’s a concrete example:

Factory Pattern in lieu of AddComponent (for timing and dependency correctness):

https://gist.github.com/kurtdekker/5dbd1d30890c3905adddf4e7ba2b8580

14

Thanks, Kurt, that is a really great idea!

Cheers

Dave