[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [bug-gettext] [bug #43404] gl_locale_name_default() thread issues on
From: |
Daiki Ueno |
Subject: |
Re: [bug-gettext] [bug #43404] gl_locale_name_default() thread issues on OS X |
Date: |
Tue, 14 Oct 2014 14:53:25 +0900 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.4.50 (gnu/linux) |
Pádraig Brady <address@hidden> writes:
> There are many consequences to having multiple threads,
> so it's worth avoiding/isolating that where possible.
>
> You mentioned it would be conditionalized on pthread_is_threaded_np
> for performance I suppose, as threaded progs would already
> have to deal with the consequences of a separate thread.
> So races in pthread_is_threaded_np would only be a small
> performance issue.
>
> Seems like a good idea to me.
Thanks for the comment. Here is a tentative patch based on the
PostgreSQL patches. Perhaps it might require a copyright assignment
from the original author (Cc'ed).
>From d8e9c96ef0c1e1c1c410cfbaec547c2e3d442dbe Mon Sep 17 00:00:00 2001
From: Daiki Ueno <address@hidden>
Date: Tue, 14 Oct 2014 12:47:06 +0900
Subject: [PATCH] localename: Avoid implicit thread creation in CoreFoundation
Problem reported by Peter Eisentraut at:
http://savannah.gnu.org/bugs/?43404.
Since some CoreFoundation functions (CFLocaleCopyCurrent for instance)
create a new thread internally, it would be better to move the calls
into a subprocess, when the caller is single-threaded.
Based on the patch created by Noah Misch:
http://www.postgresql.org/message-id/address@hidden
* tests/test-localename.c (test_locale_name_default) [__APPLE__]:
Check the return value of pthread_is_threaded_np before and after the
gl_locale_name_default.
* m4/localename.m4 (gl_LOCALENAME): Check pthread_is_threaded_np.
* lib/localename.c [__APPLE__]: Include <pthread.h>, <signal.h>, and
<unistd.h> if pthread_is_threaded_np is available.
(gl_locale_name_default_from_CoreFoundation) [__APPLE__]: New
function, split off from gl_locale_name_default.
(gl_locale_name_default_from_CoreFoundation_forked) [__APPLE__]: New
function.
(gl_locale_name_default) [__APPLE__]: Call _from_CoreFoundation or
_from_CoreFoundation_forked depending on whether or not the caller is
multi-threaded.
---
lib/localename.c | 161 ++++++++++++++++++++++++++++++++++++++++--------
m4/localename.m4 | 2 +-
tests/test-localename.c | 16 ++++-
3 files changed, 151 insertions(+), 28 deletions(-)
diff --git a/lib/localename.c b/lib/localename.c
index 78dc344..745bc92 100644
--- a/lib/localename.c
+++ b/lib/localename.c
@@ -51,6 +51,12 @@
# elif HAVE_CFPREFERENCESCOPYAPPVALUE
# include <CoreFoundation/CFPreferences.h>
# endif
+# if HAVE_PTHREAD_IS_THREADED_NP
+# define _DARWIN_C_SOURCE 1
+# include <pthread.h>
+# include <signal.h>
+# include <unistd.h>
+# endif
#endif
#if defined _WIN32 || defined __WIN32__
@@ -2836,6 +2842,122 @@ gl_locale_name_environ (int category, const char
*categoryname)
return NULL;
}
+#if HAVE_CFLOCALECOPYCURRENT || HAVE_CFPREFERENCESCOPYAPPVALUE
+static char *
+gl_locale_name_default_from_CoreFoundation (void)
+{
+ char namebuf[256];
+ char *localename = "C";
+# if HAVE_CFLOCALECOPYCURRENT /* Mac OS X 10.3 or newer */
+ CFLocaleRef locale = CFLocaleCopyCurrent ();
+ CFStringRef name = CFLocaleGetIdentifier (locale);
+
+ if (CFStringGetCString (name, namebuf, sizeof (namebuf),
+ kCFStringEncodingASCII))
+ {
+ gl_locale_name_canonicalize (namebuf);
+ localename = namebuf;
+ }
+ CFRelease (locale);
+# elif HAVE_CFPREFERENCESCOPYAPPVALUE /* Mac OS X 10.2 or newer */
+ CFTypeRef value =
+ CFPreferencesCopyAppValue (CFSTR ("AppleLocale"),
+ kCFPreferencesCurrentApplication);
+ if (value != NULL
+ && CFGetTypeID (value) == CFStringGetTypeID ()
+ && CFStringGetCString ((CFStringRef)value,
+ namebuf, sizeof (namebuf),
+ kCFStringEncodingASCII))
+ {
+ gl_locale_name_canonicalize (namebuf);
+ localename = namebuf;
+ }
+# endif
+ return strdup (localename);
+}
+
+# if HAVE_PTHREAD_IS_THREADED_NP
+static char *
+gl_locale_name_default_from_CoreFoundation_forked (void)
+{
+ char namebuf[256];
+ char *localename = "C";
+ int fd[2];
+ sigset_t sigs, old_sigs;
+ pid_t pid;
+ int child_status;
+ ssize_t nread;
+
+ /* Block SIGCHLD so we can get an exit status. */
+ sigemptyset (&sigs);
+ sigaddset (&sigs, SIGCHLD);
+ sigprocmask (SIG_BLOCK, &sigs, &old_sigs);
+
+ if (pipe (fd) < 0)
+ goto done;
+
+ pid = fork ();
+ if (pid < 0)
+ {
+ close (fd[0]);
+ close (fd[1]);
+ goto done;
+ }
+
+ if (pid == 0)
+ {
+ char *locname = gl_locale_name_default_from_CoreFoundation ();
+ size_t locname_len = strlen (locname);
+ int status = EXIT_SUCCESS;
+
+ if (close (fd[0]) < 0)
+ status = EXIT_FAILURE;
+
+ if (write (fd[1], locname, locname_len) < locname_len)
+ status = EXIT_FAILURE;
+
+ if (close (fd[1]) < 0)
+ status = EXIT_FAILURE;
+
+ free (locname);
+ _exit (status);
+ }
+
+ if (close (fd[1]) < 0)
+ {
+ close (fd[0]);
+ goto done;
+ }
+
+ if (waitpid (pid, &child_status, 0) != pid
+ || !(WIFEXITED (child_status)
+ && WEXITSTATUS (child_status) == EXIT_SUCCESS))
+ {
+ close (fd[0]);
+ close (fd[1]);
+ goto done;
+ }
+
+ nread = read (fd[0], namebuf, sizeof (namebuf));
+ if (nread <= 0 || nread == sizeof (namebuf))
+ {
+ close (fd[0]);
+ goto done;
+ }
+
+ if (close (fd[0]) < 0)
+ goto done;
+
+ namebuf[nread] = '\0';
+ localename = namebuf;
+
+ done:
+ sigprocmask (SIG_SETMASK, &old_sigs, NULL);
+ return strdup (localename);
+}
+# endif
+#endif
+
const char *
gl_locale_name_default (void)
{
@@ -2888,32 +3010,19 @@ gl_locale_name_default (void)
if (cached_localename == NULL)
{
- char namebuf[256];
-# if HAVE_CFLOCALECOPYCURRENT /* Mac OS X 10.3 or newer */
- CFLocaleRef locale = CFLocaleCopyCurrent ();
- CFStringRef name = CFLocaleGetIdentifier (locale);
-
- if (CFStringGetCString (name, namebuf, sizeof (namebuf),
- kCFStringEncodingASCII))
- {
- gl_locale_name_canonicalize (namebuf);
- cached_localename = strdup (namebuf);
- }
- CFRelease (locale);
-# elif HAVE_CFPREFERENCESCOPYAPPVALUE /* Mac OS X 10.2 or newer */
- CFTypeRef value =
- CFPreferencesCopyAppValue (CFSTR ("AppleLocale"),
- kCFPreferencesCurrentApplication);
- if (value != NULL
- && CFGetTypeID (value) == CFStringGetTypeID ()
- && CFStringGetCString ((CFStringRef)value,
- namebuf, sizeof (namebuf),
- kCFStringEncodingASCII))
- {
- gl_locale_name_canonicalize (namebuf);
- cached_localename = strdup (namebuf);
- }
-# endif
+ /* If the caller is a single-threaded process, isolate the
+ CoreFoundation calls, which may create a new thread, to a
+ separate process.
+ Based on the approach proposed by Noah Misch in:
+ http://www.postgresql.org/message-id/address@hidden
+ */
+ if (!pthread_is_threaded_np ())
+ cached_localename
+ = gl_locale_name_default_from_CoreFoundation_forked ();
+ else
+ cached_localename
+ = gl_locale_name_default_from_CoreFoundation ();
+
if (cached_localename == NULL)
cached_localename = "C";
}
diff --git a/m4/localename.m4 b/m4/localename.m4
index d865c66..499a59e 100644
--- a/m4/localename.m4
+++ b/m4/localename.m4
@@ -8,5 +8,5 @@ AC_DEFUN([gl_LOCALENAME],
[
AC_REQUIRE([gt_LC_MESSAGES])
AC_REQUIRE([gt_INTL_MACOSX])
- AC_CHECK_FUNCS([setlocale uselocale])
+ AC_CHECK_FUNCS([setlocale uselocale pthread_is_threaded_np])
])
diff --git a/tests/test-localename.c b/tests/test-localename.c
index df6c1d6..1862f02 100644
--- a/tests/test-localename.c
+++ b/tests/test-localename.c
@@ -711,10 +711,24 @@ test_locale_name_environ (void)
static void
test_locale_name_default (void)
{
- const char *name = gl_locale_name_default ();
+ const char *name;
+ int is_threaded;
+
+ /* Check if the CoreFoundation calls are properly isolated and
+ don't create additional threads. */
+#if (HAVE_CFLOCALECOPYCURRENT || HAVE_CFPREFERENCESCOPYAPPVALUE) \
+ && HAVE_PTHREAD_IS_THREADED_NP
+ is_threaded = pthread_is_threaded_np ();
+#endif
+ name = gl_locale_name_default ();
ASSERT (name != NULL);
+#if (HAVE_CFLOCALECOPYCURRENT || HAVE_CFPREFERENCESCOPYAPPVALUE) \
+ && HAVE_PTHREAD_IS_THREADED_NP
+ ASSERT (pthread_is_threaded_np () == is_threaded);
+#endif
+
/* Only Mac OS X and Windows have a facility for the user to set the default
locale. */
#if !((defined __APPLE__ && defined __MACH__) || (defined _WIN32 || defined
__WIN32__ || defined __CYGWIN__))
--
1.9.3
Regards,
--
Daiki Ueno