[Spice-devel] [PATCH spice-gtk] coroutine: abort on OOM or impossible conditions

Marc-André Lureau marcandre.lureau at gmail.com
Wed Nov 20 01:59:42 PST 2013


On Tue, Nov 19, 2013 at 10:23 PM, Marc-André Lureau
<marcandre.lureau at gmail.com> wrote:
> Make the coroutine code more glib-like, by aborting on OOM or unhandled
> conditions.
> ---
>  gtk/continuation.c        |  9 ++++-----
>  gtk/continuation.h        |  2 +-
>  gtk/coroutine.h           |  3 ++-
>  gtk/coroutine_gthread.c   | 12 ++++++------
>  gtk/coroutine_ucontext.c  | 17 ++++-------------
>  gtk/coroutine_winfibers.c |  9 ++++-----
>  gtk/spice-channel.c       |  9 +--------
>  spice-common              |  2 +-
>  8 files changed, 23 insertions(+), 40 deletions(-)
>
> diff --git a/gtk/continuation.c b/gtk/continuation.c
> index 2891a25..d1249d6 100644
> --- a/gtk/continuation.c
> +++ b/gtk/continuation.c
> @@ -18,6 +18,8 @@
>   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA
>   */
>  #include <config.h>
> +#include <errno.h>
> +#include <glib.h>
>
>  #undef _FORTIFY_SOURCE
>
> @@ -49,13 +51,12 @@ static void continuation_trampoline(int i0, int i1)
>         cc->entry(cc);
>  }
>
> -int cc_init(struct continuation *cc)
> +void cc_init(struct continuation *cc)
>  {
>         volatile union cc_arg arg;
>         arg.p = cc;
>         if (getcontext(&cc->uc) == -1)
> -               return -1;
> -
> +               g_error("getcontext() failed: %s", g_strerror(errno));
>         cc->uc.uc_link = &cc->last;
>         cc->uc.uc_stack.ss_sp = cc->stack;
>         cc->uc.uc_stack.ss_size = cc->stack_size;
> @@ -63,8 +64,6 @@ int cc_init(struct continuation *cc)
>
>         makecontext(&cc->uc, (void *)continuation_trampoline, 2, arg.i[0], arg.i[1]);
>         swapcontext(&cc->last, &cc->uc);
> -
> -       return 0;
>  }
>
>  int cc_release(struct continuation *cc)
> diff --git a/gtk/continuation.h b/gtk/continuation.h
> index 1247337..675a257 100644
> --- a/gtk/continuation.h
> +++ b/gtk/continuation.h
> @@ -39,7 +39,7 @@ struct continuation
>         jmp_buf jmp;
>  };
>
> -int cc_init(struct continuation *cc);
> +void cc_init(struct continuation *cc);
>
>  int cc_release(struct continuation *cc);
>
> diff --git a/gtk/coroutine.h b/gtk/coroutine.h
> index ef6f3db..adfc338 100644
> --- a/gtk/coroutine.h
> +++ b/gtk/coroutine.h
> @@ -56,7 +56,8 @@ struct coroutine
>  };
>
>  #define IN_MAIN_CONTEXT (coroutine_self() == NULL || coroutine_is_main_context(coroutine_self()))
> -int coroutine_init(struct coroutine *co) G_GNUC_WARN_UNUSED_RESULT;
> +
> +void coroutine_init(struct coroutine *co);
>
>  int coroutine_release(struct coroutine *co);
>
> diff --git a/gtk/coroutine_gthread.c b/gtk/coroutine_gthread.c
> index ab30631..54581c1 100644
> --- a/gtk/coroutine_gthread.c
> +++ b/gtk/coroutine_gthread.c
> @@ -86,8 +86,10 @@ static gpointer coroutine_thread(gpointer opaque)
>         return NULL;
>  }
>
> -int coroutine_init(struct coroutine *co)
> +void coroutine_init(struct coroutine *co)
>  {
> +       GError *err = NULL;
> +
>         if (run_cond == NULL)
>                 coroutine_system_init();
>
> @@ -95,15 +97,13 @@ int coroutine_init(struct coroutine *co)
>         co->thread = g_thread_create_full(coroutine_thread, co, co->stack_size,
>                                           FALSE, TRUE,
>                                           G_THREAD_PRIORITY_NORMAL,
> -                                         NULL);
> -       if (co->thread == NULL)
> -               return -1;
> +                                         &error);

s/error/err

