2

I have been beating my head on a wall for a several hours now trying to find what is causing this segfault.

I have found that the segfault occurs consistantly on the pthread_mutex_lock(lock) line (38). I have placed two print statements surrounding the lock however, only one of them prints, which is my justification for concluding that the segfault occurs at that instruction.

Am I using the mutex lock correctly, or am I making a mistake with my arrays (buffer[] and numbermarker[]?

#include <stdio.h>
#include <stdlib.h>
#include <pthread.h>

int* numbermarker = NULL;
int* buffer = NULL;
int pullposition = 0;
int placeposition = 1;
pthread_mutex_t *lock;
int ceiling;

/*This method places one of the primes in the buffer. It 
offers a safe way to manage where the next value will be placed*/
void placevalue(int value)
{
    buffer[placeposition] = value;
    placeposition++;
}

/*This method pulls the next prime and increments to the next prime in the list*/
int takevalue()
{
    pullposition++;
    return buffer[pullposition-1];
}


void* threadmethod()
{   
    int k;
    int l;
    int firstval;
    while(1)
    {
        while(numbermarker[buffer[pullposition]-1]==0)
        {   
            printf("flag1 \n");
            pthread_mutex_lock(lock);
            printf("flag2 \n");
              numbermarker[buffer[pullposition]-1] = 1;
              l = takevalue();
            pthread_mutex_unlock(lock);
            firstval = 1;
            for(k=l+1;k<=ceiling;k++)
            {
                if(k%l != 0)
                {
                    if(firstval)
                    {
                        placevalue(k);
                        firstval = 0;
                    }
                }
                else
                {
                    numbermarker[k-1] = 1;
                }
            }
        }
    }
}


int main()
{
int numthreads;
int i;

printf("Enter number of threads: \n");
scanf("%d", &numthreads);

printf("Enter the highest value to check \n");
scanf("%d", &ceiling);

    /* This will hold 1's and 0's.
        1 = number has been checked or is
            confirmed not to be a prime
        0 = number is a possible prime

    The idea behind these values is that the next
    prime can always be identified by the 0 with
    the lowest index*/
numbermarker = (int*)malloc(sizeof(int)*(ceiling));

    /*This will hold the primes as they are found*/
buffer = (int*)malloc(sizeof(int)*(ceiling));

for(i=0; i<ceiling; i++)
{
    if(i<1)
    {
        numbermarker[i] = 1;
    }
    else
    {
        numbermarker[i] = 0;
    }

    buffer[i]=0;
    printf("%d \n",numbermarker[i]);
}

placevalue(2);

pthread_t **tid = (pthread_t **) malloc(sizeof(pthread_t *) * numthreads);


for(i=0;i<numthreads;i++)
{

    tid[i] = (pthread_t *) malloc(sizeof(pthread_t));
}

for(i=0;i<numthreads;i++)
{

    if(pthread_create(tid[i],
              NULL,
              threadmethod,
              NULL))
    {

        printf("Could not create thread \n");
        exit(-1);
    }
}       


int not_done = 1;
int sum;
while(not_done)
{
    sum = 0;    
    for(i=0; i<ceiling; i++)
    {
        sum += numbermarker[i];
    }
    if(sum == ceiling)
      not_done = 0;
}
for(i=0;i<numthreads;i++)
{
    if(pthread_join(*tid[i], NULL))
    {
        printf("Error Joining with thread \n");
    }
    free(tid[i]);
}
free(tid);




for(i=0;i<ceiling;i++)
{
    if(buffer[i] != 0);
        printf("%d \n", i);
}
free(buffer);
free(numbermarker);
buffer=NULL;
numbermarker=NULL;


return(0);

}

il_guru
  • 8,383
  • 2
  • 42
  • 51
born4 battle
  • 51
  • 1
  • 2
  • Please [don't cast the return value of `malloc()` in C](http://stackoverflow.com/a/605858/28169). Thanks. – unwind Apr 05 '13 at 07:54

1 Answers1

5

lock is an uninitialised pointer. You need to allocate memory for then initialise a mutex before locking it. The simplest fix would be to change

pthread_mutex_t *lock;

to

pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;

and (since lock is no longer a pointer)

pthread_mutex_lock(lock);
....
pthread_mutex_unlock(lock);

to

pthread_mutex_lock(&lock);
....
pthread_mutex_unlock(&lock);
simonc
  • 41,632
  • 12
  • 85
  • 103
  • @jimmcnamara You just beat me to it, now edited to include this – simonc Apr 05 '13 at 07:54
  • I did some further research and found a different fix that keeps the locks as pointers: I needed to allocate space for the lock, then use the the pthread initializer method: `lock = (pthread_mutex_t*) malloc (sizeof(pthread_mutex_t));` `pthread_mutex_init(lock,NULL);` – born4 battle Apr 05 '13 at 08:01