Hello! Running the binary compiled from the simple code #include <locale.h> int main() { while (1) { locale_t n = newlocale(LC_TIME, "en_AU.UTF-8", (locale_t)0); locale_t old = uselocale(n); uselocale(old); freelocale(n); } } results to rapid memory leak. It was tested on 12.0-RELEASE/amd64, 11.2-RELEASE/amd64 and 9.3-RELEASE-p53/i386. First time the problem with memory leak was found with Perl 5.28 and Perl 5.30 on 12.0-RELEASE/amd64. The firt suspicion was about Perl, but further discussion came to this test C code. Discussion on rt.perl.org: https://rt.perl.org/Public/Bug/Display.html?id=134305
Looks like the following atomic operation is not working as expected when manipulating the reference count. /usr/src/lib/libc/locale/xlocale_private.h:183 177 __attribute__((unused)) static void 178 xlocale_release(void *val) 179 { 180 struct xlocale_refcounted *obj = val; 181 long count; 182 183 count = atomic_fetchadd_long(&(obj->retain_count), -1) - 1; 184 if (count < 0 && obj->destructor != NULL) 185 obj->destructor(obj); 186 }
1. atomic_fetchadd_long returns previous value, so I think /usr/src/lib/libc/locale/xlocale_private.h:184 should check for count == 0? 2. looks like atomic_fetchadd_long is expecting u_long which is unsigned, would passing -1 in /usr/src/lib/libc/locale/xlocale_private.h:183 works as we expected? __________________________________________________ sys/amd64/include/atomic.h 241 /* 242 * Atomically add the value of v to the long integer pointed to by p and return 243 * the previous value of *p. 244 */ 245 static __inline u_long 246 atomic_fetchadd_long(volatile u_long *p, u_long v) 247 { 248 249 __asm __volatile( 250 " " MPLOCKED " " 251 " xaddq %0,%1 ; " 252 "# atomic_fetchadd_long" 253 : "+r" (v), /* 0 */ 254 "+m" (*p) /* 1 */ 255 : : "cc"); 256 return (v); 257 }
Ok, a quick check shows atomic_fetchadd_long(_, -1) [2] should works properly, both linuxkpi and nginx do it like that.
(In reply to Baggio Kwok from comment #3) Are there plans to address this issue? It is still causing memory leaks downstream. Thank you very much. Jim Keenan
I think the refcount bug is real, but there is another leak of locale file mappings, apparently not caught by r356569.
In some cases we call xlocale_release() on a structure with a refcount of 0, so really the test should be count <= 0.
(In reply to Mark Johnston from comment #6) This is a red herring, libc is doing locale reference counting properly. The real problem is that set_thread_locale() is using pthread_getspecific() to get the old locale even in a single-threaded program, and that doesn't work.
A commit references this bug: Author: markj Date: Fri Mar 20 20:02:53 UTC 2020 New revision: 359183 URL: https://svnweb.freebsd.org/changeset/base/359183 Log: Fix uselocale(3) to not leak a reference to the old locale. In a single-threaded program pthread_getspecific() always returns NULL, so the old locale would not end up being freed. PR: 239520 MFC after: 2 weeks Sponsored by: The FreeBSD Foundation Changes: head/lib/libc/locale/xlocale.c
A commit references this bug: Author: markj Date: Fri Apr 3 00:32:48 UTC 2020 New revision: 359583 URL: https://svnweb.freebsd.org/changeset/base/359583 Log: MFC r359183: Fix uselocale(3) to not leak a reference to the old locale. PR: 239520 Changes: _U stable/12/ stable/12/lib/libc/locale/xlocale.c
A commit references this bug: Author: markj Date: Fri Apr 3 00:38:12 UTC 2020 New revision: 359584 URL: https://svnweb.freebsd.org/changeset/base/359584 Log: MFC r359183: Fix uselocale(3) to not leak a reference to the old locale. PR: 239520 Changes: _U stable/11/ stable/11/lib/libc/locale/xlocale.c
This will be fixed in 11.4 and 12.2. Thanks for the report.
Hi! It seems that issue was fixed partially. C code above does not reproduce it on 12.2-RELEASE, but perl scripts still leak memory. For example, running this code: #!/usr/local/bin/perl -w use strict; use Sys::Syslog qw(:standard :macros); openlog('leak-demo', LOG_NDELAY, LOG_LOCAL2); my $c = 0; my @chars = ("A".."Z", "a".."z", "0".."9", "." , "-", "_" ); while ($c++ <= 10000) { my $string = ''; $string .= $chars[rand @chars] for 1..32; for (1..2048) { syslog(LOG_DEBUG, "DEBUG LOG %d/%d RANDOM STRING: %s", $c, $_, $string ); } sleep 1; } closelog(); ... with locale settings like these: LANG=uk_UA.UTF-8 LC_CTYPE="uk_UA.UTF-8" LC_COLLATE="uk_UA.UTF-8" LC_TIME="uk_UA.UTF-8" LC_NUMERIC="uk_UA.UTF-8" LC_MONETARY="uk_UA.UTF-8" LC_MESSAGES="uk_UA.UTF-8" LC_ALL= ... results to memory leak: last pid: 39263; load averages: 0.46, 0.36, 0.28 up 0+15:53:09 09:55:04 49 processes: 1 running, 48 sleeping CPU: 24.1% user, 0.0% nice, 11.4% system, 0.6% interrupt, 63.9% idle Mem: 770M Active, 558M Inact, 2068K Laundry, 447M Wired, 200M Buf, 231M Free Swap: 4096M Total, 52M Used, 4044M Free, 1% Inuse PID USERNAME THR PRI NICE SIZE RES STATE C TIME WCPU COMMAND 39237 root 1 35 0 659M 646M nanslp 1 0:18 18.06% perl [...] Running the same script with "env LANG=C" does not leak memory.
(In reply to Alexander Shikov from comment #12) This does not trigger any leaks on head. The problem is actually the one fixed in PR 249416, which did not make it to 12.2. I did not request an EN because there was only a single report of the problem and it was worked around in a port, but perhaps we should release one.
(In reply to Mark Johnston from comment #13) Dear Mark, thank you for feedback. We definitely would like to have this fixed in 12-STABLE. Thank you!
(In reply to Alexander Shikov from comment #14) Well it is already fixed in stable/12, the question is whether it is worth releasing a patch for 12.2-RELEASE.
(In reply to Mark Johnston from comment #15) It's up to you. We don't experience a deep impact on our services, it's just annoying when background scripts unexpectedly crash out of working hours.
Fixed in all currently supported releases.