Using mutex locks incorrectly in C++ somehow

Question

I have the following code to try (and fail) to protect a critical region in my code. Basically I want a print statement to be fully executed by any thread before some other thread prints another statement.

In main.h:

pthread_mutex_t printer_mutex;

In main.c - the first thing the program does:

pthread_mutex_init(&printer_mutex, NULL);

The threads are created like so (I've removed some of the checks though):

for(long t = 0; t < NUM_THREADS; t++) {
   args[t].fw = fw;
   args[t].deriv_init = deriv_init;
   args[t].to_prove = fw.to_prove.at(i);
   pthread_create(&threads[t], NULL, do_derivation_helper, (void *) &args[t]);
}
for(long t = 0; t < NUM_THREADS; t++) {
  pthread_join(threads[t], NULL);
}

Then in a class of mine that does all the printing in the program I have the following. The printing should be locked and then unlocked when it is finished.

void InfoViewer::rule_x_fires() {
    // START CRITICAL REGION
    pthread_mutex_lock(&printer_mutex);
    cout << "INFO: Rule x fires" << endl;
    pthread_mutex_unlock(&printer_mutex);
    // END CRITICAL REGION
}

The undesired output I get is:

INFO: Rule x firesINFO: Rule x fires

I.e. the line is not ended before some other thread starts printing.

Any ideas? Am I not initialising correctly? Am I ok using standard C style threads in my C++ program?

Answer 1

I believe the problem is in defining the mutex variable in the header file. That way every compilation unit gets its own version of such variable, and you just lock different mutexes. Simply do this:

// .h header file: declare
extern pthread_mutex_t printer_mutex;

// one of the .c/.cpp files: define
pthread_mutex_t printer_mutex;

Edit 0:

To explain why it "worked" with multiple definitions - PThread mutex could be initialized two different ways - dynamically with pthread_mutex_init, and statically with PTHREAD_MUTEX_INITIALIZER. Looking into pthread.h one can find the following (32-bit version):

# define PTHREAD_MUTEX_INITIALIZER \
    { { 0, 0, 0, 0, 0, { 0 } } }

This means that any static variable of type pthread_mutex_t will be correctly initialized without any explicit value assigned, or init call due to the fact that static memory is zero-filled. So you explicitly dynamically initialized one mutex, all others were implicitly initialized to all zeros.

Answer 2

You can always change that use of std::cout to printf(). You are on a Mac, a POSIX-compliant OS. printf() is guaranteed to be atomic. In fact, you shouldn't even need a mutex if you can use printf such that all of the output to be produced at one time is performed by a single printf() call. There are no guarantees with std::cout. None.

If you do convert this particular use of std::cout to printf you should do that throughout your application. Mixing std::cout and printf is not the best idea.

Answer 3

That's an interesting problem. I don't think this addresses the root problem (synchronization in ostream), but try this:

void InfoViewer::rule_x_fires() {
    // START CRITICAL REGION
    pthread_mutex_lock(&printer_mutex);
    cout << "INFO: Rule x fires" << endl;
    cout.flush();
    pthread_mutex_unlock(&printer_mutex);
    // END CRITICAL REGION
}