diff options
author | Shayan Pooya <shayan@liveve.org> | 2014-01-25 00:37:12 +0000 |
---|---|---|
committer | Pádraig Brady <P@draigBrady.com> | 2014-07-13 21:09:14 +0100 |
commit | eabcccc44b452605de1a531650fd4f79e1934be0 (patch) | |
tree | 8e85ef6a395a3c3ec70c63d3392aa6805ff6c416 /src | |
parent | dbd7c9452a121f948b4eabbe22e07ad13900bc9b (diff) | |
download | coreutils-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.c | 5 |
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 |