[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#7489: [coreutils] over aggressive threads in sort
From: |
Jim Meyering |
Subject: |
bug#7489: [coreutils] over aggressive threads in sort |
Date: |
Mon, 29 Nov 2010 21:09:30 +0100 |
Paul Eggert wrote:
> Could you please try this little patch? It should fix your
> problem. I came up with this fix in my sleep (literally!
> I woke up this morning and the patch was in my head), but
> haven't had time to look at the code in this area to see
> if it's the best fix.
>
> Clearly there's at least one more bug as noted in my previous email
> <http://lists.gnu.org/archive/html/bug-coreutils/2010-11/msg00216.html>
> but I expect it's less likely to fire.
>
> diff --git a/src/sort.c b/src/sort.c
> index 7e25f6a..1aa1eb4 100644
> --- a/src/sort.c
> +++ b/src/sort.c
> @@ -3226,13 +3226,13 @@ queue_pop (struct merge_node_queue *queue)
> static void
> write_unique (struct line const *line, FILE *tfp, char const *temp_output)
> {
> - static struct line const *saved = NULL;
> + static struct line saved;
>
> if (!unique)
> write_line (line, tfp, temp_output);
> - else if (!saved || compare (line, saved))
> + else if (!saved.text || compare (line, &saved))
> {
> - saved = line;
> + saved = *line;
> write_line (line, tfp, temp_output);
> }
> }
Nice.
That looks right to me.
FYI, what happens is the first fillbuf call gets a block
of lines, and "saved" ends up pointing at one of them.
The second fillbuf's fread races to overwrite a byte or two of the
saved->text pointer that is being dereferenced by the other thread.
The patch below adds a small test case to exercise it.
It demonstrates the failure in all but ~20 trials out of 1000 for me.
So far, I've reproduced this only on multi-core systems, with --parallel=2.
>From 3b8f453a325fbd094e2605347b1d8a05efab9b0d Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sun, 28 Nov 2010 12:59:38 +0100
Subject: [PATCH] tests: add test for parallel sort -u segfault bug
* tests/misc/sort-unique-segv: New file.
* tests/Makefile.am (TESTS): Add it.
---
tests/Makefile.am | 1 +
tests/misc/sort-unique-segv | 55 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 56 insertions(+), 0 deletions(-)
create mode 100755 tests/misc/sort-unique-segv
diff --git a/tests/Makefile.am b/tests/Makefile.am
index b3be4df..d52f677 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -239,6 +239,7 @@ TESTS = \
misc/sort-rand \
misc/sort-spinlock-abuse \
misc/sort-unique \
+ misc/sort-unique-segv \
misc/sort-version \
misc/split-a \
misc/split-bchunk \
diff --git a/tests/misc/sort-unique-segv b/tests/misc/sort-unique-segv
new file mode 100755
index 0000000..0a1d4cb
--- /dev/null
+++ b/tests/misc/sort-unique-segv
@@ -0,0 +1,55 @@
+#!/bin/sh
+# parallel sort with --unique (-u) would segfault
+
+# Copyright (C) 2010 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/init.sh"; path_prepend_ ../src
+print_ver_ sort
+
+test "$(nproc)" = 1 && skip_ "requires a multi-core system"
+
+cat <<\EOF > in || framework_failure_
+
+
+
+
+
+
+
+z
+zzzzzz
+zzzzzzz
+zzzzzzz
+zzzzzzz
+zzzzzzzzz
+zzzzzzzzzzz
+zzzzzzzzzzzz
+EOF
+
+cat <<\EOF > exp || fail=1
+
+z
+zzzzzz
+zzzzzzz
+zzzzzzzzz
+zzzzzzzzzzz
+zzzzzzzzzzzz
+EOF
+
+sort --parallel=2 -u -S 10b < in > out || fail=1
+compare out exp || fail=1
+
+Exit $fail
--
1.7.3.2.846.gf4b062
- bug#7489: [coreutils] over aggressive threads in sort, (continued)
- bug#7489: [coreutils] over aggressive threads in sort, Pádraig Brady, 2010/11/26
- bug#7489: [coreutils] over aggressive threads in sort, Paul Eggert, 2010/11/27
- bug#7489: [coreutils] over aggressive threads in sort, Paul Eggert, 2010/11/27
- bug#7489: [coreutils] over aggressive threads in sort, Paul Eggert, 2010/11/27
- bug#7489: [coreutils] over aggressive threads in sort, Pádraig Brady, 2010/11/27
- bug#7489: [coreutils] over aggressive threads in sort, DJ Lucas, 2010/11/27
- bug#7489: [coreutils] over aggressive threads in sort, DJ Lucas, 2010/11/29
- bug#7489: [coreutils] over aggressive threads in sort, Pádraig Brady, 2010/11/29
- bug#7489: [coreutils] over aggressive threads in sort, Paul Eggert, 2010/11/29
- bug#7489: [coreutils] over aggressive threads in sort,
Jim Meyering <=
- bug#7489: [coreutils] over aggressive threads in sort, Jim Meyering, 2010/11/30
- bug#7489: [coreutils] over aggressive threads in sort, Paul Eggert, 2010/11/30
- bug#7489: [coreutils] over aggressive threads in sort, Chen Guo, 2010/11/30
- bug#7489: [coreutils] over aggressive threads in sort, Jim Meyering, 2010/11/29
- bug#7489: [coreutils] over aggressive threads in sort, Paul Eggert, 2010/11/29