1
0
mirror of git://git.sv.gnu.org/coreutils.git synced 2026-08-05 16:19:23 +02:00

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."
This commit is contained in:
Shayan Pooya
2014-01-25 00:37:12 +00:00
committed by Pádraig Brady
parent dbd7c9452a
commit eabcccc44b
+3 -2
View File
@@ -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