koobas.hobune.stream
Problem with Stopping Nested Coroutines: Control Never Returned to Parent
Hello!
I am having a problem with stopping a nested Coroutine. It seems that when a parent Coroutine yields to a child and then StopCoroutine() is used to stop the child, that yield never returns and the parent is either permanantly stuck or is just stopped (not sure which). This feels counterintuitive, and I feel like there has to be a solution to this problem, but I can't figure it out at all.
Here is an example class I made to highlight this:
using UnityEngine;
using System.Collections;
public class CoroutineTest : MonoBehaviour
{
private Coroutine _coLoop;
public void Awake()
{
StartCoroutine(CoMain());
StartCoroutine(CoInterrupt());
}
public IEnumerator CoMain()
{
Debug.Log("Start Main");
_coLoop = StartCoroutine(CoLoop());
yield return _coLoop;
Debug.Log("End Main");
}
public IEnumerator CoLoop()
{
Debug.Log("Start Loop");
while (Time.time < 2) {
yield return null;
}
Debug.Log("End Loop");
}
public IEnumerator CoInterrupt()
{
yield return new WaitForSeconds(1);
StopCoroutine(_coLoop);
Debug.Log("Interrupt Loop!");
}
}
I would this this to print
Start Main, Start Loop, Interrupt Loop!, End Main
but the return is
Start Main, Start Loop, Interrupt Loop!
and the yield within CoMain() is never returned from, and thus execution stops. I am asking for a solution to this problem without having to use a custom Coroutine implementaton, if possible.
Thank you for any help you can offer. I really appreciate it.
Firstly, + 1 for a well-written question :)
I think this is expected behaviour. Co$$anonymous$$ain()'s execution is paused until CoLoop() yields, and since CoLoop is stopped, this never happens. So Co$$anonymous$$ain() enters an infinite state of waiting for a yield that isn't going to come. CoLoop doesn't yield arbitrarily at the point it is stopped, and I can't find any documentation to suggest why this should happen.
Rather than "solve" this problem (which while being an excellent, clear example, is somewhat artificial), what's the actual scenario in the game you're trying to achieve?
Thank you for your response.
A summary of what I'm trying to do is that I have entities with AI routines attached to them. The routines deter$$anonymous$$e when to move, when to use abilities, etc. If the entity is using an ability, it yields to that ability's routine, which may take a few seconds to end, depending on a few factors within the game. If the entity's use of that ability is interrupted before the ability finishes, the parent AI routine stops execution, and the entity just sits idle for the rest of play.
A heavily abstracted example:
IEnumerator AIRoutine()
{
while (!canCast) {
$$anonymous$$ove();
yield return null;
}
yield return ability.Cast();
}
class Ability {
IEnumerator Cast()
{
yield return new WaitForSeconds(castTime);
//do ability stuff
}
void Interrupt()
{
StopAllCoroutines();
}
}
When Interrupt() is called on an Ability, AIRoutine() stops executing. If this is intended behavior, then how can what I want be accomplished? Is looping over yield return null; and checking if the Ability should be interrupted rather than WaitForSeconds() a better solution? $$anonymous$$aybe I'm wrong, but that feels really expensive if you have dozens or even hundreds of Ability routines running at once, no?
First of all, coroutines fundamentally run independent from each other. When you call StartCoroutine Unity registers a new coroutine which will take it's place in some internal list. It you "yield" on another Coroutine, that coroutine is basically on hold until the "nested" coroutine finishes. It's possible that they implemented the continuation with some sort of callback which gets invoked inside the coroutine scheduler when the nested coroutine finishes. A coroutine is finished when MoveNext returns false.
However stopping the coroutine completely breaks this mechanism since the coroutine is simply kicked out of the internal list. So the "outer" coroutine still waits for it's signal to continue which will never be send.
As i said in a comment on the other question, stopping a coroutine should in general be avoided. While it's a lot saver than terminating a thread from the outside (since a coroutine can only be stopped at yields) it's still a bit unpredictable in which state the coroutine is at the moment. You should implement the termination into the nested coroutine and let it finish gracefully (i.e. yield break;)
One option would be to run the nested coroutine "inline" without actually starting a new coroutine.
bool terminateNested = false;
public IEnumerator CoMain()
{
Debug.Log("Start Main");
terminateNested = false;
IEnumerator nested = CoLoop(); // no StartCoroutine!!!
while(!terminateNested && nested.MoveNext())
yield return nested.Current;
Debug.Log("End Main");
}
Here we iterate the IEnumerator manually. We simply yield the yielded value of the nested coroutine, so it has the same effect. You can simply set "terminateNested" to "true" and the nested coroutine will "terminate". However the last yield will still complete. So if the nested coroutine yields a "WaitForSeconds(1000)" CoMain can not continue until that wait is done.
Another solution is to build a wrapper around the IEnumerator of the nested coroutine and provide a seperate way of stopping the coroutine. The wrapper would simply return another controlling coroutine which you actually yield on in your CoMain. When we receive the terminate signal we stop the "wrapped" coroutine and let the controlling coroutine terminate gracefully.
Something like that:
public class StoppableCoroutine
{
bool terminated = false;
IEnumerator payload;
Coroutine nested;
MonoBehaviour mb;
public StoppableCoroutine(MonoBehaviour mb, IEnumerator aCoroutine)
{
payload = aCoroutine;
nested = mb.StartCoroutine(wrapper());
this.mb = mb;
}
public Coroutine WaitFor()
{
return mb.StartCoroutine(wait());
}
public void Stop()
{
terminated = true;
mb.StopCoroutine(nested);
}
private IEnumerator wrapper()
{
while (payload.MoveNext())
yield return payload.Current;
terminated = true;
}
private IEnumerator wait()
{
while(!terminated)
yield return null;
}
}
public statoc class MonoBehaviourExtension
{
public static StoppableCoroutine StartCoroutineEx(this MonoBehaviour mb, IEnumerator coroutine)
{
return new StoppableCoroutine(mb, coroutine);
}
}
With that class and extension method you can simply do this:
StoppableCoroutine _coLoop;
public IEnumerator CoMain()
{
Debug.Log("Start Main");
_coLoop = StartCoroutineEx(CoLoop());
yield return _coLoop.WaitFor();
Debug.Log("End Main");
}
To actually stop the nested coroutine you would simply call
_coLoop.Stop();
This approach has several advantages. Now you can have multiple coroutines to wait for a single one which isn't possible with the Coroutine object. You can only yield a Coroutine object once. Since our WaitFor method creates a new coroutine each time you call it that problem doesn't exist.
Finally a link to my CoroutineHelper class which does something similar.
Not using StopCoroutine() anymore makes sense. Thank you. It seems strange that the outer Coroutine can be put in the position of waiting for a signal it will never get. Does it sit in the Coroutine scheduler indefinitely? Is that a memory use concern?
What I ended up doing is I replaced WaitForSeconds() with something similar to this:
float endTime = Time.time + timeToWait;
while (Time.time < endTime) {
if (interrupt) {
yield break;
}
yield return null;
}
I think this is functionally pretty similar to your example, but I think I like yours better. Thanks again!
Yes, the coroutine would most likely sit in the scheduler and keep the memory of all related classes alive. It should go away when you call StopAllCoroutines or when you destroy the object the coroutine runs on.
Bunny, your work here is fantastic! It has made my life a lot easier.
I wanted to point out a bug I found in it.
nested = mb.StartCoroutine(wrapper());
That line of code should be in "WaitFor()| rather than the constructor. The reason is that if someone were to call this line of code...
_currentTest$$anonymous$$ethod = $$anonymous$$onoBehaviourExtension.StartCoroutineEx(instance, _currentTestEnumerator);
... the variable "_currentTest$$anonymous$$ethod" will not actually be set until "mb.StartCoroutine(wrapper());" is complete.
The use case where this caused an issue for me is in a test automation framework. One test after another is being launched. When an assertion fails, a property retrieves "_currentTest$$anonymous$$ethod" and "Stop()" is called on this object. However, since the test method is not complete, the constructor has still not returned the constructed StoppableCoroutine, so the "_currentTest$$anonymous$$ethod" variable is still set to the previous test's StoppableCoroutine.
Since that method is long gone, it tries to stop an already destroyed object. This throws an error, of course. It is not a fatal error, but it took some time to understand the root cause.
After I move that line of code, everything works as I expect it to.
Well, that's a very specific case and yes it will cause a bug, but it's not a bug in this class since the behaviour is pretty clear. The problem is that Unity immediately runs the coroutine up to the first yield. One simply solution is to place a simple "yield return null;" right at the start of the coroutine to make it wait one frame. Another way would be to use a second internal coroutine which will add that extra yield before the actual coroutine is started.
You shouldn't start the actual coroutine in "WaitFor" as WaitFor is designed in a way so it can be used in multiple coroutines. If you want to defer the launch of the coroutine you can add a Start method (just like the Thread class has one). That would be a cleaner solution.
I'm not sure if there is a chance to still get some answer from you but what pros and cons are in stopping parent coroutine in such scenario? Would you recommend it to prevent leaks?
ex. StartCorotouine(ParentIEnum = ParentCoroutine())
and later if(ParentIEnum != null) { StopCoroutine(ParentIEnum); ParentIEnum = null; }
This actually is a Unity drama, stopping a coroutine without yielding nothing and blocking the "yield return StartCoroutine". A solution would be, to never use "yield return StartCoroutine(aCoroutine)" if that aCoroutine is to be stopped with StopCoroutine() at some point, rather simply StartCoroutine(aCoroutine), and after that "yield return null" until that aCoroutine gets null:
IEnumerator coroutineMethod;
void Start(){
StartCoroutine(MainCoroutine());
}
void Update(){
if(conditionToStopCoroutine){
conditionToStopCoroutine = false;
if(null != coroutineMethod){
StopCoroutine(coroutineMethod);
coroutineMethod = null;
}
}
}
IEnumerator MainCoroutine(){
coroutineMethod = CoroutineMethod();
StartCoroutine(coroutineMethod);
while(coroutineMethod != null){
yield return null;
}
//so the code that follows this should be executed propely
Debug.LogError("Sipvay Barmane!!!");
}
IEnumerator CoroutineMethod(){
// ..
// ..
// ..
//last statement:
coroutineMethod = null;
}
This is quite an old thread but writing this answer here because Bunny83's code gave me some insight!
So, I wanted a more flexible nestable coroutine so that extended @Bunny83 's class with IEnumerator.
Code :
public class StoppableCoroutine
{
MonoBehaviour monoParent;
Coroutine wrapper;
NestableCoroutine nestableCoroutine;
bool terminated = false;
public StoppableCoroutine(MonoBehaviour monoParent, IEnumerator initialCoroutine)
{
this.monoParent = monoParent;
nestableCoroutine = new NestableCoroutine(initialCoroutine);
wrapper = monoParent.StartCoroutine(Run());
}
public void Stop()
{
terminated = true;
monoParent.StopCoroutine(wrapper);
}
public IEnumerator StartNested(IEnumerator newCoroutine)
{
nestableCoroutine.Add(newCoroutine);
while(nestableCoroutine.IsCoroutineDone(newCoroutine) == false)
{
yield return null;
}
}
private IEnumerator Run()
{
//Wait a frame so that user can safely access this instance inside 'initialCoroutine'
//JUST AFTER constructor returns.
yield return null;
while (!terminated && nestableCoroutine != null && nestableCoroutine.MoveNext())
yield return nestableCoroutine.Current;
}
internal class NestableCoroutine : IEnumerator
{
List<IEnumerator> all;
IEnumerator latestCoroutine
{
get
{
if(all == null || all.Count == 0)
{
return null;
}
else
{
return all[all.Count - 1];
}
}
}
public NestableCoroutine(IEnumerator first)
{
all = new List<IEnumerator>();
all.Add(first);
}
public void Add(IEnumerator recent)
{
all.Add(recent);
}
public bool IsCoroutineDone(IEnumerator coroutineToCheck)
{
return all.Contains(coroutineToCheck);
}
public object Current
{
get
{
if (latestCoroutine == null)
{
return null;
}
else
{
return latestCoroutine.Current;
}
}
}
public bool MoveNext()
{
if (latestCoroutine == null)
{
return false;
}
else
{
bool canMoveNext = latestCoroutine.MoveNext();
if (canMoveNext == false)
{
all.RemoveAt(all.Count - 1);
if(all.Count == 0)
{
return false;
}
else
{
return true;
}
}
else
{
return canMoveNext;
}
}
}
public void Reset()
{
}
}
}
With this you can nest coroutine at any depth as you want, and there's no need to keep track of every StoppableCoroutine class instances - Only initial one.
Sample Code:
using System.Collections;
using System.Collections.Generic;
using UnityEngine;
public class Test_StoppableCoroutine : MonoBehaviour
{
StoppableCoroutine stoppableCoroutine;
// Update is called once per frame
void Update ()
{
if(Input.GetKeyDown(KeyCode.S))
{
stoppableCoroutine = this.StartStoppableCoroutine(Outer());
}
else if (Input.GetKeyDown(KeyCode.Q))
{
Debug.Log(Time.time + " / Q Pressed!");
stoppableCoroutine.Stop();
}
}
IEnumerator Outer()
{
Debug.Log(Time.time + " / Start Outer");
yield return stoppableCoroutine.StartNested(Inner());
Debug.Log(Time.time + " / End Outer");
}
IEnumerator Inner()
{
int count = 0;
while (count < 5)
{
Debug.Log("Inner running...");
yield return new WaitForSeconds(0.5f);
count++;
}
yield return stoppableCoroutine.StartNested(InnerInner());
}
IEnumerator InnerInner()
{
while (true)
{
Debug.Log("InnerInner running...");
yield return new WaitForSeconds(0.5f);
}
}
}
If there's any mistakes or caveats I missed, Please let me know!
