Your code is dangerous for several reasons:
input file
You use the same stream for read operations. That is, if in a thread you make a reading the internal cursor of the stream will move ... in all threads . Operations on entrada
should be considered critical regions and this is not the case. This can cause the appearance of dirty readings (if error flags or EOF
are activated) or jumps (numbers that are not read in a thread.
To solve this problem you have two options:
If you need each thread to process the file completely, let each thread open the file in read mode (the reading modes should not be blocking each other). With this action you get each thread to handle its own stream , thus avoiding the aforementioned problems.
If the threads must compete for the data in the file, implement critical regions. You can do this very easily using pthread_mutex
. In this case you should avoid using rewind
.
Dirty writings
The console output is also not protected in a critical region. This can cause the wire exits to step on each other. If two threads tried to print "Hola\n"
a possible result could be:
HoHlola
a
This problem can only be solved by defining critical regions, which is nothing more than parts of the code that can only be executed by one thread at a time. An example:
pthread_mutex_t lock;
void *recorreNumero (FILE *entrada, char nomT[], int numT){
int t = 0, numero = 0 ,primo = -1;
char linea[13];
FILE *salida;
rewind(entrada);
fgets(linea,sizeof(linea),entrada);
salida = fopen (nomT,"w");
while (fscanf(entrada, "%i", &numero) != EOF){
t++;
pthread_mutex_lock(&lock); // Inicio region critica
printf("%i\n",numero );
pthread_mutex_unlock(&lock); // Fin region critica
}
int lineasT = t/numT;
int cont=1;
rewind(entrada);
fgets(linea,sizeof(linea),entrada);
while (fscanf(entrada, "%i", &numero) != EOF){
pthread_mutex_lock(&lock);
printf("Extraido: %i \n", numero);
pthread_mutex_unlock(&lock);
primo = numprimo(numero);
fprintf(salida, "%i %i\n",numero,primo);
pthread_mutex_lock(&lock);
printf("%i\n",primo);
pthread_mutex_unlock(&lock);
cont++;
if (cont == lineasT)
break;
}
pthread_mutex_lock(&lock);
printf("Numero de filas: %d %i %s \n",t,primo,nomT);
pthread_mutex_unlock(&lock);
fclose(salida);
}
The structure lock
must be global because it has to be shared by all threads ... if you move it within the function then each thread will define its own critical region and the problem will be reproduced again.
In any case, it would not hurt to simplify the initialization of the threads, paying attention to the comments made by @EddieRuiz
try to implement a thread_exit(NULL)
at the end of recorreNumero
but this only allows you to run the first thread but not the second one
That instruction kills the current process, but at this point the stream has left the input to the second thread. This function is intended to be used in complex situations in which leaving the function recorreNumero
is not as simple as putting a return
.
Also think that this function must be used with care, since resources can be left open (files, sockets, ...).
also place at the end of recorreNumero
a return NULL
to completely end the procedure but it also does not work the violation of the segment is still present.
Since the function has as output type void*
you should put a return
ALWAYS .
The return
is not going to mark that the thread stops giving problems, but it is a good practice that can avoid unpleasant surprises. Think that the function is going to return something yes or yes ... and if you do not tell him what he must return he will take a value from somewhere ... and the same you do not like his choice.