Another try at correcting the ping pong semaphore test (HDFGroup#4754)

* Add ifdef around prototype * Add casts to make MSVC happy * Fix missing atomic load
derobins · Aug 25, 2024 · c38064b · c38064b
1 parent 7c4b501
commit c38064b
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 12 deletions.
diff --git a/src/H5TSwin.c b/src/H5TSwin.c
@@ -45,10 +45,12 @@
 /********************/
 /* Local Prototypes */
 /********************/
+#ifdef H5_HAVE_THREADSAFE
 #if defined(H5_BUILT_AS_DYNAMIC_LIB) && defined(H5_HAVE_WIN32_API)
 static herr_t H5TS__win32_thread_enter(void);
 static herr_t H5TS__win32_thread_exit(void);
 #endif
+#endif
 
 /*********************/
 /* Package Variables */

diff --git a/test/ttsafe_semaphore.c b/test/ttsafe_semaphore.c
@@ -20,14 +20,14 @@
 
 #if defined(H5_HAVE_THREADS)
 
-#define NUM_PINGPONG     (1000 * 1000)
+#define NUM_PINGPONG     (500 * 1000)
 #define NUM_CLIENTSERVER (50 * 1000)
 
 #define NUM_THREADS 16
 
 typedef struct {
-    H5TS_semaphore_t ping_sem, pong_sem;
-    unsigned         counter;
+    H5TS_semaphore_t   ping_sem, pong_sem;
+    H5TS_atomic_uint_t counter;
 } pingpong_t;
 
 typedef struct {
@@ -40,19 +40,18 @@ static H5TS_THREAD_RETURN_TYPE
 ping(void *_test_info)
 {
     pingpong_t       *test_info = (pingpong_t *)_test_info;
-    unsigned          count;
     herr_t            result;
     H5TS_thread_ret_t ret_value = 0;
 
     do {
         result = H5TS_semaphore_wait(&test_info->ping_sem);
         CHECK_I(result, "H5TS_semaphore_wait");
 
-        count = ++test_info->counter;
+        H5TS_atomic_fetch_add_uint(&test_info->counter, (unsigned)1);
 
         result = H5TS_semaphore_signal(&test_info->pong_sem);
         CHECK_I(result, "H5TS_semaphore_signal");
-    } while (count < NUM_PINGPONG);
+    } while (H5TS_atomic_load_uint(&test_info->counter) < NUM_PINGPONG);
 
     return ret_value;
 }
@@ -61,19 +60,18 @@ static H5TS_THREAD_RETURN_TYPE
 pong(void *_test_info)
 {
     pingpong_t       *test_info = (pingpong_t *)_test_info;
-    unsigned          count;
     herr_t            result;
     H5TS_thread_ret_t ret_value = 0;
 
     do {
         result = H5TS_semaphore_wait(&test_info->pong_sem);
         CHECK_I(result, "H5TS_semaphore_wait");
 
-        count = ++test_info->counter;
+        H5TS_atomic_fetch_add_uint(&test_info->counter, (unsigned)1);
 
         result = H5TS_semaphore_signal(&test_info->ping_sem);
         CHECK_I(result, "H5TS_semaphore_signal");
-    } while (count < NUM_PINGPONG);
+    } while (H5TS_atomic_load_uint(&test_info->counter) < NUM_PINGPONG);
 
     return ret_value;
 }
@@ -95,7 +93,7 @@ tts_semaphore_pingpong(void)
     CHECK_I(result, "H5TS_semaphore_init");
     result = H5TS_semaphore_init(&test_info.pong_sem, 0);
     CHECK_I(result, "H5TS_semaphore_init");
-    test_info.counter = 0;
+    H5TS_atomic_init_uint(&test_info.counter, (unsigned)0);
 
     /* Start ping & pong threads */
     result = H5TS_thread_create(&ping_thread, ping, &test_info);
@@ -113,13 +111,15 @@ tts_semaphore_pingpong(void)
     result = H5TS_thread_join(pong_thread, NULL);
     CHECK_I(result, "H5TS_thread_join");
 
-    VERIFY(test_info.counter, (NUM_PINGPONG + 1), "ping pong");
+    VERIFY(H5TS_atomic_load_uint(&test_info.counter), (NUM_PINGPONG + 1), "ping pong");
 
-    /* Destroy semaphores */
+    /* Destroy semaphores, etc. */
     result = H5TS_semaphore_destroy(&test_info.ping_sem);
     CHECK_I(result, "H5TS_semaphore_destroy");
     result = H5TS_semaphore_destroy(&test_info.pong_sem);
     CHECK_I(result, "H5TS_semaphore_destroy");
+
+    H5TS_atomic_destroy_uint(&test_info.counter);
 } /* end tts_semaphore_pingpong() */
 
 static H5TS_THREAD_RETURN_TYPE