summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorShayan Pooya <shayan@liveve.org>2014-01-25 00:37:12 +0000
committerPádraig Brady <P@draigBrady.com>2014-07-13 21:09:14 +0100
commiteabcccc44b452605de1a531650fd4f79e1934be0 (patch)
tree8e85ef6a395a3c3ec70c63d3392aa6805ff6c416 /src
parentdbd7c9452a121f948b4eabbe22e07ad13900bc9b (diff)
downloadcoreutils-eabcccc44b452605de1a531650fd4f79e1934be0.tar.xz
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."
Diffstat (limited to 'src')
-rw-r--r--src/sort.c5
1 files changed, 3 insertions, 2 deletions
diff --git a/src/sort.c b/src/sort.c
index 49caae525..af95f7110 100644
--- 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