Pádraig Brady <p...@draigbrady.com> 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 <u...@gnu.org> 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/20141011002433.ga15...@tornado.leadboat.com * 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/20141011002433.ga15...@tornado.leadboat.com + */ + 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
