[Valgrind-developers] valgrind/corecheck/tests [POSSIBLY UNSAFE]

[Nicholas N. <nj...@ca...>]

CVS commit by nethercote: 

Improve pth_cvsimple test in two ways:

1. Make the output deterministic;  different thread interleaving from expected
was causing failures for me.

2. Make it actually use the condition variable -- the condvar stupidly wasn't
actually being used in the expected case, because the other threads finished
all their work before pthread_cond_wait() even got called, and this prevented
the condition guarding pthread_cond_wait() from succeeding.


  M +22 -15    pth_cvsimple.c   1.3 [POSSIBLY UNSAFE: printf]
  M +22 -22    pth_cvsimple.stdout.exp   1.3


--- valgrind/corecheck/tests/pth_cvsimple.c  #1.2:1.3
@@ -10,5 +10,5 @@
  * cvsimple.c
  *
- * Demonstrates pthread cancellation.
+ * Demonstrates pthread condvars.
  *
  */
@@ -21,4 +21,5 @@
 #define COUNT_THRES 12
 
+int     condvar_was_hit = 0;
 int     count = 0;
 int     thread_ids[3] = {0,1,2};
@@ -26,16 +27,14 @@ pthread_mutex_t count_lock=PTHREAD_MUTEX
 pthread_cond_t count_hit_threshold=PTHREAD_COND_INITIALIZER; 
 
-void *inc_count(void *idp)
+void *inc_count(void *null)
 {
   int i=0;
-  int *my_id = idp;
 
   for (i=0; i<TCOUNT; i++) {
     pthread_mutex_lock(&count_lock);
     count++;
-    printf("inc_counter(): thread %d, count = %d, unlocking mutex\n", 
-           *my_id, count);
+    printf("inc_counter(): count = %d, unlocking mutex\n", count);
     if (count == COUNT_THRES) {
-      printf("inc_count(): Thread %d, count %d\n", *my_id, count);
+      printf("hit threshold!\n");
       pthread_cond_signal(&count_hit_threshold);
     }
@@ -46,15 +45,11 @@ void *inc_count(void *idp)
 }
 
-void *watch_count(void *idp)
+void *watch_count(void *null)
 {
-  int *my_id = idp;
-
-  printf("watch_count(): thread %d\n", *my_id);
-  fflush(stdout);
   pthread_mutex_lock(&count_lock);
 
   while (count < COUNT_THRES) {
     pthread_cond_wait(&count_hit_threshold, &count_lock);
-    printf("watch_count(): thread %d, count %d\n", *my_id, count);
+    condvar_was_hit = 1;
   }
 
@@ -70,7 +65,7 @@ main(void)
   pthread_t threads[3];
 
-  pthread_create(&threads[0], NULL, inc_count, (void *)&thread_ids[0]);
-  pthread_create(&threads[1], NULL, inc_count, (void *)&thread_ids[1]);
-  pthread_create(&threads[2], NULL, watch_count, (void *)&thread_ids[2]);
+  pthread_create(&threads[0], NULL, watch_count, NULL);
+  pthread_create(&threads[1], NULL, inc_count,   NULL);
+  pthread_create(&threads[2], NULL, inc_count,   NULL);
 
   for (i = 0; i < NUM_THREADS; i++) {
@@ -78,4 +73,16 @@ main(void)
   }
 
+  // Nb: it's not certain that we'll hit here.  It's possible that the two
+  // inc_count threads could fully run before watch_count begins, and so
+  // pthread_cond_wait() is never called.  Or, we could get a spurious
+  // wake-up in watch_count().  Nonetheless, it's very likely that things
+  // will work out as expected, since we're starting watch_count() first.
+  if (condvar_was_hit == 1)
+    printf("condvar was hit!\n");
+  else if (condvar_was_hit > 1)
+    printf("condvar was multi-hit...\n");
+  else
+    printf("condvar was missed...\n");
+  
   return 0;
 }

--- valgrind/corecheck/tests/pth_cvsimple.stdout.exp  #1.2:1.3
@@ -1,22 +1,22 @@
-inc_counter(): thread 0, count = 1, unlocking mutex
-inc_counter(): thread 0, count = 2, unlocking mutex
-inc_counter(): thread 0, count = 3, unlocking mutex
-inc_counter(): thread 0, count = 4, unlocking mutex
-inc_counter(): thread 0, count = 5, unlocking mutex
-inc_counter(): thread 0, count = 6, unlocking mutex
-inc_counter(): thread 0, count = 7, unlocking mutex
-inc_counter(): thread 0, count = 8, unlocking mutex
-inc_counter(): thread 0, count = 9, unlocking mutex
-inc_counter(): thread 0, count = 10, unlocking mutex
-inc_counter(): thread 1, count = 11, unlocking mutex
-inc_counter(): thread 1, count = 12, unlocking mutex
-inc_count(): Thread 1, count 12
-inc_counter(): thread 1, count = 13, unlocking mutex
-inc_counter(): thread 1, count = 14, unlocking mutex
-inc_counter(): thread 1, count = 15, unlocking mutex
-inc_counter(): thread 1, count = 16, unlocking mutex
-inc_counter(): thread 1, count = 17, unlocking mutex
-inc_counter(): thread 1, count = 18, unlocking mutex
-inc_counter(): thread 1, count = 19, unlocking mutex
-inc_counter(): thread 1, count = 20, unlocking mutex
-watch_count(): thread 2
+inc_counter(): count = 1, unlocking mutex
+inc_counter(): count = 2, unlocking mutex
+inc_counter(): count = 3, unlocking mutex
+inc_counter(): count = 4, unlocking mutex
+inc_counter(): count = 5, unlocking mutex
+inc_counter(): count = 6, unlocking mutex
+inc_counter(): count = 7, unlocking mutex
+inc_counter(): count = 8, unlocking mutex
+inc_counter(): count = 9, unlocking mutex
+inc_counter(): count = 10, unlocking mutex
+inc_counter(): count = 11, unlocking mutex
+inc_counter(): count = 12, unlocking mutex
+hit threshold!
+inc_counter(): count = 13, unlocking mutex
+inc_counter(): count = 14, unlocking mutex
+inc_counter(): count = 15, unlocking mutex
+inc_counter(): count = 16, unlocking mutex
+inc_counter(): count = 17, unlocking mutex
+inc_counter(): count = 18, unlocking mutex
+inc_counter(): count = 19, unlocking mutex
+inc_counter(): count = 20, unlocking mutex
+condvar was hit!