[Spice-devel] [spice-gtk RFC 0/3] coroutine: Make signal/notify coroutine code more robust against unexpected coroutine_init()

[Marc-André Lureau]

Hi

----- Original Message -----
> Hey,
> 
> Before the recent rework of disconnect/channel_reset, it was possible to get
> in a situation where
> an idle would get queued, then an attempt to emit a signal from coroutine
> context would be attempted.
> The first idle would run, call coroutine_init() which would reset the
> coroutine state, and then the

I see, what's missing is a warning/abort when doing re-init() on an unfinished coroutine.

GCoroutine, which we hope to depend on soon, will disallow this, you'll have to use a different coroutine.

And the client code could check if a coroutine is finished before creating a new one with g_coroutine_resumable(). It could eventually try to finish it by yielding in a loop, but this might point to a bug depending on how you use the coroutine.

> signal emission idle would run in an invalid context (the stack allocated
> signal_data data would no longer be valid), causing a hard to diagnose crash.
> This series tries to improve that by showing a warning when coroutine_init()
> is called while
> a signal/notify idle is queued.
> For now this is only done for the ucontext implementation, depending on where
> this RFC goes,
> I'll add support to the other variants. While this adds some code to
> coroutine_init() and a member to
> struct coroutine, the only thing which is needed is some way to run a check
> when coroutine_init() gets called. For example, an alternative could be a
> g_coroutine_init() wrapper
> doing the check and then calling into coroutine_init().

You might get some warnings/abort for this specific idle case, but there are many other cases where unfinished coroutines won't give you a warning. That's why I think the proper solution is to prevent clearing an unfinished coroutine.