patterncMinor
Thread synchronization with mutex
Viewed 0 times
withsynchronizationthreadmutex
Problem
This program prints odd numbers by
thread1 and even numbers by thread2 sequentially. Can this code be optimized or made more efficient?#include
#include
#include
pthread_mutex_t count_mutex = PTHREAD_MUTEX_INITIALIZER;
pthread_cond_t condition_var = PTHREAD_COND_INITIALIZER;
void *function_count1();
void *function_count2();
int count = 1;
int main()
{
pthread_t thread1, thread2;
int ret;
if( (ret=pthread_create( &thread1, NULL, &function_count1, "thread1")) )
{
printf("Thread 1 creation failed: %d\n", ret);
exit (EXIT_FAILURE);
}
if( (ret=pthread_create( &thread2, NULL, &function_count2, "thread2")) )
{
printf("Thread 2 creation failed: %d\n", ret);
exit (EXIT_FAILURE);
}
pthread_join( thread1, NULL);
pthread_join( thread2, NULL);
exit(EXIT_SUCCESS);
}
void *function_count1(void *arg)
{
while(count <= 10)
{
pthread_mutex_lock( &count_mutex );
if (count % 2 != 0)
{
pthread_cond_wait(&condition_var, &count_mutex);
printf("%s : counter = %d\n", (char *)arg, count++);
}
else
{
pthread_cond_signal(&condition_var);
}
pthread_mutex_unlock( &count_mutex );
}
}
void *function_count2(void *arg)
{
while(count <= 10)
{
pthread_mutex_lock( &count_mutex );
if (count % 2 == 0)
{
pthread_cond_wait(&condition_var, &count_mutex);
printf("%s : counter = %d\n", (char *)arg, count++);
}
else
{
pthread_cond_signal(&condition_var);
}
pthread_mutex_unlock(&count_mutex);
}
}Solution
-
Since
Notice that I'm calling
Also, while you'd normally not need all this code for printing an error, it's necessary since
For code that does set
This would also eliminate the need for variables like
-
The functions called by
-
There's no need to call
-
This isn't a criticism, but a tip: make sure you're running this through valgrind to check for any memory leaks along the way.
Actually, this may already cause a leak, but I haven't checked to make sure. If it does, then you may need to call both
Since
pthread_create() returns an error number on failure, you can put ret into strerror() to get the associated error message instead of just printing the number:fprintf(stderr, "Thread 1 creation failed: %s\n", strerror(ret));Notice that I'm calling
fprintf() instead of printf(). Since you're printing an error, it must be sent to stderr, not stdin (used by default with printf()).Also, while you'd normally not need all this code for printing an error, it's necessary since
pthread_create() doesn't set errno (which is why ret is useful here).For code that does set
errno (which would be stated on man pages), you'd only need to call perror() to print the corresponding error message:perror("");This would also eliminate the need for variables like
ret in such code. However, with pthread_create() and related functions, you could probably just put errno in its place and then use perror().-
The functions called by
pthread_create() should still return something. Since the return value won't be useful here, you can just return NULL.-
There's no need to call
exit() right at the end of main(). You're preventing it from returning a value, and it'll always return 0 at this point anyway.-
This isn't a criticism, but a tip: make sure you're running this through valgrind to check for any memory leaks along the way.
Actually, this may already cause a leak, but I haven't checked to make sure. If it does, then you may need to call both
pthread_mutex_destroy() and pthread_cond_destroy() before exiting the program at any point.Code Snippets
fprintf(stderr, "Thread 1 creation failed: %s\n", strerror(ret));perror("<insert source of failure here>");Context
StackExchange Code Review Q#129601, answer score: 5
Revisions (0)
No revisions yet.