From eabcccc44b452605de1a531650fd4f79e1934be0 Mon Sep 17 00:00:00 2001 From: Shayan Pooya Date: Sat, 25 Jan 2014 00:37:12 +0000 Subject: 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." --- src/sort.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'src/sort.c') 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 -- cgit v1.2.3-54-g00ecf