4

I'm trying to implement a class that creates a thread, increments a value and sends it to another thread, which number is defined as (value * value) % number of threads

#include <iostream>
#include <pthread.h>
#include <unistd.h>
#include <cstdlib>
#include <vector>

pthread_mutex_t m = PTHREAD_MUTEX_INITIALIZER;
pthread_cond_t cv = PTHREAD_COND_INITIALIZER;
volatile int counter = 0;
volatile int maxval = 0;
volatile int next = 0;

extern "C" void *func(void *p);

class Worker {
private:
    pthread_t thread = 0;
    int nth = 0;
    int nproc = 0;
public:
    Worker () {};
    Worker(int _nproc, int _nth) {
        nth = _nth;
        nproc = _nproc;
    };
    void start() {
        pthread_create(&thread, NULL, func, NULL); // Line 27

    };
    void wait() {
        while (nth != next) {
            pthread_cond_wait(&cv, &m);
        }
    };
    void notify() {
        pthread_cond_broadcast(&cv);
    };
    void run() {
        while (counter != maxval) {
            pthread_mutex_lock(&m);
            Worker::wait();
            if (counter != maxval) {
                printf("%d %d\n", nth, counter);
                ++counter;
            }
            next = (counter * counter) % nproc;
            Worker::notify();
            pthread_mutex_unlock(&m);
        }
    };
    void join() {
        pthread_join(thread, NULL);
    }
};

extern "C" void *func(void *p) {
    Worker *w = reinterpret_cast<Worker*>(p);
    w->run();
    return NULL;
}

int main(int argc, char *argv[]) {
    int nthreads = atoi(argv[1]);
    maxval = atoi(argv[2]);
    std::vector<Worker> workers;
    for (int i = 0; i != nthreads; ++i) {
        workers.push_back(Worker(nthreads, i));
    }
    for (int i = 0; i != workers.size(); ++i) {
        workers[i].start();
    }
    for (int i = 0; i != workers.size(); ++i) {
        workers[i].join();
    }
    return 0;
}

Can't check if the algorithm is correct since I get a Segmentation Error when I call pthread_create (line 27)

This is what gdb said:

#0  0x0000000000400f5a in Worker::wait (this=0x0) at ht19-4.cpp:30
#1  0x0000000000400fd5 in Worker::run (this=0x0) at ht19-4.cpp:40
#2  0x0000000000400d26 in func (p=0x0) at ht19-4.cpp:57
#3  0x00007ffff76296aa in start_thread (arg=0x7ffff6f4f700)
    at pthread_create.c:333
#4  0x00007ffff735eeed in clone ()
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109

Could anyone explain what exactly happens in this function please? How do I implement this correctly?

Thanks a lot.

Foggzie
  • 9,691
  • 1
  • 31
  • 48

2 Answers2

2

You're func doesn't handle NULL being passed in and the 4th argument of pthread_create is what gets sent the function in the third argument.

Change func to properly handle NULL and you should be good:

extern "C" void *func(void *p) {
    if (NULL == p)
        return NULL;
    Worker *w = reinterpret_cast<Worker*>(p);
    w->run();
    return NULL;
}

Also, +1 for posting gdb output so here's some more information:

If we follow your stack trace upwards (I'm typing them in the order they happen) we see that func does its job of casting and calling run on the Worker it casted from NULL. Notice the (p=0x0) and (this=0x0):

#2  0x0000000000400d26 in func (p=0x0) at ht19-4.cpp:57
#1  0x0000000000400fd5 in Worker::run (this=0x0) at ht19-4.cpp:40

Worker::run works fine because it only accesses counter, maxval, and m before getting to Worker::wait()

#0  0x0000000000400f5a in Worker::wait (this=0x0) at ht19-4.cpp:30

In Worker::wait() you are accessing nth which is a member of the null Worker instance and you finally get your segfault.

Foggzie
  • 9,691
  • 1
  • 31
  • 48
2

The problem is while creating thread you are passing NULL for last argument.

pthread_create(&thread, NULL, func, NULL);

While the thread is started it calls func() with NULL value passed to it. So inside func() p is NULL and you are trying to cast and then access that memory location. That's why you're getting segmentation fault.

alk
  • 69,737
  • 10
  • 105
  • 255
Vikash Gupta
  • 121
  • 7