From 0bb6bfe0b08eb9fc2ba62f64bdb21cb5b592c292 Mon Sep 17 00:00:00 2001
From: Tavian Barnes <tavianator@gmail.com>
Date: Mon, 26 Oct 2009 00:54:15 +0000
Subject: Fix some concurrency mistakes.

TODO: eliminate the too_late field on canvases.
---
 libdimension/progress.c | 38 +++++++++++++++++++++++++++-----------
 libdimension/raytrace.c |  5 ++++-
 2 files changed, 31 insertions(+), 12 deletions(-)

(limited to 'libdimension')

diff --git a/libdimension/progress.c b/libdimension/progress.c
index 301f043..bfeeff7 100644
--- a/libdimension/progress.c
+++ b/libdimension/progress.c
@@ -178,7 +178,9 @@ void
 dmnsn_wait_progress(const dmnsn_progress *progress, double prog)
 {
   if (pthread_mutex_lock(progress->mutex) != 0) {
-    dmnsn_error(DMNSN_SEVERITY_LOW, "Couldn't lock condition mutex.");
+    dmnsn_error(DMNSN_SEVERITY_MEDIUM, "Couldn't lock condition mutex.");
+    /* Busy-wait if we can't use the condition variable */
+    while (dmnsn_get_progress(progress) < prog);
   } else {
     while (dmnsn_get_progress(progress) < prog) {
       if (pthread_cond_wait(progress->cond, progress->mutex) != 0) {
@@ -188,7 +190,7 @@ dmnsn_wait_progress(const dmnsn_progress *progress, double prog)
     }
 
     if (pthread_mutex_unlock(progress->mutex) != 0) {
-      dmnsn_error(DMNSN_SEVERITY_LOW, "Couldn't unlock condition mutex.");
+      dmnsn_error(DMNSN_SEVERITY_MEDIUM, "Couldn't unlock condition mutex.");
     }
   }
 }
@@ -198,7 +200,9 @@ void
 dmnsn_new_progress_element(dmnsn_progress *progress, unsigned int total)
 {
   dmnsn_progress_element element = { .progress = 0, .total = total };
-  dmnsn_array_push(progress->elements, &element);
+  dmnsn_progress_wrlock(progress);
+    dmnsn_array_push(progress->elements, &element);
+  dmnsn_progress_unlock(progress);
 }
 
 /* Only the innermost loop needs to call this function - it handles the rest
@@ -221,11 +225,17 @@ dmnsn_increment_progress(dmnsn_progress *progress)
       element = dmnsn_array_at(progress->elements, size - 1);
       ++element->progress; /* Increment the next element */
     }
-
-    if (pthread_cond_broadcast(progress->cond) != 0) {
-      dmnsn_error(DMNSN_SEVERITY_MEDIUM, "Couldn't signal condition variable.");
-    }
   dmnsn_progress_unlock(progress);
+
+  if (pthread_mutex_lock(progress->mutex) != 0) {
+    dmnsn_error(DMNSN_SEVERITY_MEDIUM, "Couldn't lock condition mutex.");
+  }
+  if (pthread_cond_broadcast(progress->cond) != 0) {
+    dmnsn_error(DMNSN_SEVERITY_MEDIUM, "Couldn't signal condition variable.");
+  }
+  if (pthread_mutex_unlock(progress->mutex) != 0) {
+    dmnsn_error(DMNSN_SEVERITY_MEDIUM, "Couldn't unlock condition mutex.");
+  }
 }
 
 /* Immediately set to 100% completion */
@@ -238,11 +248,17 @@ dmnsn_done_progress(dmnsn_progress *progress)
     dmnsn_array_resize(progress->elements, 1);
     element = dmnsn_array_at(progress->elements, 0);
     element->progress = element->total;
-
-    if (pthread_cond_broadcast(progress->cond) != 0) {
-      dmnsn_error(DMNSN_SEVERITY_MEDIUM, "Couldn't signal condition variable.");
-    }
   dmnsn_progress_unlock(progress);
+
+  if (pthread_mutex_lock(progress->mutex) != 0) {
+    dmnsn_error(DMNSN_SEVERITY_MEDIUM, "Couldn't lock condition mutex.");
+  }
+  if (pthread_cond_broadcast(progress->cond) != 0) {
+    dmnsn_error(DMNSN_SEVERITY_MEDIUM, "Couldn't signal condition variable.");
+  }
+  if (pthread_mutex_unlock(progress->mutex) != 0) {
+    dmnsn_error(DMNSN_SEVERITY_MEDIUM, "Couldn't unlock condition mutex.");
+  }
 }
 
 /* Thread synchronization */
diff --git a/libdimension/raytrace.c b/libdimension/raytrace.c
index bb88940..6cb6d8b 100644
--- a/libdimension/raytrace.c
+++ b/libdimension/raytrace.c
@@ -133,7 +133,7 @@ dmnsn_raytrace_scene_multithread(dmnsn_raytrace_payload *payload)
   dmnsn_new_progress_element(payload->progress,
                              nthreads*payload->scene->canvas->y);
 
-  /* Create the threads */
+  /* Create the payloads */
   for (i = 0; i < nthreads; ++i) {
     payloads[i] = *payload;
     payloads[i].index = i;
@@ -142,7 +142,10 @@ dmnsn_raytrace_scene_multithread(dmnsn_raytrace_payload *payload)
       payloads[i].kD_splay_tree =
         dmnsn_kD_splay_copy(payloads[0].kD_splay_tree);
     }
+  }
 
+  /* Create the threads */
+  for (i = 0; i < nthreads; ++i) {
     if (pthread_create(&threads[i], NULL,
                        &dmnsn_raytrace_scene_multithread_thread,
                        &payloads[i]) != 0)
-- 
cgit v1.2.3