> +       if (err != NULL)
> +               g_error("g_thread_create_full() failed: %s", err->message);
>
>         co->exited = 0;
>         co->runnable = FALSE;
>         co->caller = NULL;
> -
> -       return 0;
>  }
>
>  int coroutine_release(struct coroutine *co G_GNUC_UNUSED)
> diff --git a/gtk/coroutine_ucontext.c b/gtk/coroutine_ucontext.c
> index 4689166..a26b607 100644
> --- a/gtk/coroutine_ucontext.c
> +++ b/gtk/coroutine_ucontext.c
> @@ -63,10 +63,8 @@ static void coroutine_trampoline(struct continuation *cc)
>         co->data = co->entry(co->data);
>  }
>
> -int coroutine_init(struct coroutine *co)
> +void coroutine_init(struct coroutine *co)
>  {
> -       int inited;
> -
>         if (co->stack_size == 0)
>                 co->stack_size = 16 << 20;
>
> @@ -76,20 +74,13 @@ int coroutine_init(struct coroutine *co)
>                             MAP_PRIVATE | MAP_ANONYMOUS,
>                             -1, 0);
>         if (co->cc.stack == MAP_FAILED)
> -               g_error("Failed to allocate %u bytes for coroutine stack: %s",
> -                       (unsigned)co->stack_size, strerror(errno));
> +               g_error("mmap(%zu) failed: %s", co->stack_size, g_strerror(errno));
> +
>         co->cc.entry = coroutine_trampoline;
>         co->cc.release = _coroutine_release;
>         co->exited = 0;
>
> -       inited = cc_init(&co->cc);
> -       if (inited != 0) {
> -               munmap(co->cc.stack, co->cc.stack_size);
> -               co->cc.stack = NULL;
> -               co->exited = 1;
> -       }
> -
> -       return inited;
> +       cc_init(&co->cc);
>  }
>
>  #if 0
> diff --git a/gtk/coroutine_winfibers.c b/gtk/coroutine_winfibers.c
> index 266b1de..1ed1981 100644
> --- a/gtk/coroutine_winfibers.c
> +++ b/gtk/coroutine_winfibers.c
> @@ -51,20 +51,19 @@ static void WINAPI coroutine_trampoline(LPVOID lpParameter)
>         SwitchToFiber(caller->fiber);
>  }
>
> -int coroutine_init(struct coroutine *co)
> +void coroutine_init(struct coroutine *co)
>  {
>         if (leader.fiber == NULL) {
>                 leader.fiber = ConvertThreadToFiber(&leader);
>                 if (leader.fiber == NULL)
> -                       return -1;
> +                       g_error("ConvertThreadToFiber() failed");
>         }
>
>         co->fiber = CreateFiber(0, &coroutine_trampoline, co);
> -       co->ret = 0;
>         if (co->fiber == NULL)
> -               return -1;
> +               g_error("CreateFiber() failed");
>
> -       return 0;
> +       co->ret = 0;
>  }
>
>  struct coroutine *coroutine_self(void)
> diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
> index 2c8c1b9..a045767 100644
> --- a/gtk/spice-channel.c
> +++ b/gtk/spice-channel.c
> @@ -2366,7 +2366,6 @@ static gboolean connect_delayed(gpointer data)
>      SpiceChannel *channel = data;
>      SpiceChannelPrivate *c = channel->priv;
>      struct coroutine *co;
> -    int inited;
>
>      CHANNEL_DEBUG(channel, "Open coroutine starting %p", channel);
>      c->connect_delayed_id = 0;
> @@ -2377,13 +2376,7 @@ static gboolean connect_delayed(gpointer data)
>      co->entry = spice_channel_coroutine;
>      co->release = NULL;
>
> -    inited = coroutine_init(co);
> -    if (inited != 0) {
> -        g_warning("Failed to initialize channel coroutine");
> -        CHANNEL_DEBUG(channel, "coroutine_init failed");
> -        g_object_unref(G_OBJECT(channel));
> -        return FALSE;
> -    }
> +    coroutine_init(co);
>      coroutine_yieldto(co, channel);
>
>      return FALSE;
> diff --git a/spice-common b/spice-common
> index 1450bb4..261d270 160000
> --- a/spice-common
> +++ b/spice-common
> @@ -1 +1 @@
> -Subproject commit 1450bb4ddbd8ceab9192e4f84606aa5ae54c5ea6
> +Subproject commit 261d270cc8e9af0915d5248df240828214ec996e

removed

> --
> 1.8.3.1
>



-- 
Marc-André Lureau


More information about the Spice-devel mailing list