gh-845 (cord_cojoin()): review fixes

src/fiber.cc

@@ -38,7 +38,6 @@
 #include "assoc.h"
 #include "memory.h"
 #include "trigger.h"
-#include <typeinfo>
 #include "third_party/pmatomic.h"
 
 /*
@@ -47,16 +46,16 @@
  * implement cord_cojoin.
  */
 struct cord_on_exit {
-	void (*callback) (void*);
+	void (*callback)(void*);
 	void *argument;
 };
 
 /*
- * The special value distinct from any valid pointer to cord_on_exit
- * structure AND NULL). This value is stored in cord()->on_exit by the
- * thread func prior to a thread termination.
+ * A special value distinct from any valid pointer to cord_on_exit
+ * structure AND NULL. This value is stored in cord()->on_exit by the
+ * thread function prior to thread termination.
  */
-#define CORD_ON_EXIT_WONT_RUN ((struct cord_on_exit*)1)
+static const struct cord_on_exit CORD_ON_EXIT_WONT_RUN = { NULL, NULL };
 
 static struct cord main_cord;
 __thread struct cord *cord_ptr = NULL;
@@ -652,22 +651,20 @@ void *cord_thread_func(void *p)
 		res = NULL;
 	}
 	/*
-	 * cord()->on_exit initially holds a NULL value. This field is
+	 * cord()->on_exit initially holds NULL. This field is
 	 * change-once.
-	 * Either a handler installation suceeds (in cord_cojoin) or prior
-	 * to thread exit the thread func discovers that no handler was installed
-	 * so far and it stores CORD_ON_EXIT_WONT_RUN to prevent a future
-	 * handler installation (since a handler won't run anyway).
+	 * Either handler installation succeeds (in cord_cojoin())
+	 * or prior to thread exit the thread function discovers
+	 * that no handler was installed so far and it stores
+	 * CORD_ON_EXIT_WONT_RUN to prevent a future handler
+	 * installation (since a handler won't run anyway).
 	 */
 	struct cord_on_exit *handler = NULL; /* expected value */
 	bool rc = pm_atomic_compare_exchange_strong(&cord()->on_exit,
 	                                            &handler,
-	                                            CORD_ON_EXIT_WONT_RUN);
-	if (!rc) {
-		assert(handler);
-		assert(handler->callback);
+	                                            &CORD_ON_EXIT_WONT_RUN);
+	if (rc == 0)
 		handler->callback(handler->argument);
-	}
 	return res;
 }
 
@@ -711,37 +708,36 @@ cord_join(struct cord *cord)
 	return res;
 }
 
-struct cord_cojoin_state
+/** The state of the waiter for a thread to complete. */
+struct cord_cojoin_ctx
 {
 	struct ev_loop *loop;
+	/** Waiting fiber. */
 	struct fiber *fiber;
+	/*
+	 * This event is signalled when the subject thread is
+	 * about to die.
+	 */
 	struct ev_async async;
-	bool task_complete;
 };
 
 static void
 cord_cojoin_on_exit(void *arg)
 {
-	struct cord_cojoin_state *b =
-		(struct cord_cojoin_state *)arg;
+	struct cord_cojoin_ctx *ctx = (struct cord_cojoin_ctx *)arg;
 
-	assert(b->loop);
-	ev_async_send(b->loop, &b->async);
+	ev_async_send(ctx->loop, &ctx->async);
 }
 
 static void
 cord_cojoin_wakeup(struct ev_loop *loop, struct ev_async *ev, int revents)
 {
-	assert(ev);
-	assert(ev->data);
 	(void)loop;
 	(void)revents;
 
-	struct cord_cojoin_state *b = (cord_cojoin_state *)ev->data;
+	struct cord_cojoin_ctx *ctx = (struct cord_cojoin_ctx *)ev->data;
 
-	assert(b->fiber);
-	b->task_complete = true;
-	fiber_wakeup(b->fiber);
+	fiber_wakeup(ctx->fiber);
 }
 
 int
@@ -749,16 +745,15 @@ cord_cojoin(struct cord *cord)
 {
 	assert(cord() != cord); /* Can't join self. */
 
-	struct cord_cojoin_state b;
-	b.loop = loop();
-	b.fiber = fiber();
-	b.task_complete = false;
+	struct cord_cojoin_ctx ctx;
+	ctx.loop = loop();
+	ctx.fiber = fiber();
 
-	ev_async_init(&b.async, cord_cojoin_wakeup);
-	b.async.data = &b;
-	ev_async_start(loop(), &b.async);
+	ev_async_init(&ctx.async, cord_cojoin_wakeup);
+	ctx.async.data = &ctx;
+	ev_async_start(loop(), &ctx.async);
 
-	struct cord_on_exit handler = { cord_cojoin_on_exit, &b };
+	struct cord_on_exit handler = { cord_cojoin_on_exit, &ctx };
 
 	/*
 	 * cord->on_exit initially holds a NULL value. This field is
@@ -771,25 +766,26 @@ cord_cojoin(struct cord *cord)
 	 * A handler installation fails either if the thread did exit or
 	 * if someone is already joining this cord (BUG).
 	 */
-	if (!rc) {
+	if (rc == 0) {
 		/* Assume cord's thread already exited. */
-		assert(prev_handler == CORD_ON_EXIT_WONT_RUN);
+		assert(prev_handler == &CORD_ON_EXIT_WONT_RUN);
 	} else {
 		/*
-		 * Wait until the thread exits. Prior to exit the thread invokes
-		 * cord_cojoin_on_exit, signaling ev_async, making the event loop
-		 * call cord_cojoin_wakeup, waking up this fiber again.
+		 * Wait until the thread exits. Prior to exit the
+		 * thread invokes cord_cojoin_on_exit, signaling
+		 * ev_async, making the event loop call
+		 * cord_cojoin_wakeup, waking up this fiber again.
 		 *
-		 * The fiber is non-cancellable during the wait to avoid
-		 * invalidating the state struct on stack.
+		 * The fiber is non-cancellable during the wait to
+		 * avoid invalidating of the cord_cojoin_ctx
+		 * object declared on stack.
 		 */
 		bool cancellable = fiber_set_cancellable(false);
 		fiber_yield();
-		assert(b.task_complete);
 		fiber_set_cancellable(cancellable);
 	}
 
-	ev_async_stop(loop(), &b.async);
+	ev_async_stop(loop(), &ctx.async);
 	return cord_join(cord);
 }
 

third_party/pmatomic.h

@@ -4,7 +4,7 @@
  * Borrowed from FreeBSD (original copyright follows).
  *
  * Standard atomic facilities in stdatomic.h are great, unless you are
- * stuck with an old compiler, or you attemt to compile a code using
+ * stuck with an old compiler, or you attempt to compile code using
  * stdatomic.h in C++ mode [gcc 4.9], or if you were desperate enough to
  * enable OpenMP in C mode [gcc 4.9].
  *
@@ -42,7 +42,7 @@
  * reason we comment unused code regions with #if 0 instead of removing
  * them.
  *
- * Renames are carried out by a scipt generating the final header.
+ * Renames are carried out by a script generating the final header.
  */
 
 /*-