This is the mail archive of the libc-hacker@sourceware.org mailing list for the glibc project.
Note that libc-hacker is a closed list. You may look at the archives of this list, but subscription and posting are not open.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
Hi! As the attached testcase shows (at least on quad core, perhaps not every time but very often), BZ#5058 fixes didn't cure all races. One problem is that nconversions isn't reread after acquiring wrlock, which means that several threads can use a stale cached nconversions value, which means they won't be able to find the conversion added by another thread, and can happily overwrite that entry. The other problem is that when convd->conv_tab is NULL, the setting of it to calloc returned address isn't atomic nor guarded by any lock, so multiple threads can do that. This can mean both memory leak, but also crash when the value is midway through _nl_find_msg one value and then suddenly changes to a different one. 2008-03-30 Jakub Jelinek <jakub@redhat.com> * intl/dcigettext.c (_nl_find_msg): Reread nconversions after acquiring wrlock. Do conv_tab allocation while holding lock. * intl/Makefile: Add rules to build and run tst-gettext6. * intl/tst-gettext6.c: New test. * intl/tst-gettext6.sh: New file. * Makefile (LOCALES): Add ja_JP.UTF-8. --- libc/intl/dcigettext.c 17 Nov 2007 07:37:30 -0000 1.58 +++ libc/intl/dcigettext.c 30 Mar 2008 21:42:59 -0000 @@ -879,6 +879,7 @@ _nl_find_msg (domain_file, domainbinding { /* We have to allocate a new conversions table. */ __libc_rwlock_wrlock (domain->conversions_lock); + nconversions = domain->nconversions; /* Maybe in the meantime somebody added the translation. Recheck. */ @@ -1033,6 +1034,7 @@ _nl_find_msg (domain_file, domainbinding # endif ) { + __libc_lock_define_initialized (static, lock) /* We are supposed to do a conversion. First allocate an appropriate table with the same structure as the table of translations in the file, where we can put the pointers @@ -1042,13 +1044,21 @@ _nl_find_msg (domain_file, domainbinding handle this case by converting RESULTLEN bytes, including NULs. */ - if (convd->conv_tab == NULL - && ((convd->conv_tab = - (char **) calloc (nstrings + domain->n_sysdep_strings, - sizeof (char *))) - == NULL)) - /* Mark that we didn't succeed allocating a table. */ - convd->conv_tab = (char **) -1; + if (__builtin_expect (convd->conv_tab == NULL, 0)) + { + __libc_lock_lock (lock); + if (convd->conv_tab == NULL) + { + convd->conv_tab + = calloc (nstrings + domain->n_sysdep_strings, + sizeof (char *)); + if (convd->conv_tab != NULL) + goto not_translated_yet; + /* Mark that we didn't succeed allocating a table. */ + convd->conv_tab = (char **) -1; + } + __libc_lock_unlock (lock); + } if (__builtin_expect (convd->conv_tab == (char **) -1, 0)) /* Nothing we can do, no more memory. We cannot use the @@ -1057,12 +1067,14 @@ _nl_find_msg (domain_file, domainbinding if (convd->conv_tab[act] == NULL) { + __libc_lock_lock (lock); + not_translated_yet:; + /* We haven't used this string so far, so it is not translated yet. Do this now. */ /* We use a bit more efficient memory handling. We allocate always larger blocks which get used over time. This is faster than many small allocations. */ - __libc_lock_define_initialized (static, lock) # define INITIAL_BLOCK_SIZE 4080 static unsigned char *freemem; static size_t freemem_size; @@ -1074,8 +1086,6 @@ _nl_find_msg (domain_file, domainbinding transmem_block_t *transmem_list = NULL; # endif - __libc_lock_lock (lock); - inbuf = (const unsigned char *) result; outbuf = freemem + sizeof (size_t); --- libc/intl/Makefile 4 May 2005 17:53:42 -0000 1.54 +++ libc/intl/Makefile 30 Mar 2008 21:42:58 -0000 @@ -1,4 +1,4 @@ -# Copyright (C) 1995-2003, 2005 Free Software Foundation, Inc. +# Copyright (C) 1995-2003, 2005, 2008 Free Software Foundation, Inc. # This file is part of the GNU C Library. # The GNU C Library is free software; you can redistribute it and/or @@ -32,11 +32,11 @@ distribute = gmo.h gettextP.h hash-strin tst-codeset.sh tstcodeset.po \ tst-gettext3.sh \ tst-gettext4.sh tst-gettext4-de.po tst-gettext4-fr.po \ - tst-gettext5.sh + tst-gettext5.sh tst-gettext6.sh include ../Makeconfig -multithread-test-srcs := tst-gettext4 tst-gettext5 +multithread-test-srcs := tst-gettext4 tst-gettext5 tst-gettext6 test-srcs := tst-gettext tst-translit tst-gettext2 tst-codeset tst-gettext3 ifeq ($(have-thread-library),yes) test-srcs += $(multithread-test-srcs) @@ -67,7 +67,8 @@ ifneq ($(strip $(MSGFMT)),:) tests: $(objpfx)tst-translit.out $(objpfx)tst-gettext2.out \ $(objpfx)tst-codeset.out $(objpfx)tst-gettext3.out ifeq ($(have-thread-library),yes) -tests: $(objpfx)tst-gettext4.out $(objpfx)tst-gettext5.out +tests: $(objpfx)tst-gettext4.out $(objpfx)tst-gettext5.out \ + $(objpfx)tst-gettext6.out endif ifneq (no,$(PERL)) tests: $(objpfx)mtrace-tst-gettext @@ -90,6 +91,8 @@ $(objpfx)tst-gettext4.out: tst-gettext4. $(SHELL) -e $< $(common-objpfx) '$(run-program-prefix)' $(common-objpfx)intl/ $(objpfx)tst-gettext5.out: tst-gettext5.sh $(objpfx)tst-gettext5 $(SHELL) -e $< $(common-objpfx) '$(run-program-prefix)' $(common-objpfx)intl/ +$(objpfx)tst-gettext6.out: tst-gettext6.sh $(objpfx)tst-gettext6 + $(SHELL) -e $< $(common-objpfx) '$(run-program-prefix)' $(common-objpfx)intl/ endif endif @@ -104,6 +107,7 @@ CFLAGS-tst-codeset.c = -DOBJPFX=\"$(objp CFLAGS-tst-gettext3.c = -DOBJPFX=\"$(objpfx)\" CFLAGS-tst-gettext4.c = -DOBJPFX=\"$(objpfx)\" CFLAGS-tst-gettext5.c = -DOBJPFX=\"$(objpfx)\" +CFLAGS-tst-gettext6.c = -DOBJPFX=\"$(objpfx)\" ifeq ($(have-thread-library),yes) ifeq (yes,$(build-shared)) @@ -122,6 +126,7 @@ $(objpfx)tst-codeset.out: $(objpfx)tst-g $(objpfx)tst-gettext3.out: $(objpfx)tst-gettext.out $(objpfx)tst-gettext4.out: $(objpfx)tst-gettext.out $(objpfx)tst-gettext5.out: $(objpfx)tst-gettext.out +$(objpfx)tst-gettext6.out: $(objpfx)tst-gettext.out CPPFLAGS += -D'LOCALEDIR="$(msgcatdir)"' \ -D'LOCALE_ALIAS_PATH="$(msgcatdir)"' --- libc/intl/tst-gettext6.c 1 Jan 1970 00:00:00 -0000 +++ libc/intl/tst-gettext6.c 30 Mar 2008 21:42:59 -0000 @@ -0,0 +1,86 @@ +/* Test that gettext() in multithreaded applications works correctly. + Copyright (C) 2008 Free Software Foundation, Inc. + This file is part of the GNU C Library. + Contributed by Jakub Jelinek <jakub@redhat.com>, 2008. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library 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 + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, write to the Free + Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA + 02111-1307 USA. */ + +#include <libintl.h> +#include <locale.h> +#include <pthread.h> +#include <stdio.h> +#include <stdlib.h> +#include <sys/wait.h> +#include <unistd.h> + +pthread_barrier_t b; + +static void * +tf (void *arg) +{ + pthread_barrier_wait (&b); + return gettext ("Operation not permitted"); +} + +int +test (void) +{ + pthread_t th[4]; + unsetenv ("LANGUAGE"); + unsetenv ("OUTPUT_CHARSET"); + textdomain ("tstgettext6"); + bindtextdomain ("tstgettext6", OBJPFX "domaindir"); + setlocale (LC_ALL, "ja_JP.UTF-8"); + pthread_barrier_init (&b, NULL, 4); + for (int i = 0; i < 4; i++) + if (pthread_create (&th[i], NULL, tf, NULL)) + { + puts ("pthread_create failed"); + return 1; + } + for (int i = 0; i < 4; i++) + pthread_join (th[i], NULL); + return 0; +} + +int +main (void) +{ + for (int i = 0; i < 300; i++) + { + pid_t p = fork (); + if (p == -1) + { + printf ("fork failed: %m\n"); + return 1; + } + if (p == 0) + _exit (test ()); + int status; + wait (&status); + if (WIFEXITED (status) && WEXITSTATUS (status) != 0) + { + printf ("child exited with %d\n", WEXITSTATUS (status)); + return 1; + } + else if (WIFSIGNALED (status)) + { + printf ("child killed by signal %d\n", WTERMSIG (status)); + return 1; + } + } + return 0; +} --- libc/intl/tst-gettext6.sh 1 Jan 1970 00:00:00 -0000 +++ libc/intl/tst-gettext6.sh 30 Mar 2008 21:42:59 -0000 @@ -0,0 +1,41 @@ +#! /bin/sh +# Test that gettext() in multithreaded applications works correctly. +# Copyright (C) 2008 Free Software Foundation, Inc. +# This file is part of the GNU C Library. +# + +# The GNU C Library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. + +# The GNU C Library 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 +# Lesser General Public License for more details. + +# You should have received a copy of the GNU Lesser General Public +# License along with the GNU C Library; if not, write to the Free +# Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA +# 02111-1307 USA. + +common_objpfx=$1 +run_program_prefix=$2 +objpfx=$3 + +LC_ALL=C +export LC_ALL + +# Create the domain directory. +mkdir -p ${objpfx}domaindir/ja_JP/LC_MESSAGES +# Populate it. +msgfmt -o ${objpfx}domaindir/ja_JP/LC_MESSAGES/tstgettext6.mo ../po/ja.po + +GCONV_PATH=${common_objpfx}iconvdata +export GCONV_PATH +LOCPATH=${common_objpfx}localedata +export LOCPATH + +${run_program_prefix} ${objpfx}tst-gettext6 > ${objpfx}tst-gettext6.out + +exit $? --- libc/localedata/Makefile 2 Oct 2007 17:19:40 -0000 1.107 +++ libc/localedata/Makefile 30 Mar 2008 21:46:25 -0000 @@ -133,7 +133,8 @@ LOCALES := de_DE.ISO-8859-1 de_DE.UTF-8 en_US.ISO-8859-1 ja_JP.EUC-JP da_DK.ISO-8859-1 \ hr_HR.ISO-8859-2 sv_SE.ISO-8859-1 ja_JP.SJIS fr_FR.ISO-8859-1 \ vi_VN.TCVN5712-1 nb_NO.ISO-8859-1 nn_NO.ISO-8859-1 \ - tr_TR.UTF-8 cs_CZ.UTF-8 zh_TW.EUC-TW fa_IR.UTF-8 fr_FR.UTF-8 + tr_TR.UTF-8 cs_CZ.UTF-8 zh_TW.EUC-TW fa_IR.UTF-8 fr_FR.UTF-8 \ + ja_JP.UTF-8 LOCALE_SRCS := $(shell echo "$(LOCALES)"|sed 's/\([^ .]*\)[^ ]*/\1/g') CHARMAPS := $(shell echo "$(LOCALES)" | \ sed -e 's/[^ .]*[.]\([^ ]*\)/\1/g' -e s/SJIS/SHIFT_JIS/g) Jakub
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |