Re: [Qemu-devel] [PATCH] qemu-coroutine: fix qemu_co_queue_run_restart error

[Bin Wu]

sorry, there is a mistake in this patch: the "ret" variable is not
defined :<

I will send a new patch to fix this problem.

On 2015/2/9 12:09, Bin Wu wrote:
> From: Bin Wu <address@hidden>
> 
> The error scenario is as follow: coroutine C1 enters C2, C2 yields
> back to C1, then C1 ternimates and the related coroutine memory
> becomes invalid. After a while, the C2 coroutine is entered again.
> At this point, C1 is used as a parameter passed to
> qemu_co_queue_run_restart. Therefore, qemu_co_queue_run_restart
> accesses an invalid memory and a segfault error ocurrs.
> 
> The qemu_co_queue_run_restart function re-enters coroutines waiting
> in the co_queue. However, this function should be only used int the
> qemu_coroutine_enter context. Only in this context, when the current
> coroutine gets execution control again(after the execution of
> qemu_coroutine_switch), we can restart the target coutine because the
> target coutine has yielded back to the current coroutine or it has
> terminated.
> 
> First we want to put qemu_co_queue_run_restart in qemu_coroutine_enter,
> but we find we can not access the target coroutine if it terminates.
> 
> Signed-off-by: Bin Wu <address@hidden>
> ---
>  qemu-coroutine.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/qemu-coroutine.c b/qemu-coroutine.c
> index 525247b..9a294c4 100644
> --- a/qemu-coroutine.c
> +++ b/qemu-coroutine.c
> @@ -99,24 +99,25 @@ static void coroutine_delete(Coroutine *co)
>      qemu_coroutine_delete(co);
>  }
>  
> -static void coroutine_swap(Coroutine *from, Coroutine *to)
> +static CoroutineAction coroutine_swap(Coroutine *from, Coroutine *to)
>  {
>      CoroutineAction ret;
>  
>      ret = qemu_coroutine_switch(from, to, COROUTINE_YIELD);
>  
> -    qemu_co_queue_run_restart(to);
> -
>      switch (ret) {
>      case COROUTINE_YIELD:
> -        return;
> +        break;
>      case COROUTINE_TERMINATE:
>          trace_qemu_coroutine_terminate(to);
> +        qemu_co_queue_run_restart(to);
>          coroutine_delete(to);
> -        return;
> +        break;
>      default:
>          abort();
>      }
> +
> +    return ret;
>  }
>  
>  void qemu_coroutine_enter(Coroutine *co, void *opaque)
> @@ -133,6 +134,8 @@ void qemu_coroutine_enter(Coroutine *co, void *opaque)
>      co->caller = self;
>      co->entry_arg = opaque;
>      coroutine_swap(self, co);
> +    if (ret == COROUTINE_YIELD)
> +        qemu_co_queue_run_restart(co);
>  }
>  
>  void coroutine_fn qemu_coroutine_yield(void)
> 

-- 
Bin Wu