Use of set_jmp/longjmp in C++ is not working

Question

I am trying to implement simple user level thread library in c.when one thread start and this thread call second thread. this second thread run correctly but when it exit program crash.here is my coding.

//**********************************************
#include <setjmp.h>

typedef void *any_ptr;
/* Define Boolean type, and associated constants. */
typedef int Boolean;
typedef void (*ThreadFunc)(any_ptr);

#define TRUE ((Boolean)1);
#define FALSE ((Boolean)0);

typedef struct TheadSystem
{
  queue<any_ptr> readyQ;
  // currently executing thread
  jmp_buf lastContext; // location on which the system jumps after all threads have exited
  char name[30]; // optional name string of a thread, may be used for debugging
  jmp_buf context;   // saved context of this thread
  signal_t *sig; // signal that wakes up a waiting thread
  ThreadFunc func; // function that this thread started executing
  any_ptr    arg;
}TheadSystem;

void t_start(ThreadFunc f, any_ptr v, char *name);
void t_yield();
void block();
void unblock();
void t_sig(Condition cond, any_ptr val, Boolean queue_signal);
void t_fork(ThreadFunc f, any_ptr v, char *name);
void t_exit(int val);

My implementation of threads.h

#include "threads.h"
#include<iostream>
#include<queue>

using namespace std;
TheadSystem th;

queue<any_ptr> blocked_queue;
jmp_buf set_env,ready_env,yeild_buf;

void t_start(ThreadFunc f, any_ptr v, char *name){  
  if(!th.ready_queue.empty()){
    cout<<"sorry thread already started now you have to create by t_fork:"<<endl;
  }
  else{
    th.ready_queue.push(th.context);
      if(!setjmp(th.context)){
        memcpy(th.lastContext,th.context,sizeof(jmp_buf));
        th.arg=v;
        th.func=f;

        //memcpy(th.currentThread->context,set_env,sizeof(jmp_buf));
        //cout<<"when jmp buf set then:"<<endl;
        th.ready_queue.push(th.context);
        th.func(th.arg);
      }
    //cout<<"after come back from long jump:"<<endl;
  }
}

void t_yield(){
  jmp_buf *j=(jmp_buf *)th.ready_queue.front();
  th.ready_queue.front()=th.context;

  longjmp(*j,2);
}

void t_fork(ThreadFunc f, any_ptr v, char *name){
  memcpy(th.lastContext,th.context,sizeof(jmp_buf));
  if(!setjmp(th.context)){
    f(v);
    th.ready_queue.push(th.context);
  }else
  {
  }
}//end of t_fork

void t_exit(int val){   
  cout<<"before long jump in t_exit"<<endl;
  jmp_buf *j=(jmp_buf *)th.ready_queue.front();
  th.ready_queue.pop();
  longjmp(*j,2);    
}

void block(){
  blocked_queue.push(th.context);
  jmp_buf *j=(jmp_buf *)th.ready_queue.front();
  th.ready_queue.pop();
  longjmp(*j,2);    
}

void unblock(){
  th.ready_queue.push(th.context);
  jmp_buf *j=(jmp_buf *)blocked_queue.front();
  blocked_queue.pop();
  longjmp(*j,2);    
}

my test case is

#include<iostream>
#include<setjmp.h>
#include<stdio.h>
#include "threads.h"
#include<queue>

using namespace std;    
void fun2(any_ptr v){
  cout<<"in 2nd function:"<<endl;
  t_exit(0);
}

void helloworld(any_ptr v){
  cout<<"in hello world from start"<<endl;
  t_fork(fun2,NULL,"no value");
  cout<<"after second thread:"<<endl;
  cout<<"before exit"<<endl;
  t_exit(0);
}

void main(){
  cout<<"1 start"<<endl;
  t_start(helloworld, NULL, "my function");
  cout<<"main function"<<endl;
}//end of void main

Answer 1

Here is one problem:

In the t_start function you do this:

th.ready_queue.push(th.context);

The ready_queue is a queue of pointers, but th.context is not a pointer.

Then in the t_yield function you do

jmp_buf *j=(jmp_buf *)th.ready_queue.front();

So you push non-pointer object, and pop them as pointers. If you try to access a non-pointer object as a pointer you have undefined behavior.

You code, if it compiles without errors, should at least give you a lot of warnings, and if you only get a few warnings then I suggest you enable more warnings. When the compiler gives you a warning, it's often a sign about you doing something you should not be doing, like doing something that leads just to undefined behavior. Just silencing the warnings by e.g. type-casting is a very bad solution as it doesn't actually solve the cause of the warning.

Also, using void* is a very good sign of bad code coming. Don't use it if you can avoid it, and in this case it's really not needed in most of the places you use it (like the ready_queue).

Answer 2

There are SEVERAL problems with this code, some of which Joachim Pileborg points out.

Another problem is that you only have one context, which you are using multiple times to store different data, yet expect the data to be there when you come back.

The solution is to split your ThreadSystem and your Thread (the actual context of a thread) into separate objects:

struct Thread
{
    jmp_buf context;   // saved context of this thread
    void*    arg;
    ThreadFunc func; // function that this thread started executing
};

After removing stuff that isn't currently used, the ThreadSystem looks like this:

struct ThreadSystem
{
    queue<Thread*> ready_queue;
};

The thread creation/exit functions now look like this:

void t_start(ThreadFunc f, void* v)
{    
    if(!sys.ready_queue.empty()){
        cout<<"sorry thread already started now you have to create by t_fork:"<<endl;
    }
    else{
    Thread* th = new Thread;
        sys.ready_queue.push(th);


        if(!setjmp(th->context)){
            th->arg=v;
            th->func=f;

        cout << "&th->context=" << &th->context << endl;
            th->func(th->arg);
        }
    }
}

void t_fork(ThreadFunc f, void* v){
    Thread* th = new Thread; 
    th->func = f;
    th->arg = v;
    if(!setjmp(th->context))
    {
    cout << "&th->context=" << &th->context << endl;
        f(v);
        sys.ready_queue.push(th);
    }
}//end of t_fork

void t_exit(int val){   
    cout<<"before long jump in t_exit"<<endl;
    Thread* th=sys.ready_queue.front();
    sys.ready_queue.pop();
    // Memory leak here. We can't delete `th`, and still have a context.
    longjmp(th->context,2);  
}

But as you can see, there is a problem in destroying the thread - so some other solution would have to be found for this. I'm not sure this is a great solution, but this works (to the limited degree of executing the test-code posted), where the original code didn't.

Answer 3

OK. My first pass at this was inadequate as I didn't spend sufficient time understanding the original code.

The code is buggy and messy, but probably fixable. When you push th.context onto the ready_queue you need to save the whole buffer, not just the buffer address. Probably many other problems.

Update 1

Solved first problem by wrapping the jmp_buf in a struct declaration and then making ready_queue and blocked_queue queues of structs. Then a simple assign will transfer the buffer contents.

struct SJBuff
{
    jmp_buf jb;
};

Second problem: in t_start(), don't push th.context before it is first initialised.

else
{
    // remove this line
    // th.readyQ.push(th.context);

    if(!setjmp(th.context.jb))
    {

End Update 1

Notwithstanding that, I really cannot recommend using setjmp(). Modern architectures have moved on and just saving a few registers does not really capture enough state. I shudder to think what an optimizing compiler might do to your code. Piplining, conditional execution, lazy evaluation, extra registers, unscheduled system interrupts, ...

If you focus on your real objectives, there is probably a better way to do it.