How can this simple coroutine implementation be improved?

This is a quick and dirty implementation of coroutines that implements yield by saving and restoring the stack to and from the heap. Here's an earlier version, which does the most naive possible thing, and just allocates enough space on the stack for every possible coroutine. The relevant bits from the current code are below, and the whole thing is here.

I'm open to feedback both on pretty much anything: completely different approaches, how to restructure this approach, bugs, idiomatic C style pointers, etc.

An obvious performance improvement would be to use ucontext, but that seems to be deprecated and somewhat broken on Mac OS X (although it works fine on the linux distros I've tried). I'm not sure what the next step is to make this 'better'.

`typedef void (*coroutine_cb)();

static void scheduler(coroutine_cb);
static int spawn(coroutine_cb);
static void yield(int);

jmp_buf scheduler_jmp;
void *scheduler_rbp;
int coro_pid;

coroutine_cb scheduler_next_coro;

#define MAX_COROS 100

struct {
jmp_buf jmp;
void *stack;
long stack_sz;
} coroutines[MAX_COROS];

static void scheduler(coroutine_cb coro)
{
printf("starting the scheduler...\n");
static int max_pid = 1;

// we move down 0x1000 to give scheduler space to call functions and allocate stack variables without having them get
// overwritten by the memcpy below. Before we did this, we had to manually copy memory instead of using memcpy
// because we would overwrite memcpy's stack
scheduler_rbp = __builtin_frame_address(0) - 0x1000;
scheduler_next_coro = coro;
int value = setjmp(scheduler_jmp);
// value == 0 means just starting
// value == -1 means spawning new coroutine
// value positive means hop back to a specific pid
if (value == 0 || value == -1)
{
coro_pid = max_pid++;
printf("about to run coro %d...\n", coro_pid);
char *buf = alloca(0x2000); // was 0x1000 when we didn't allocate extra space for scheduler stack
asm volatile("" :: "m" (buf));
scheduler_ne

Things you did well on:

• Overall, this looks like a pretty decent little bit of code. It looks like some research went into this

• I like the comments. Everywhere I was confused on something, there was a comment to clarify. You also didn't go overboard with them!

Things you could improve on:

Preprocessor:

• Group all of your #defines at the top of your code, just after your #imports. Otherwise you will have to jump around your code to find them.

Initialization:

-
typedef your structs.

struct {
  jmp_buf jmp;
  void *stack;
  long stack_sz;
} coroutines[MAX_COROS];

The typedef means you no longer have to write struct all over the place. That not only saves keystrokes, it also can make the code cleaner since it provides a smidgen more abstraction.

-
You can initialize some variables right away.

int stack_sz; 
stack_sz = coroutines[coro_pid].stack_sz;

int stack_sz = coroutines[coro_pid].stack_sz;

Syntax:

-
You use print() where it is unneeded.

printf("starting the scheduler...\n");

You can use puts() instead.

puts("Starting the scheduler...");

Memory:

-
You allocated memory to stack, but never free it.

void *stack = malloc(stack_sz);

Failure to deallocate memory using free leads to buildup of non-reusable memory, which is no longer used by the program. This wastes memory resources and can lead to allocation failures when these resources are exhausted.

free(stack);

Comments:

-
You could use the / ... / comment format instead of the // ... format when commenting over multiple lines.

// value == 0 means just starting
// value == -1 means spawning new coroutine
// value positive means hop back to a specific pid

You could also condense your comments down a bit.

/* 
 * If 'value' is 0, it is just starting; if -1 it is spawning.
 * If 'value' is positive, we "hop" to a specific PID.
 */

Resources:

• Coroutine implementations for C

• Coroutines in C

• Building Coroutines

• Libtask: a Coroutine Library for C and Unix

StackExchange Code Review Q#30214, answer score: 7