From 2312622923d98cda9ec397815932d4658b3e79e1 Mon Sep 17 00:00:00 2001
From: Jan Maria Matejka <mq@ucw.cz>
Date: Wed, 5 Sep 2018 14:29:54 +0200
Subject: [PATCH] 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.
---
 lib/coroutine.h         |  1 +
 sysdep/unix/coroutine.c | 45 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/lib/coroutine.h b/lib/coroutine.h
index 93a41233..f3cce40c 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 fc650a57..8ffb8148 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