This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
locale.c: Don't try to recreate the LC_ALL C locale
authorKarl Williamson <khw@cpan.org>
Thu, 21 Mar 2019 04:59:39 +0000 (22:59 -0600)
committerKarl Williamson <khw@cpan.org>
Thu, 21 Mar 2019 16:50:01 +0000 (10:50 -0600)
On threaded perls, we create a locale object for LC_ALL "C" early in the
startup phase.  When the user asks for that locale, we can just switch
to it instead of trying to create a new one.

Doing the creation worked, but ended up with a memory leak.  My guess,
and its only a guess, is that it's a bug in glibc newlocale.c, in which
it does an early return, not doing proper cleanup, when it discovers it
can re-use an existing locale without needing to create a new one.

The reason I think its a glibc bug is that the sample one-liner sent
to me

PERL_DESTRUCT_LEVEL=2 valgrind --leak-check=full ./perl -DLv -Ilib -e'require POSIX;POSIX::setlocale(&POSIX::LC_ALL, "C");' 2>&1 | more

produced a stack output of where the leaked memory had been allocated.
I put a print immediately after that line, and prints at the points
where things get freed.  Every allocation was matched by an attempt to
free it.  But clearly at least one failed.  freelocale() returns void,
so can't be checked for failing.

Anyway, it's better to try not to create a new locale when we already
have an existing one, and doing so, as this commit does, causes the leak
to go away.

No tests are added, as there are plenty of similar tests already in the
suite, and they all should have been leaking.

locale.c

index 71cf1a3..508ebb4 100644 (file)
--- a/locale.c
+++ b/locale.c
@@ -1007,6 +1007,28 @@ S_emulate_setlocale(const int category,
 
 #  endif
 
+    /* If we are switching to the LC_ALL C locale, it already exists.  Use
+     * it instead of trying to create a new locale */
+    if (mask == LC_ALL_MASK && isNAME_C_OR_POSIX(locale)) {
+
+#  ifdef DEBUGGING
+
+        if (DEBUG_Lv_TEST || debug_initialization) {
+            PerlIO_printf(Perl_debug_log,
+                          "%s:%d: will stay in C object\n", __FILE__, __LINE__);
+        }
+
+#  endif
+
+        new_obj = PL_C_locale_obj;
+
+        /* We already had switched to the C locale in preparation for freeing
+         * old_obj */
+        if (old_obj != LC_GLOBAL_LOCALE && old_obj != PL_C_locale_obj) {
+            freelocale(old_obj);
+        }
+    }
+    else {
     /* If we weren't in a thread safe locale, set so that newlocale() below
      which uses 'old_obj', uses an empty one.  Same for our reserved C object.
      The latter is defensive coding, so that, even if there is some bug, we
@@ -1084,6 +1106,7 @@ S_emulate_setlocale(const int category,
         RESTORE_ERRNO;
         return NULL;
     }
+    }
 
 #  ifdef DEBUGGING