diff options
author | Pádraig Brady <P@draigBrady.com> | 2014-07-11 16:11:22 +0100 |
---|---|---|
committer | Pádraig Brady <P@draigBrady.com> | 2014-07-13 21:09:14 +0100 |
commit | fe08796d7c5e7f67d934ce9e8285433651b0f538 (patch) | |
tree | e1f691036302d24c5b0fa0d591d14a84ad043fea /src | |
parent | eabcccc44b452605de1a531650fd4f79e1934be0 (diff) | |
download | coreutils-fe08796d7c5e7f67d934ce9e8285433651b0f538.tar.gz |
sort: avoid undefined operation with destroying locked mutex
This didn't seem to cause any invalid operation on GNU/Linux at least,
but depending on the implementation, mutex deadlocks could occur.
For example this might be the cause of lockups seen on Solaris:
http://lists.gnu.org/archive/html/coreutils/2013-03/msg00048.html
This was identified with valgrind 3.9.0 with this setup:
seq 200000 > file.sort
valgrind --tool=drd src/sort file.sort -o file.sort
With that, valgrind would _intermittently_ report the following:
Destroying locked mutex: mutex 0x5419548, recursion count 1, owner 2.
at 0x4C2E3F0: pthread_mutex_destroy(in vgpreload_drd-amd64-linux.so)
by 0x409FA2: sortlines (sort.c:3649)
by 0x409E26: sortlines (sort.c:3621)
by 0x40AA9E: sort (sort.c:3955)
by 0x40C5D9: main (sort.c:4739)
mutex 0x5419548 was first observed at:
at 0x4C2DE82: pthread_mutex_init(in vgpreload_drd-amd64-linux.so)
by 0x409266: init_node (sort.c:3276)
by 0x4092F4: init_node (sort.c:3286)
by 0x4090DD: merge_tree_init (sort.c:3234)
by 0x40AA5A: sort (sort.c:3951)
by 0x40C5D9: main (sort.c:4739)
Thread 2:
The object at address 0x5419548 is not a mutex.
at 0x4C2F4A4: pthread_mutex_unlock(in vgpreload_drd-amd64-linux.so)
by 0x4093CA: unlock_node (sort.c:3323)
by 0x409C85: merge_loop (sort.c:3531)
by 0x409F8F: sortlines (sort.c:3644)
by 0x409CE3: sortlines_thread (sort.c:3574)
by 0x4E44F32: start_thread (in /usr/lib64/libpthread-2.18.so)
by 0x514EEAC: clone (in /usr/lib64/libc-2.18.so)
* src/sort.c (sortlines): Move pthread_mutex_destroy() out to
merge_tree_destroy(), so that we don't overlap mutex destruction
with threads still operating on the nodes.
(sort): Call the destructors only with "lint" defined, as the
memory used will be deallocated implicitly at process end.
* NEWS: Mention the bug fix.
Diffstat (limited to 'src')
-rw-r--r-- | src/sort.c | 23 |
1 files changed, 15 insertions, 8 deletions
diff --git a/src/sort.c b/src/sort.c index af95f7110..c24931920 100644 --- a/src/sort.c +++ b/src/sort.c @@ -3237,10 +3237,17 @@ merge_tree_init (size_t nthreads, size_t nlines, struct line *dest) /* Destroy the merge tree. */ static void -merge_tree_destroy (struct merge_node *merge_tree) +merge_tree_destroy (size_t nthreads, struct merge_node *merge_tree) { - struct merge_node *root = merge_tree; - pthread_mutex_destroy (&root->lock); + size_t n_nodes = nthreads * 2; + struct merge_node *node = merge_tree; + + while (n_nodes--) + { + pthread_mutex_destroy (&node->lock); + node++; + } + free (merge_tree); } @@ -3642,8 +3649,6 @@ sortlines (struct line *restrict lines, size_t nthreads, queue_insert (queue, node); merge_loop (queue, total_lines, tfp, temp_output); } - - pthread_mutex_destroy (&node->lock); } /* Scan through FILES[NTEMPS .. NFILES-1] looking for files that are @@ -3947,12 +3952,14 @@ sort (char *const *files, size_t nfiles, char const *output_file, queue_init (&queue, nthreads); struct merge_node *merge_tree = merge_tree_init (nthreads, buf.nlines, line); - struct merge_node *root = merge_tree + 1; - sortlines (line, nthreads, buf.nlines, root, + sortlines (line, nthreads, buf.nlines, merge_tree + 1, &queue, tfp, temp_output); + +#ifdef lint + merge_tree_destroy (nthreads, merge_tree); queue_destroy (&queue); - merge_tree_destroy (merge_tree); +#endif } else write_unique (line - 1, tfp, temp_output); |