future: Fix race with two pausing threads.

3 files changed, 124 insertions, 24 deletions

diff --git a/libdimension/future.c b/libdimension/future.c
index ef3f5e2..8133812 100644
--- a/libdimension/future.c
+++ b/libdimension/future.c
@@ -164,10 +164,9 @@ dmnsn_future_pause(dmnsn_future *future)
     while (future->nrunning < future->nthreads) {
       dmnsn_cond_wait_safely(&future->all_running_cond, &future->mutex);
     }
-    if (future->npaused++ == 0) {
-      while (future->nrunning > 0) {
-        dmnsn_cond_wait_safely(&future->none_running_cond, &future->mutex);
-      }
+    ++future->npaused;
+    while (future->nrunning > 0) {
+      dmnsn_cond_wait_safely(&future->none_running_cond, &future->mutex);
     }
   dmnsn_unlock_mutex(&future->mutex);
 }
@@ -177,8 +176,7 @@ void
 dmnsn_future_resume(dmnsn_future *future)
 {
   dmnsn_lock_mutex(&future->mutex);
-    dmnsn_assert(future->npaused > 0,
-                 "dmnsn_future_resume() without matching dmnsn_future_pause()");
+    dmnsn_assert(future->npaused > 0, "dmnsn_future_resume() without matching dmnsn_future_pause()");
     if (--future->npaused == 0) {
       dmnsn_cond_broadcast(&future->resume_cond);
     }
diff --git a/libdimension/tests/Makefile.am b/libdimension/tests/Makefile.am
index 0349850..0dc9a87 100644
--- a/libdimension/tests/Makefile.am
+++ b/libdimension/tests/Makefile.am
@@ -22,7 +22,10 @@ AM_CXXFLAGS = $(AM_CFLAGS)
 
 check_LTLIBRARIES = libdimension-tests.la                                      \
                     libdimension-unit-test.la
-check_PROGRAMS    = warning.test                                               \
+check_PROGRAMS    = c89.test                                                   \
+                    c99.test                                                   \
+                    cxx.test                                                   \
+                    warning.test                                               \
                     warning-as-error.test                                      \
                     error.test                                                 \
                     custom-error-fn.test                                       \
@@ -30,12 +33,10 @@ check_PROGRAMS    = warning.test                                               \
                     dictionary.test                                            \
                     polynomial.test                                            \
                     prtree.test                                                \
+                    future.test                                                \
                     png.test                                                   \
                     gl.test                                                    \
-                    render.test                                                \
-                    c89.test                                                   \
-                    c99.test                                                   \
-                    cxx.test
+                    render.test
 TESTS             = $(check_PROGRAMS)
 XFAIL_TESTS       = warning-as-error.test error.test
 
@@ -64,6 +65,19 @@ libdimension_unit_test_la_SOURCES = unit-test.c
 libdimension_unit_test_la_CFLAGS  = $(CHECK_CFLAGS) $(AM_CFLAGS)
 libdimension_unit_test_la_LIBADD  = $(CHECK_LIBS) $(top_builddir)/libdimension/libdimension.la
 
+c89_test_SOURCES = c89.c
+c89_test_LDADD   = libdimension-tests.la
+
+c89.o: CFLAGS += -std=c89 -Wpedantic
+
+c99_test_SOURCES = c99.c
+c99_test_LDADD   = libdimension-tests.la
+
+c99.o: CFLAGS += -std=c99 -Wpedantic
+
+cxx_test_SOURCES = cxx.cpp
+cxx_test_LDADD   = libdimension-tests.la
+
 warning_test_SOURCES = warning.c
 warning_test_LDADD   = libdimension-tests.la
 
@@ -88,6 +102,9 @@ polynomial_test_LDADD   = libdimension-unit-test.la
 prtree_test_SOURCES = prtree.c
 prtree_test_LDADD   = libdimension-tests.la
 
+future_test_SOURCES = future.c
+future_test_LDADD   = libdimension-unit-test.la
+
 png_test_SOURCES = png.c
 png_test_LDADD   = libdimension-tests.la
 
@@ -97,18 +114,5 @@ gl_test_LDADD   = libdimension-tests.la
 render_test_SOURCES = render.c
 render_test_LDADD   = libdimension-tests.la
 
-c89_test_SOURCES = c89.c
-c89_test_LDADD   = libdimension-tests.la
-
-c89.o: CFLAGS += -std=c89 -Wpedantic
-
-c99_test_SOURCES = c99.c
-c99_test_LDADD   = libdimension-tests.la
-
-c99.o: CFLAGS += -std=c99 -Wpedantic
-
-cxx_test_SOURCES = cxx.cpp
-cxx_test_LDADD   = libdimension-tests.la
-
 clean-local:
 	rm -f *.png
diff --git a/libdimension/tests/future.c b/libdimension/tests/future.c
new file mode 100644
index 0000000..ee2511a
--- /dev/null
+++ b/libdimension/tests/future.c
@@ -0,0 +1,98 @@
+/*************************************************************************
+ * Copyright (C) 2014 Tavian Barnes <tavianator@tavianator.com>          *
+ *                                                                       *
+ * This file is part of The Dimension Test Suite.                        *
+ *                                                                       *
+ * The Dimension Test Suite is free software; you can redistribute it    *
+ * and/or modify it under the terms of the GNU General Public License as *
+ * published by the Free Software Foundation; either version 3 of the    *
+ * License, or (at your option) any later version.                       *
+ *                                                                       *
+ * The Dimension Test Suite is distributed in the hope that it will be   *
+ * useful, but WITHOUT ANY WARRANTY; without even the implied warranty   *
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU  *
+ * General Public License for more details.                              *
+ *                                                                       *
+ * You should have received a copy of the GNU General Public License     *
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>. *
+ *************************************************************************/
+
+/**
+ * @file
+ * Tests for dmnsn_future.
+ */
+
+#include "../platform.c"
+#include "../future.c"
+#include "../threads.c"
+#include "tests.h"
+#include <stdatomic.h>
+
+static dmnsn_future *future;
+static atomic_int counter = ATOMIC_VAR_INIT(0);
+static const int NTHREADS = 8;
+static const int CHUNK = 100;
+
+static int
+dmnsn_future_test_ccthread(void *ptr, unsigned int thread, unsigned int nthreads)
+{
+  for (int i = 0; i < CHUNK; ++i) {
+    for (int j = 0; j < CHUNK; ++j) {
+      atomic_fetch_add(&counter, 1);
+    }
+    dmnsn_future_increment(future);
+  }
+  return 0;
+}
+
+static int
+dmnsn_future_test_thread(void *ptr)
+{
+  dmnsn_future_set_total(future, NTHREADS*CHUNK);
+  return dmnsn_execute_concurrently(future, dmnsn_future_test_ccthread, ptr, NTHREADS);
+}
+
+DMNSN_TEST_SETUP(future)
+{
+  future = dmnsn_new_future();
+  dmnsn_new_thread(future, dmnsn_future_test_thread, NULL);
+}
+
+DMNSN_TEST_TEARDOWN(future)
+{
+  int res = dmnsn_future_join(future);
+  ck_assert_int_eq(res, 0);
+}
+
+void *
+dmnsn_future_pause_and_check(void *ptr)
+{
+  for (int i = 0; i < CHUNK; ++i) {
+    dmnsn_future_pause(future);
+
+      int value = atomic_load(&counter);
+      ck_assert_int_eq(value%CHUNK, 0);
+
+      for (int j = 0; j < CHUNK; ++j) {
+        int new_value = atomic_load(&counter);
+        ck_assert_int_eq(new_value, value);
+      }
+
+    dmnsn_future_resume(future);
+  }
+
+  return NULL;
+}
+
+DMNSN_TEST(future, pause_safely)
+{
+  // Check dmnsn_future_pause() behaviour from two concurrent threads
+  pthread_t thread;
+  if (pthread_create(&thread, NULL, dmnsn_future_pause_and_check, NULL) != 0) {
+    dmnsn_error("Couldn't start thread.");
+  }
+
+  dmnsn_future_pause_and_check(NULL);
+
+  dmnsn_join_thread(thread, NULL);
+}