Hello, A while ago Matthias Clasen pointed me to a bug that is caused by a race condition between a getenv() call in gettext() and a setenv() call in another thread: https://bugzilla.gnome.org/show_bug.cgi?id=754951
The direct cause of this bug is that gettext() tries to check LANGUAGE envvar, while the string content returned by getenv() can be overwritten by setenv() before being used. To mitigate this, I was wondering if it would be possible to move the getenv() call from gettext() to setlocale(), which is typically called at program startup, and cache the value in a static variable. The attached patch is an experiment implementing that in libintl, though in practice it would require a change in glibc's setlocale implementation. Comments appreciated. Regards, -- Daiki Ueno
>From df259036a888e89d3fae7f4b4781b015776852db Mon Sep 17 00:00:00 2001 From: Daiki Ueno <u...@gnu.org> Date: Tue, 10 May 2016 17:53:42 +0900 Subject: [PATCH] intl: Check LANGUAGE envvar in setlocale Problem reported by Matthias Clasen in: https://bugzilla.gnome.org/show_bug.cgi?id=754951 * gettext-runtime/intl/dcigettext.c (_nl_default_overriding_languages): New static variable. (_nl_overriding_languages): New variable to cache the value of LANGUAGE envvar. (_nl_locale_name_override): New function, split from guess_category_value. Set _nl_overriding_languages once LANGUAGE envvar is successfully retrieved. (guess_category_value): Call _nl_locale_name_override instead of directly calling getenv. * gettext-runtime/intl/gettextP.h (_nl_locale_name_override): New function declaration. (_nl_overriding_languages): New variable declaration. * gettext-runtime/intl/setlocale.c (libintl_setlocale) (libintl_newlocale): Call _nl_locale_name_override to ensure _nl_overriding_languages. --- gettext-runtime/intl/dcigettext.c | 33 +++++++++++++++++++++++++++++++-- gettext-runtime/intl/gettextP.h | 12 ++++++++++++ gettext-runtime/intl/setlocale.c | 12 ++++++++++-- 3 files changed, 53 insertions(+), 4 deletions(-) diff --git a/gettext-runtime/intl/dcigettext.c b/gettext-runtime/intl/dcigettext.c index 83bd775..318c27b 100644 --- a/gettext-runtime/intl/dcigettext.c +++ b/gettext-runtime/intl/dcigettext.c @@ -342,6 +342,12 @@ libc_hidden_data_def (_nl_default_dirname) struct binding *_nl_domain_bindings; #endif +/* Default value of the 'LANGUAGE' environment variable used by + gettext(3) prior any call to setlocale(3) or gettext(3). The + default value for this is "". */ +static const char _nl_default_overriding_languages[] = ""; +const char *_nl_overriding_languages = _nl_default_overriding_languages; + /* Prototypes for local functions. */ static char *plural_lookup (struct loaded_l10nfile *domain, unsigned long int n, @@ -1510,6 +1516,28 @@ category_to_name (int category) } #endif +const char * +internal_function +_nl_locale_name_override (void) +{ + const char *language; + + /* The value is stored in _nl_overriding_languages to avoid race + when setenv and gettext are called in separate threads. */ + if (_nl_overriding_languages != _nl_default_overriding_languages) + return _nl_overriding_languages; + + language = getenv ("LANGUAGE"); + + if (language != NULL && language[0] != '\0') + _nl_overriding_languages = strdup (language); + else + _nl_overriding_languages = NULL; + language = _nl_overriding_languages; + + return language; +} + /* Guess value of current locale from value of the environment variables or system-dependent defaults. */ static const char * @@ -1584,9 +1612,10 @@ guess_category_value (int category, const char *categoryname) /* The highest priority value is the value of the 'LANGUAGE' environment variable. */ - language = getenv ("LANGUAGE"); - if (language != NULL && language[0] != '\0') + language = _nl_locale_name_override (); + if (language != NULL) return language; + #if !defined IN_LIBGLOCALE && !defined _LIBC /* The next priority value is the locale name, if not defaulted. */ if (locale_defaulted) diff --git a/gettext-runtime/intl/gettextP.h b/gettext-runtime/intl/gettextP.h index 52be2fc..59c6cf1 100644 --- a/gettext-runtime/intl/gettextP.h +++ b/gettext-runtime/intl/gettextP.h @@ -254,6 +254,7 @@ extern const char *_nl_locale_name_default (void); # define gl_locale_name _nl_locale_name /* extern const char *_nl_locale_name (int category, const char *categoryname); */ +extern const char *_nl_locale_name_override (void); #endif struct loaded_l10nfile *_nl_find_domain (const char *__dirname, char *__locale, @@ -308,6 +309,17 @@ extern const char _nl_default_default_domain[] attribute_hidden; /* Default text domain in which entries for gettext(3) are to be found. */ extern const char *_nl_current_default_domain attribute_hidden; +/* The internal variables in the standalone libintl.a must have different + names than the internal variables in GNU libc, otherwise programs + using libintl.a cannot be linked statically. */ +#if !defined _LIBC +# define _nl_overriding_languages libintl_nl_overriding_languages +#endif + +/* Default list of languages which override locale environment + variables. */ +extern const char *_nl_overriding_languages attribute_hidden; + /* @@ begin of epilog @@ */ #endif /* gettextP.h */ diff --git a/gettext-runtime/intl/setlocale.c b/gettext-runtime/intl/setlocale.c index 07c2305..3e0fea8 100644 --- a/gettext-runtime/intl/setlocale.c +++ b/gettext-runtime/intl/setlocale.c @@ -884,6 +884,7 @@ libintl_setlocale (int category, const char *locale) } /* All steps were successful. */ + (void) _nl_locale_name_override (); ++_nl_msg_cat_cntr; free (saved_locale); return setlocale (LC_ALL, NULL); @@ -904,7 +905,10 @@ libintl_setlocale (int category, const char *locale) result = setlocale_single (category, name); if (result != NULL) - ++_nl_msg_cat_cntr; + { + (void) _nl_locale_name_override (); + ++_nl_msg_cat_cntr; + } return result; } } @@ -941,6 +945,7 @@ libintl_setlocale (int category, const char *locale) } /* It was really successful. */ + (void) _nl_locale_name_override (); ++_nl_msg_cat_cntr; free (saved_locale); return setlocale (LC_ALL, NULL); @@ -950,7 +955,10 @@ libintl_setlocale (int category, const char *locale) { char *result = setlocale_single (category, locale); if (result != NULL) - ++_nl_msg_cat_cntr; + { + (void) _nl_locale_name_override (); + ++_nl_msg_cat_cntr; + } return result; } } -- 2.5.5
