I'm trying to make a multi-threaded prime numbers counter program. I've added the critical codes for the understanding (the parseargs function works great in another program and feels unnecessary and I don't want to overload you with code).
Long story short, the program compile and work well with small numbers (up to 1000 max val and 8 threads), but when I'm trying to go with bigger numbers I keep getting a lot of errors like:
segmentation fault: core dumped, Floating point exception, realloc(): invalid next size.
What's wrong with the code? I know that I should use fewer global variables but I'm trying to make it work and then I will make cleaner and better. And the only place that I'm using realloc is inside of isprime; that works great in my other programs. What's the problem there?
#include <ctype.h>
#include <stdio.h>
#include <stdlib.h>
#include <math.h>
#include <unistd.h>
#include <pthread.h>
#include <sys/mman.h>
void parseargs(char *argv[], int argc, int *lval, int *uval, int *nval, int *tval);
int isprime(int n);
void setFlags();
pthread_mutex_t num_lock;
pthread_mutex_t count_lock;
char *flagarr = NULL;
int num;
int lval = 1;//min value
int uval = 100;//max value
int count = 0;//primes counter
int main (int argc, char **argv)
{
int nval = 10;// how much numbers to print
int tval = 4;// threads number
num = lval;
if (pthread_mutex_init(&num_lock, NULL) != 0) {
printf("\n mutex init has failed\n");
return 1;
}
if (pthread_mutex_init(&count_lock, NULL) != 0) {
printf("\n mutex init has failed\n");
return 1;
}
// Parse arguments
parseargs(argv, argc, &lval, &uval, &nval, &tval);// works 100% (change the defult starting vals)
if (uval < lval)
{
fprintf(stderr, "Upper bound should not be smaller then lower bound\n");
exit(1);
}
if (lval < 2)
{
lval = 2;
uval = (uval > 1) ? uval : 1;
}
if (tval<1)
tval=4;
// Allocate flags
flagarr= (char *)malloc(sizeof(char) * (uval-lval 1));
if (flagarr == NULL)
exit(1);
//Allocate threads
pthread_t* t = (pthread_t *)malloc(sizeof(pthread_t)*tval);
if (t == NULL)
exit(1);
//start threads
for(int i =0; i<tval ; i ){
printf("%d\n",i);
pthread_create(&t[i],NULL,(void*)setFlags,NULL);
}
for(int i =0; i<tval ; i )
pthread_join(t[i],NULL);
// Print results
nval=count;
printf("Found %d primes from %d to %d%c", count, lval,uval , count ? ' ' : '.');
if(count && nval>0)
printf("and printing the first %d of them:\n",nval);
else
printf("\n");
for (num = lval; (num <= uval) && (nval) ; num )
if (flagarr[num - lval])
{
nval--;
count--;
printf("%d%c", num, (count && nval) ? ',' : '\n');
}
free(flagarr);
free(t);
return 0;
}
void setFlags()
{
int myNum;
for (; num <= uval;)
{
pthread_mutex_lock(&num_lock);
myNum=num;
num ;
pthread_mutex_unlock(&num_lock);
if (isprime(myNum))
{
flagarr[myNum - lval] = 1;
pthread_mutex_lock(&count_lock);
count ;
pthread_mutex_unlock(&count_lock);
} else {
flagarr[myNum - lval] = 0;
}
}
}
int isprime(int n)
{
static int *primes = NULL; // NOTE: static !
static int size = 0; // NOTE: static !
static int maxprime; // NOTE: static !
int root;
int i;
// Init primes array (executed on first call)
pthread_mutex_lock(&first_lock);
if (primes == NULL)
{
primes = (int *)malloc(2*sizeof(int));
if (primes == NULL)
exit(1);
size = 2;
primes[0] = 2;
primes[1] = 3;
maxprime = 3;
}
pthread_mutex_unlock(&first_lock);
root = (int)(sqrt(n));
// Update primes array, if needed
while (root > maxprime)
for (i = maxprime 2 ; ; i =2)
if (isprime(i))
{
pthread_mutex_lock(&primeFunc_lock);
size ;
primes = (int *)realloc(primes, size * sizeof(int));
if (primes == NULL)
exit(1);
primes[size-1] = i;
maxprime = i;
pthread_mutex_unlock(&primeFunc_lock);
break;
}
// Check 'special' cases
if (n <= 0)
return -1;
if (n == 1)
return 0;
// Check prime
for (i = 0 ; ((i < size) && (root >= primes[i])) ; i )
if ((n % primes[i]) == 0)
return 0;
return 1;
}
CodePudding user response：
I know that I should use fewer global variables but I'm trying to make it work and then I will make cleaner and better.
For multithreaded programs this approach doesn't work -- you can't write a buggy program first, and make it pretty and bug-free later -- you have to get it bug-free first.
Also, global and static variables and threads generally don't mix.
Now, as others have noted, you access static variables in isprime() without any locks. Think about what would happen when thread T1 executes this code:
for (i = 0 ; ((i < size) && (root >= primes[i])) ; i )
if ((n % primes[i]) == 0)
and another thread T2 executes this statement at the same time:
primes = (int *)realloc(primes, size * sizeof(int));
Since primes is not an atomic variable, the compiler will (likely) load the value of primes into some register, and continue using that value through the entire loop in T1. The loop will be totally oblivious to the fact that primes may have already been free()d and realloc()ed in T2, and now points to entirely different memory block. T1 will continue reading the old (now dangling) memory.
Similar problems exist throughout your isprimes() function, and easily explain your observed crashes.
If you are on a Linux system, Thread sanitizer (and Address sanitizer) are your friends.