Grab thread_counter_pools LOCK when increasing leftover_task_counter.

2008-01-29  Sebastian Wilhelmi  <wilhelmi@google.com>

	* tests/threadpool-test.c (test_thread_pools): Grab
	thread_counter_pools LOCK when increasing
	leftover_task_counter. Fixes race in test. (#512624, Simon Murray)

svn path=/trunk/; revision=6406

1 files changed, 78 insertions, 75 deletions

diff --git a/tests/threadpool-test.c b/tests/threadpool-test.c
index 889fee9b9..7466847af 100644
--- a/tests/threadpool-test.c
+++ b/tests/threadpool-test.c
@@ -5,16 +5,16 @@
 
 #include <glib.h>
 
-#define DEBUG_MSG(x)  
+#define DEBUG_MSG(x)
 /* #define DEBUG_MSG(args) g_printerr args ; g_printerr ("\n");  */
 
 #define WAIT                5    /* seconds */
 #define MAX_THREADS         10
 
 /* if > 0 the test will run continously (since the test ends when
- * thread count is 0), if -1 it means no limit to unused threads  
+ * thread count is 0), if -1 it means no limit to unused threads
  * if 0 then no unused threads are possible */
-#define MAX_UNUSED_THREADS -1    
+#define MAX_UNUSED_THREADS -1
 
 G_LOCK_DEFINE_STATIC (thread_counter_pools);
 
@@ -47,15 +47,15 @@ test_thread_functions (void)
 
   max_unused_threads = 3;
 
-  DEBUG_MSG (("[funcs] Setting max unused threads to %d", 
+  DEBUG_MSG (("[funcs] Setting max unused threads to %d",
 	      max_unused_threads));
   g_thread_pool_set_max_unused_threads (max_unused_threads);
 
-  DEBUG_MSG (("[funcs] Getting max unused threads = %d", 
+  DEBUG_MSG (("[funcs] Getting max unused threads = %d",
 	     g_thread_pool_get_max_unused_threads ()));
   g_assert (g_thread_pool_get_max_unused_threads() == max_unused_threads);
 
-  DEBUG_MSG (("[funcs] Getting num unused threads = %d", 
+  DEBUG_MSG (("[funcs] Getting num unused threads = %d",
 	     g_thread_pool_get_num_unused_threads ()));
   g_assert (g_thread_pool_get_num_unused_threads () == 0);
 
@@ -64,24 +64,24 @@ test_thread_functions (void)
 
   max_idle_time = 10 * G_USEC_PER_SEC;
 
-  DEBUG_MSG (("[funcs] Setting max idle time to %d", 
+  DEBUG_MSG (("[funcs] Setting max idle time to %d",
 	      max_idle_time));
   g_thread_pool_set_max_idle_time (max_idle_time);
 
-  DEBUG_MSG (("[funcs] Getting max idle time = %d", 
+  DEBUG_MSG (("[funcs] Getting max idle time = %d",
 	     g_thread_pool_get_max_idle_time ()));
   g_assert (g_thread_pool_get_max_idle_time () == max_idle_time);
 
   DEBUG_MSG (("[funcs] Setting max idle time to 0"));
   g_thread_pool_set_max_idle_time (0);
 
-  DEBUG_MSG (("[funcs] Getting max idle time = %d", 
+  DEBUG_MSG (("[funcs] Getting max idle time = %d",
 	     g_thread_pool_get_max_idle_time ()));
   g_assert (g_thread_pool_get_max_idle_time () == 0);
 }
 
 static void
-test_count_threads_foreach (GThread *thread, 
+test_count_threads_foreach (GThread *thread,
 			    guint   *count)
 {
    ++*count;
@@ -91,32 +91,32 @@ static guint
 test_count_threads (void)
 {
   guint count = 0;
-  
+
   g_thread_foreach ((GFunc) test_count_threads_foreach, &count);
-  
+
   /* Exclude main thread */
   return count - 1;
 }
 
 static void
 test_thread_stop_unused (void)
-{ 
+{
    GThreadPool *pool;
    guint i;
    guint limit = 100;
-   
+
    /* Spawn a few threads. */
    g_thread_pool_set_max_unused_threads (-1);
    pool = g_thread_pool_new ((GFunc) g_usleep, NULL, -1, FALSE, NULL);
-   
+
    for (i = 0; i < limit; i++)
      g_thread_pool_push (pool, GUINT_TO_POINTER (1000), NULL);
 
    DEBUG_MSG (("[unused] ===> pushed %d threads onto the idle pool",
 	       limit));
-   
+
    /* Wait for the threads to migrate. */
-   g_usleep (G_USEC_PER_SEC); 
+   g_usleep (G_USEC_PER_SEC);
 
    DEBUG_MSG (("[unused] current threads %d",
 	       test_count_threads()));
@@ -127,15 +127,15 @@ test_thread_stop_unused (void)
    DEBUG_MSG (("[unused] waiting ONE second for threads to die"));
 
    /* Some time for threads to die. */
-   g_usleep (G_USEC_PER_SEC); 
-   
+   g_usleep (G_USEC_PER_SEC);
+
    DEBUG_MSG (("[unused] stopped idle threads, %d remain, %d threads still exist",
-	       g_thread_pool_get_num_unused_threads (), 
+	       g_thread_pool_get_num_unused_threads (),
 	       test_count_threads ()));
-   
+
    g_assert (g_thread_pool_get_num_unused_threads () == test_count_threads ());
    g_assert (g_thread_pool_get_num_unused_threads () == 0);
-   
+
    g_thread_pool_set_max_unused_threads (MAX_THREADS);
 
    DEBUG_MSG (("[unused] cleaning up thread pool"));
@@ -163,9 +163,9 @@ test_thread_pools_entry_func (gpointer data, gpointer user_data)
   leftover_task_counter--;
 
   DEBUG_MSG (("[pool] ---> [%3.3d] exiting thread (abs count:%ld, "
-	      "running count:%ld, left over:%ld)", 
-	      id, abs_thread_counter, 
-	      running_thread_counter, leftover_task_counter)); 
+	      "running count:%ld, left over:%ld)",
+	      id, abs_thread_counter,
+	      running_thread_counter, leftover_task_counter));
   G_UNLOCK (thread_counter_pools);
 }
 
@@ -175,7 +175,7 @@ test_thread_pools (void)
   GThreadPool *pool1, *pool2, *pool3;
   guint runs;
   guint i;
-  
+
   pool1 = g_thread_pool_new ((GFunc)test_thread_pools_entry_func, NULL, 3, FALSE, NULL);
   pool2 = g_thread_pool_new ((GFunc)test_thread_pools_entry_func, NULL, 5, TRUE, NULL);
   pool3 = g_thread_pool_new ((GFunc)test_thread_pools_entry_func, NULL, 7, TRUE, NULL);
@@ -186,15 +186,18 @@ test_thread_pools (void)
       g_thread_pool_push (pool1, GUINT_TO_POINTER (i + 1), NULL);
       g_thread_pool_push (pool2, GUINT_TO_POINTER (i + 1), NULL);
       g_thread_pool_push (pool3, GUINT_TO_POINTER (i + 1), NULL);
+
+      G_LOCK (thread_counter_pools);
       leftover_task_counter += 3;
-    } 
-  
+      G_UNLOCK (thread_counter_pools);
+    }
+
   g_thread_pool_free (pool1, TRUE, TRUE);
   g_thread_pool_free (pool2, FALSE, TRUE);
   g_thread_pool_free (pool3, FALSE, TRUE);
 
   g_assert (runs * 3 == abs_thread_counter + leftover_task_counter);
-  g_assert (running_thread_counter == 0);  
+  g_assert (running_thread_counter == 0);
 }
 
 static gint
@@ -205,7 +208,7 @@ test_thread_sort_compare_func (gconstpointer a, gconstpointer b, gpointer user_d
   id1 = GPOINTER_TO_UINT (a);
   id2 = GPOINTER_TO_UINT (b);
 
-  return (id1 > id2 ? +1 : id1 == id2 ? 0 : -1); 
+  return (id1 > id2 ? +1 : id1 == id2 ? 0 : -1);
 }
 
 static void
@@ -219,8 +222,8 @@ test_thread_sort_entry_func (gpointer data, gpointer user_data)
   thread_id = GPOINTER_TO_UINT (data);
   is_sorted = GPOINTER_TO_INT (user_data);
 
-  DEBUG_MSG (("%s ---> entered thread:%2.2d, last thread:%2.2d", 
-	      is_sorted ? "[  sorted]" : "[unsorted]", 
+  DEBUG_MSG (("%s ---> entered thread:%2.2d, last thread:%2.2d",
+	      is_sorted ? "[  sorted]" : "[unsorted]",
 	      thread_id, last_thread_id));
 
   if (is_sorted) {
@@ -228,13 +231,13 @@ test_thread_sort_entry_func (gpointer data, gpointer user_data)
 
     if (last_thread_id > thread_id) {
       if (last_failed) {
-	g_assert (last_thread_id <= thread_id);  
+	g_assert (last_thread_id <= thread_id);
       }
 
       /* Here we remember one fail and if it concurrently fails, it
        * can not be sorted. the last thread id might be < this thread
        * id if something is added to the queue since threads were
-       * created  
+       * created
        */
       last_failed = TRUE;
     } else {
@@ -267,26 +270,26 @@ test_thread_sort (gboolean sort)
 
   /* It is important that we only have a maximum of 1 thread for this
    * test since the results can not be guranteed to be sorted if > 1.
-   * 
+   *
    * Threads are scheduled by the operating system and are executed at
    * random. It cannot be assumed that threads are executed in the
    * order they are created. This was discussed in bug #334943.
    */
-  
-  pool = g_thread_pool_new (test_thread_sort_entry_func, 
-			    GINT_TO_POINTER (sort), 
-			    max_threads, 
+
+  pool = g_thread_pool_new (test_thread_sort_entry_func,
+			    GINT_TO_POINTER (sort),
+			    max_threads,
 			    FALSE,
 			    NULL);
 
-  g_thread_pool_set_max_unused_threads (MAX_UNUSED_THREADS); 
+  g_thread_pool_set_max_unused_threads (MAX_UNUSED_THREADS);
 
   if (sort) {
-    g_thread_pool_set_sort_function (pool, 
+    g_thread_pool_set_sort_function (pool,
 				     test_thread_sort_compare_func,
 				     GUINT_TO_POINTER (69));
   }
-  
+
   for (i = 0; i < limit; i++) {
     guint id;
 
@@ -294,8 +297,8 @@ test_thread_sort (gboolean sort)
     g_thread_pool_push (pool, GUINT_TO_POINTER (id), NULL);
     DEBUG_MSG (("%s ===> pushed new thread with id:%d, number "
 		"of threads:%d, unprocessed:%d",
-		sort ? "[  sorted]" : "[unsorted]", 
-		id, 
+		sort ? "[  sorted]" : "[unsorted]",
+		id,
 		g_thread_pool_get_num_threads (pool),
 		g_thread_pool_unprocessed (pool)));
   }
@@ -308,7 +311,7 @@ static void
 test_thread_idle_time_entry_func (gpointer data, gpointer user_data)
 {
   guint thread_id;
-  
+
   thread_id = GPOINTER_TO_UINT (data);
 
   DEBUG_MSG (("[idle] ---> entered thread:%2.2d", thread_id));
@@ -318,23 +321,23 @@ test_thread_idle_time_entry_func (gpointer data, gpointer user_data)
   DEBUG_MSG (("[idle] <--- exiting thread:%2.2d", thread_id));
 }
 
-static gboolean 
+static gboolean
 test_thread_idle_timeout (gpointer data)
 {
   guint interval;
   gint i;
 
   interval = GPOINTER_TO_UINT (data);
-  
+
   for (i = 0; i < 2; i++) {
-    g_thread_pool_push (idle_pool, GUINT_TO_POINTER (100 + i), NULL); 
+    g_thread_pool_push (idle_pool, GUINT_TO_POINTER (100 + i), NULL);
     DEBUG_MSG (("[idle] ===> pushed new thread with id:%d, number "
 		"of threads:%d, unprocessed:%d",
-		100 + i, 
+		100 + i,
 		g_thread_pool_get_num_threads (idle_pool),
 		g_thread_pool_unprocessed (idle_pool)));
   }
-  
+
 
   return FALSE;
 }
@@ -346,20 +349,20 @@ test_thread_idle_time ()
   guint interval = 10000;
   gint i;
 
-  idle_pool = g_thread_pool_new (test_thread_idle_time_entry_func, 
-				 NULL, 
+  idle_pool = g_thread_pool_new (test_thread_idle_time_entry_func,
+				 NULL,
 				 MAX_THREADS,
 				 FALSE,
 				 NULL);
 
-  g_thread_pool_set_max_unused_threads (MAX_UNUSED_THREADS);  
-  g_thread_pool_set_max_idle_time (interval); 
+  g_thread_pool_set_max_unused_threads (MAX_UNUSED_THREADS);
+  g_thread_pool_set_max_idle_time (interval);
 
-  g_assert (g_thread_pool_get_max_unused_threads () == MAX_UNUSED_THREADS);   
+  g_assert (g_thread_pool_get_max_unused_threads () == MAX_UNUSED_THREADS);
   g_assert (g_thread_pool_get_max_idle_time () == interval);
 
   for (i = 0; i < limit; i++) {
-    g_thread_pool_push (idle_pool, GUINT_TO_POINTER (i + 1), NULL); 
+    g_thread_pool_push (idle_pool, GUINT_TO_POINTER (i + 1), NULL);
     DEBUG_MSG (("[idle] ===> pushed new thread with id:%d, "
 		"number of threads:%d, unprocessed:%d",
 		i,
@@ -368,7 +371,7 @@ test_thread_idle_time ()
   }
 
   g_timeout_add ((interval - 1000),
-		 test_thread_idle_timeout, 
+		 test_thread_idle_timeout,
 		 GUINT_TO_POINTER (interval));
 }
 
@@ -382,9 +385,9 @@ test_check_start_and_stop (gpointer user_data)
 
   if (test_number == 0) {
     run_next = TRUE;
-    DEBUG_MSG (("***** RUNNING TEST %2.2d *****", test_number)); 
+    DEBUG_MSG (("***** RUNNING TEST %2.2d *****", test_number));
   }
-   
+
   if (run_next) {
     test_number++;
 
@@ -396,22 +399,22 @@ test_check_start_and_stop (gpointer user_data)
       test_thread_stop_unused ();
       break;
     case 3:
-      test_thread_pools ();   
+      test_thread_pools ();
       break;
     case 4:
-      test_thread_sort (FALSE);  
+      test_thread_sort (FALSE);
       break;
     case 5:
-      test_thread_sort (TRUE);  
+      test_thread_sort (TRUE);
       break;
     case 6:
       test_thread_stop_unused ();
       break;
     case 7:
-      test_thread_idle_time ();   
+      test_thread_idle_time ();
       break;
     default:
-      DEBUG_MSG (("***** END OF TESTS *****")); 
+      DEBUG_MSG (("***** END OF TESTS *****"));
       g_main_loop_quit (main_loop);
       continue_timeout = FALSE;
       break;
@@ -422,19 +425,19 @@ test_check_start_and_stop (gpointer user_data)
   }
 
   if (test_number == 3) {
-    G_LOCK (thread_counter_pools); 
+    G_LOCK (thread_counter_pools);
     quit &= running_thread_counter <= 0;
-    DEBUG_MSG (("***** POOL RUNNING THREAD COUNT:%ld", 
-		running_thread_counter)); 
-    G_UNLOCK (thread_counter_pools); 
+    DEBUG_MSG (("***** POOL RUNNING THREAD COUNT:%ld",
+		running_thread_counter));
+    G_UNLOCK (thread_counter_pools);
   }
 
   if (test_number == 4 || test_number == 5) {
     G_LOCK (thread_counter_sort);
     quit &= sort_thread_counter <= 0;
-    DEBUG_MSG (("***** POOL SORT THREAD COUNT:%ld", 
-		sort_thread_counter)); 
-    G_UNLOCK (thread_counter_sort); 
+    DEBUG_MSG (("***** POOL SORT THREAD COUNT:%ld",
+		sort_thread_counter));
+    G_UNLOCK (thread_counter_sort);
   }
 
   if (test_number == 7) {
@@ -444,7 +447,7 @@ test_check_start_and_stop (gpointer user_data)
     quit &= idle < 1;
     DEBUG_MSG (("***** POOL IDLE THREAD COUNT:%d, UNPROCESSED JOBS:%d",
 		idle, g_thread_pool_unprocessed (idle_pool)));
-  }    
+  }
 
   if (quit) {
     run_next = TRUE;
@@ -453,7 +456,7 @@ test_check_start_and_stop (gpointer user_data)
   return continue_timeout;
 }
 
-int 
+int
 main (int argc, char *argv[])
 {
   /* Only run the test, if threads are enabled and a default thread
@@ -463,8 +466,8 @@ main (int argc, char *argv[])
   g_thread_init (NULL);
 
   DEBUG_MSG (("Starting... (in one second)"));
-  g_timeout_add (1000, test_check_start_and_stop, NULL); 
-  
+  g_timeout_add (1000, test_check_start_and_stop, NULL);
+
   main_loop = g_main_loop_new (NULL, FALSE);
   g_main_loop_run (main_loop);
 #endif