From: Jan Maria Matejka <mq@ucw.cz>
Date: Wed, 5 Sep 2018 12:29:54 +0000 (+0200)
Subject: Coroutine: use pthread_exit() instead of pthread_cancel()
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2312622923d98cda9ec397815932d4658b3e79e1;p=thirdparty%2Fbird.git

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 93a41233a..f3cce40c7 100644
--- 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 fc650a57d..8ffb81486 100644
--- 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