[Spice-commits] 8 commits - gtk/coroutine.h gtk/coroutine_gthread.c gtk/coroutine_ucontext.c gtk/coroutine_winfibers.c gtk/spice-channel.c
Christophe Fergau
teuf at kemper.freedesktop.org
Tue Nov 19 05:42:53 PST 2013
gtk/coroutine.h | 6 ++++--
gtk/coroutine_gthread.c | 11 +++++++----
gtk/coroutine_ucontext.c | 33 ++++++++++++++++++++++++++-------
gtk/coroutine_winfibers.c | 12 ++++++++----
gtk/spice-channel.c | 9 ++++++++-
5 files changed, 53 insertions(+), 18 deletions(-)
New commits:
commit 13826c95f3424059a6d6535b01335582506c2fa4
Author: Christophe Fergeau <cfergeau at redhat.com>
Date: Mon Nov 18 18:13:57 2013 +0100
Fix leak of mmapped memory when cc_init() fails
diff --git a/gtk/coroutine_ucontext.c b/gtk/coroutine_ucontext.c
index f4ff22e..4689166 100644
--- a/gtk/coroutine_ucontext.c
+++ b/gtk/coroutine_ucontext.c
@@ -65,6 +65,8 @@ static void coroutine_trampoline(struct continuation *cc)
int coroutine_init(struct coroutine *co)
{
+ int inited;
+
if (co->stack_size == 0)
co->stack_size = 16 << 20;
@@ -80,7 +82,14 @@ int coroutine_init(struct coroutine *co)
co->cc.release = _coroutine_release;
co->exited = 0;
- return cc_init(&co->cc);
+ 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;
}
#if 0
commit 63272af5966d4866789ce4f2b6ae8842fee195b5
Author: Christophe Fergeau <cfergeau at redhat.com>
Date: Mon Nov 18 17:49:46 2013 +0100
Check coroutine_init() return value
coroutine_init() can fail, but spice-channel.c was not checking its return
value, which could lead to some crashes if coroutine_init() failed and we
then try to use coroutine_yieldto()
diff --git a/gtk/coroutine.h b/gtk/coroutine.h
index 15b90b4..ef6f3db 100644
--- a/gtk/coroutine.h
+++ b/gtk/coroutine.h
@@ -56,7 +56,7 @@ struct coroutine
};
#define IN_MAIN_CONTEXT (coroutine_self() == NULL || coroutine_is_main_context(coroutine_self()))
-int coroutine_init(struct coroutine *co);
+int coroutine_init(struct coroutine *co) G_GNUC_WARN_UNUSED_RESULT;
int coroutine_release(struct coroutine *co);
diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
index 41d5eab..d0b93f4 100644
--- a/gtk/spice-channel.c
+++ b/gtk/spice-channel.c
@@ -2358,6 +2358,7 @@ 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;
@@ -2368,7 +2369,13 @@ static gboolean connect_delayed(gpointer data)
co->entry = spice_channel_coroutine;
co->release = NULL;
- coroutine_init(co);
+ 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_yieldto(co, channel);
return FALSE;
commit acb9b6863fe4527ca2b8ba197c1f97b07aca0c5f
Author: Daniel P. Berrange <berrange at redhat.com>
Date: Fri Sep 13 13:30:18 2013 +0100
Free coroutine stack when releasing coroutine
The coroutine_init function mmap's a stack for the
ucontext coroutine, but nothing ever munmaps it.
Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
diff --git a/gtk/coroutine_ucontext.c b/gtk/coroutine_ucontext.c
index 3663dfb..f4ff22e 100644
--- a/gtk/coroutine_ucontext.c
+++ b/gtk/coroutine_ucontext.c
@@ -50,6 +50,8 @@ static int _coroutine_release(struct continuation *cc)
return ret;
}
+ munmap(co->cc.stack, co->cc.stack_size);
+
co->caller = NULL;
return 0;
commit eb679f3138a1b1e0f6f00cc901333828bf1fd47e
Author: Daniel P. Berrange <berrange at redhat.com>
Date: Fri Sep 13 13:30:04 2013 +0100
Abort if mmap of coroutine stack fails
If we fail to mmap the stack, abort the processs rather
than returning an error. This is standard practice in
glib apps, and the caller was not checking the
coroutine_init() return code leading to memory corruption.
Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
diff --git a/gtk/coroutine_ucontext.c b/gtk/coroutine_ucontext.c
index f960cfb..3663dfb 100644
--- a/gtk/coroutine_ucontext.c
+++ b/gtk/coroutine_ucontext.c
@@ -25,8 +25,10 @@
#include <sys/types.h>
#endif
#include <sys/mman.h>
+#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
+#include <string.h>
#include "coroutine.h"
#ifndef MAP_ANONYMOUS
@@ -70,7 +72,8 @@ int coroutine_init(struct coroutine *co)
MAP_PRIVATE | MAP_ANONYMOUS,
-1, 0);
if (co->cc.stack == MAP_FAILED)
- return -1;
+ g_error("Failed to allocate %u bytes for coroutine stack: %s",
+ (unsigned)co->stack_size, strerror(errno));
co->cc.entry = coroutine_trampoline;
co->cc.release = _coroutine_release;
co->exited = 0;
commit 143afbb8851a114397692f4ae0334d439af77637
Author: Christophe Fergeau <cfergeau at redhat.com>
Date: Fri Nov 15 16:03:17 2013 +0100
Fix IN_MAIN_CONTEXT when using coroutine=gthread
The IN_MAIN_CONTEXT macro checks coroutine_self()->caller against NULL to
decide whether we are in the main context or not. However, this is an
implementation detail of the ucontext coroutine implementation, in the
gthread implementation, coroutine_self()->caller will be non-NULL even in
the main context.
This commit introduces a coroutine_is_main_context() method which each
coroutine backend implements. It turns out it can be implemented the same
way for each backend.
diff --git a/gtk/coroutine.h b/gtk/coroutine.h
index 7e3bc28..15b90b4 100644
--- a/gtk/coroutine.h
+++ b/gtk/coroutine.h
@@ -55,7 +55,7 @@ struct coroutine
#endif
};
-#define IN_MAIN_CONTEXT (coroutine_self() == NULL || coroutine_self()->caller == NULL)
+#define IN_MAIN_CONTEXT (coroutine_self() == NULL || coroutine_is_main_context(coroutine_self()))
int coroutine_init(struct coroutine *co);
int coroutine_release(struct coroutine *co);
@@ -68,6 +68,8 @@ void *coroutine_yieldto(struct coroutine *to, void *arg);
void *coroutine_yield(void *arg);
+gboolean coroutine_is_main_context(struct coroutine *co);
+
#endif
/*
* Local variables:
diff --git a/gtk/coroutine_gthread.c b/gtk/coroutine_gthread.c
index 51d2d6f..ab30631 100644
--- a/gtk/coroutine_gthread.c
+++ b/gtk/coroutine_gthread.c
@@ -160,3 +160,7 @@ void *coroutine_yield(void *arg)
return coroutine_swap(coroutine_self(), to, arg);
}
+gboolean coroutine_is_main_context(struct coroutine *co)
+{
+ return (co == &leader);
+}
diff --git a/gtk/coroutine_ucontext.c b/gtk/coroutine_ucontext.c
index 889f0d6..f960cfb 100644
--- a/gtk/coroutine_ucontext.c
+++ b/gtk/coroutine_ucontext.c
@@ -130,6 +130,11 @@ void *coroutine_yield(void *arg)
coroutine_self()->caller = NULL;
return coroutine_swap(coroutine_self(), to, arg);
}
+
+gboolean coroutine_is_main_context(struct coroutine *co)
+{
+ return (co == &leader);
+}
/*
* Local variables:
* c-indent-level: 8
diff --git a/gtk/coroutine_winfibers.c b/gtk/coroutine_winfibers.c
index f4a4481..266b1de 100644
--- a/gtk/coroutine_winfibers.c
+++ b/gtk/coroutine_winfibers.c
@@ -111,6 +111,11 @@ void *coroutine_yield(void *arg)
coroutine_self()->caller = NULL;
return coroutine_swap(coroutine_self(), to, arg);
}
+
+gboolean coroutine_is_main_context(struct coroutine *co)
+{
+ return (co == &leader);
+}
/*
* Local variables:
* c-indent-level: 8
commit 812c85b21bc1b9eeb0ff5c5659da53bc64786af0
Author: Christophe Fergeau <cfergeau at redhat.com>
Date: Fri Nov 15 12:01:45 2013 +0100
Fix crash on remote-viewer startup with gthread coroutine
g_object_notify_main_context() was recently changed to automatically
detect whether we are running in the main context or not, and SpiceSession
is now using g_object_notify_main_context().
In order to achieve that, IN_MAIN_CONTEXT is used, but it's not safe
to be used before coroutine_init() is called. IN_MAIN_CONTEXT expands to
(coroutine_self()->caller == NULL), but coroutine_self() will be NULL when
using gthreads for the coroutine implementation until coroutine_init() has
been called.
Before coroutine_init() has been called, we'll always be running in the
main context, so we can just add a check for coroutine_self() != NULL to
IN_MAIN_CONTEXT to solve that issue.
diff --git a/gtk/coroutine.h b/gtk/coroutine.h
index a9b3a87..7e3bc28 100644
--- a/gtk/coroutine.h
+++ b/gtk/coroutine.h
@@ -55,7 +55,7 @@ struct coroutine
#endif
};
-#define IN_MAIN_CONTEXT (coroutine_self()->caller == NULL)
+#define IN_MAIN_CONTEXT (coroutine_self() == NULL || coroutine_self()->caller == NULL)
int coroutine_init(struct coroutine *co);
int coroutine_release(struct coroutine *co);
commit a9a2c162e0ee03f80e3e6f7450a002719a80d06a
Author: Marc-André Lureau <marcandre.lureau at gmail.com>
Date: Fri Nov 15 22:03:40 2013 +0100
coroutine: fix current coroutine
When leaving a coroutine, it swaps back to where it came from, not to
the leader/main coroutine.
diff --git a/gtk/coroutine_ucontext.c b/gtk/coroutine_ucontext.c
index 6251528..889f0d6 100644
--- a/gtk/coroutine_ucontext.c
+++ b/gtk/coroutine_ucontext.c
@@ -103,7 +103,7 @@ void *coroutine_swap(struct coroutine *from, struct coroutine *to, void *arg)
return from->data;
else if (ret == 1) {
coroutine_release(to);
- current = &leader;
+ current = from;
to->exited = 1;
return to->data;
}
commit baa51c5ee34ca6d0448d02901a0a61dc87c3dff8
Author: Marc-André Lureau <marcandre.lureau at gmail.com>
Date: Fri Nov 15 22:03:39 2013 +0100
coroutine: add some preconditions
diff --git a/gtk/coroutine_gthread.c b/gtk/coroutine_gthread.c
index 14231a0..51d2d6f 100644
--- a/gtk/coroutine_gthread.c
+++ b/gtk/coroutine_gthread.c
@@ -140,10 +140,9 @@ struct coroutine *coroutine_self(void)
void *coroutine_yieldto(struct coroutine *to, void *arg)
{
- if (to->caller) {
- fprintf(stderr, "Co-routine is re-entering itself\n");
- abort();
- }
+ g_return_val_if_fail(!to->caller, NULL);
+ g_return_val_if_fail(!to->exited, NULL);
+
CO_DEBUG("SWAP");
return coroutine_swap(coroutine_self(), to, arg);
}
diff --git a/gtk/coroutine_ucontext.c b/gtk/coroutine_ucontext.c
index af811a7..6251528 100644
--- a/gtk/coroutine_ucontext.c
+++ b/gtk/coroutine_ucontext.c
@@ -19,6 +19,7 @@
*/
#include <config.h>
+#include <glib.h>
#ifdef HAVE_SYS_TYPES_H
#include <sys/types.h>
@@ -112,10 +113,9 @@ void *coroutine_swap(struct coroutine *from, struct coroutine *to, void *arg)
void *coroutine_yieldto(struct coroutine *to, void *arg)
{
- if (to->caller) {
- fprintf(stderr, "Co-routine is re-entering itself\n");
- abort();
- }
+ g_return_val_if_fail(!to->caller, NULL);
+ g_return_val_if_fail(!to->exited, NULL);
+
to->caller = coroutine_self();
return coroutine_swap(coroutine_self(), to, arg);
}
diff --git a/gtk/coroutine_winfibers.c b/gtk/coroutine_winfibers.c
index a4cd14b..f4a4481 100644
--- a/gtk/coroutine_winfibers.c
+++ b/gtk/coroutine_winfibers.c
@@ -94,10 +94,9 @@ void *coroutine_swap(struct coroutine *from, struct coroutine *to, void *arg)
void *coroutine_yieldto(struct coroutine *to, void *arg)
{
- if (to->caller) {
- fprintf(stderr, "Co-routine is re-entering itself\n");
- abort();
- }
+ g_return_val_if_fail(!to->caller, NULL);
+ g_return_val_if_fail(!to->exited, NULL);
+
to->caller = coroutine_self();
return coroutine_swap(coroutine_self(), to, arg);
}
More information about the Spice-commits
mailing list