sort: fix two threading issues reported by valgrind

Neither issue impacts on the correct operation of sort.
The issues were detected by both valgrind 3.8.1 and 3.9.0 using:

  seq 200000 > file.sort
  valgrind --tool=drd src/sort file.sort -o file.sort

For tool usage and error details see:
 http://valgrind.org/docs/manual/drd-manual.html

* src/sort.c (queue_insert): Unlock mutex _after_ signalling the
associated condition variable.  Valgrind flags this with:
  "Probably a race condition: condition variable 0xffeffffb0 has been
   signaled but the associated mutex 0xffeffff88 is not locked by the
   signalling thread."
The explanation at the above URL is:
  "Sending a signal to a condition variable while no lock is held on
   the mutex associated with the condition variable.  This is a common
   programming error which can cause subtle race conditions and
   unpredictable behavior."
This should at least give more defined scheduling behavior.

(merge_tree_destroy): Make symmetrical with merge_tree_init() thus
destroying the correct mutex.  Valgrind flags this with:
  "The object at address 0x5476cf8 is not a mutex."

diff --git a/src/sort.c b/src/sort.c
index 49caae525702ce6a3ca22d800c2fd396070889e2..af95f7110d2a5d5eebc4ca7cddabe24ad0064d66 100644 (file)
--- a/src/sort.c
+++ b/src/sort.c
@@ -3239,6 +3239,8 @@ merge_tree_init (size_t nthreads, size_t nlines, struct line *dest)
 static void
 merge_tree_destroy (struct merge_node *merge_tree)
 {
+  struct merge_node *root = merge_tree;
+  pthread_mutex_destroy (&root->lock);
   free (merge_tree);
 }
 
@@ -3354,8 +3356,8 @@ queue_insert (struct merge_node_queue *queue, struct merge_node *node)
   pthread_mutex_lock (&queue->mutex);
   heap_insert (queue->priority_queue, node);
   node->queued = true;
-  pthread_mutex_unlock (&queue->mutex);
   pthread_cond_signal (&queue->cond);
+  pthread_mutex_unlock (&queue->mutex);
 }
 
 /* Pop the top node off the priority QUEUE, lock the node, return it.  */
@@ -3950,7 +3952,6 @@ sort (char *const *files, size_t nfiles, char const *output_file,
               sortlines (line, nthreads, buf.nlines, root,
                          &queue, tfp, temp_output);
               queue_destroy (&queue);
-              pthread_mutex_destroy (&root->lock);
               merge_tree_destroy (merge_tree);
             }
           else