pthread_create start routine not executing

Question

I'm trying to create a train station simulation for an assignment at school in C. This is an exercise in understanding threaded programming. Here is my current code:

#include <stdio.h>
#include <stdlib.h>
#include <pthread.h>
#include <string.h>
#include <readline/readline.h>

void *train_function(void *args) {
    printf("Train created!\n");
    pthread_exit(0);
}

int main() {
    FILE *ptr_file;
    char buff[10];
    int ch;
    int train_count = 0;

    ptr_file = fopen("./trains.txt", "r");

    if (!ptr_file)
        return 0;

    /* Count number of trains */
    while(!feof(ptr_file))
    {
        ch = fgetc(ptr_file);
        if(ch == '\n')
        {
            train_count++;
        }
    }
    pthread_t trains[train_count];

    /* Create train for each line of file */
    int train_index = 0;
    while (fgets(buff,10, ptr_file)!=NULL) {
        pthread_create(&trains[train_index], NULL, train_function, NULL);
        train_index++;
    }
    fclose(ptr_file);

    /* Wait for all trains to leave the station */
    for (int x = 0; x < train_count; x++) {
        pthread_join(trains[x], NULL);
    }
    exit(0);
}

The code reads information about trains from a file trains.txt and creates a new thread for each line of the file(each train).

e:10,6
W:5,7
E:3,10

I would expect the output to be "Train Created!" three times. Compiling the code produces a Segmentation fault. What am I missing?

Use a debugger to find out where the seg fault occurs. At the minimum you should protect against overflowing the `trains` array. You assume that your logic is correct and that `train_count` is correctly set. That's a dangerous assumption to make and robust code would protect against that in at least the `fgets` loop. — kaylum, Jun 15 '15 at 20:52

score 3 · Accepted Answer · edited May 23 '17 at 12:29

 while (fgets(buff,10, ptr_file)!=NULL) {
        pthread_create(&trains[train_index], NULL, train_function, NULL);
        train_index++;
    }

Here, you are reading the file again but the file pointer was already at the end of the file since you have read the whole file before this while loop. So you are not creating any threads at all but later trying to join (pthread_join calls) with non-existing threads. This is undefined behaviour. You simply need to use train_count and create threads in a for loop:

 for (int x = 0; x < train_count; x++) {
    pthread_create(&trains[x], NULL, train_function, NULL);
 }

Also, notice that the file reading part is flawed:

while(!feof(ptr_file)) {...}

See Why is “while ( !feof (file) )” always wrong? for fixing it.

score 1 · Answer 2 · answered Jun 15 '15 at 22:34

there are several little problems with the code.

The following code eliminates statements that are not used in the posted code.

Do Not use feof() as a loop control. For several reasons it fails as a loop control.

The necessary error checking is added to the code.

Notice the proper output of the correct error message to stderr via the use of the perror() function

Notice the proper error exiting via the use of the exit() function

Notice the proper exit of main, at the end, via the use of the return() function

The following code is tested and works correctly.

#include <stdio.h>
#include <stdlib.h>
#include <pthread.h>
#include <string.h>
//#include <readline/readline.h>

void *train_function(void *args __attribute__((unused)) )
{
    printf("Train created!\n");
    pthread_exit(0);
}

int main()
{
    FILE *ptr_file;
    int ch;
    int train_count = 0;

    ptr_file = fopen("trains.txt", "r");

    if (!ptr_file)
    {
        perror( "fopen for trains.txt failed" );
        exit( EXIT_FAILURE);
    }

    // implied else, fopen successful

    /* Count number of trains */
    while( (ch = fgetc(ptr_file) ) != EOF)
    {
        if(ch == '\n')
        {
            train_count++;
        }
    }
    pthread_t trains[train_count];

    /* Create train for each line of file */
    int train_index = 0;
    for ( train_index = 0; train_index < train_count; train_index++ )
    {
        if( pthread_create(&trains[train_index], NULL, train_function, NULL) )
        { // then pthread_create failed
            perror( "pthread_create failed" );
            exit( EXIT_FAILURE ); // this will cause all threads to also exit
        }

        // implied else, pthread_create successful
    }

    fclose(ptr_file);

    /* Wait for all trains to leave the station */
    for (int x = 0; x < train_count; x++)
    {
        pthread_join(trains[x], NULL);
    }
    return(0);
} // end function: main

This is great advice for someone trying to learn C. Thank you. — CaddyShack, Jun 17 '15 at 03:03

pthread_create start routine not executing

2 Answers2