Coroutine: use pthread_exit() instead of pthread_cancel()

The coroutine itself may hold some resources when going across pthread
cancellable points. Now it is ensured (by semaphores) that either the
main process or the coroutine is running so the coroutine is always
cancelled inside coro_suspend() where everything is clean but it will
change in future.

Instead, we explicitly mark the coroutine freeze/cancel points by
yielding there -- calling coro_suspend() and checking whether the
master process has requested to stop.

Where pthread_cancel() was, we instead set a flag and resume that
thread to finish its work and exit itself.

diff --git a/lib/coroutine.h b/lib/coroutine.h
index 93a41233a8cbfdbb4d8f70e21228ee9c7cc16d57..f3cce40c73368993fa29b713596cdf20464007dc 100644 (file)
--- a/lib/coroutine.h
+++ b/lib/coroutine.h
@@ -15,6 +15,7 @@ typedef struct coroutine coroutine;
 coroutine *coro_new(struct pool *pool, void (*entry_point)(void *arg), void *arg);
 void coro_suspend(void);
 void coro_resume(coroutine *c);
+void coro_done(void *retval) NORET;
 
 struct birdsock;
 int coro_sk_read(struct birdsock *s);

diff --git a/sysdep/unix/coroutine.c b/sysdep/unix/coroutine.c
index fc650a57dbe750fdc4ee1ce8c34930bb5917e443..8ffb81486b5bdda63edaca469af7578503c301e2 100644 (file)
--- a/sysdep/unix/coroutine.c
+++ b/sysdep/unix/coroutine.c
@@ -100,6 +100,17 @@ coro_new(pool *p, void (*entry_point)(void *), void *arg)
   return c;
 }
 
+void
+coro_done(void *retval)
+{
+  ASSERT(coro_inited);
+  ASSERT(coro_current);
+  coroutine *c = coro_current;
+  c->retval = retval;
+  coro_suspend();
+  bug("Coroutine suspend after coro_done() should never return");
+}
+
 void
 coro_suspend(void)
 {
@@ -129,12 +140,16 @@ coro_resume(coroutine *c)
 #include <pthread.h>
 #include <semaphore.h>
 
+#define CORO_STOP 1    /* The coroutine should stop at first coro_suspend(). */
+#define CORO_DONE 2    /* The coroutine has already stopped. */
+
 struct coroutine {
   resource r;
   pthread_t thread;
   void (*entry_point)(void *arg);
   void *arg;
   sem_t sem;
+  uint flags;
 };
 
 static coroutine *coro_current;                // NULL for main context
@@ -147,7 +162,11 @@ coro_free(resource *r)
 {
   coroutine *c = (coroutine *) r;
   ASSERT(coro_current != c);
-  pthread_cancel(c->thread);
+
+  c->flags |= CORO_STOP;
+  coro_resume(c);
+
+  ASSERT(c->flags & CORO_DONE);
   pthread_join(c->thread, NULL);
 }
 
@@ -211,16 +230,40 @@ coro_new(pool *p, void (*entry_point)(void *), void *arg)
   return c;
 }
 
+static inline void
+coro_check_stop(void)
+{
+  ASSERT(coro_inited);
+  ASSERT(coro_current);
+  coroutine *c = coro_current;
+  if (c->flags & CORO_STOP)
+    coro_done(NULL);
+}
+
+void
+coro_done(void *retval)
+{
+  ASSERT(coro_inited);
+  ASSERT(coro_current);
+  coroutine *c = coro_current;
+  c->flags |= CORO_DONE;
+  sem_post(&coro_main_sem);
+  pthread_exit(retval);
+  bug("pthread_exit should never return");
+}
+
 void
 coro_suspend(void)
 {
   ASSERT(coro_inited);
   ASSERT(coro_current);
   coroutine *c = coro_current;
+  coro_check_stop();
   sem_post(&coro_main_sem);
   while (sem_wait(&c->sem) < 0)
     ;
   coro_current = c;
+  coro_check_stop();
 }
 
 void