Created attachment 218038 [details]
newlocale() test program
There appears to be a memory usage issue with newlocale(3) when called with a base argument. This recently started showing symptoms in devel/json-c programs using version 0.14 and higher which started using newlocale() due to a change in the cmake configuration. Here is the devel/json-c PR:
Here is the upstream ticket I created:
Eric Hawicz responded, suggesting that FreeBSD's implementation of newlocale() might be causing the issue. His helpful test program confirms this. Attached to this PR is a modified version that uses sysctl() to automatically track and report changes to rss and vsz.
Eric points out that FreeBSD and linux man pages describe different behavior; FreeBSD:
"Creates a new locale, inheriting some properties from an existing locale."
Note that there do not appear to be uses of newlocale() with a non-null base argument in the current source tree; linux:
"The newlocale() function creates a new locale object, or modifies an existing object..."
And that posix says:
"The newlocale() function shall create a new locale object or modify an existing one."
I did some testing with versions of lib/libc/locale going back to 2012 and all had this issue.
I suspect it'll be ok to make newlocale() release a reference on the base locale. I tested a patch that does exactly this and it appears to fix the leak.
Created attachment 218084 [details]
Here is a proposed patch. I am not certain about it yet, so please be careful if you want to test it. It is missing a man page update. An exp-run may be warranted, I'm not sure. The patch may create bugs in anything that relies on being able to access the base locale after newlocale() returns. Linux explicitly prohibits this, so I guess we should be ok most of the time.
(In reply to Mark Johnston from comment #2)
The patch tests good for me on my 13.0-CURRENT box; both my localetest and jsontest programs went from memory growth to none just by installing an updated libc.so.
(In reply to Craig Leres from comment #3)
Thanks. Are you able to test it with the original application?
A commit references this bug:
Date: Fri Oct 2 18:35:56 UTC 2020
New revision: 366375
newlocale(3): Fix a memory leak.
newlocale() optionally takes a "base" locale, from which components not
specified in the mask are inherited. POSIX says that newlocale() may
modify "base" and return it, or free "base" and return a newly allocated
locale. We were not doing either, so applications which use newlocale()
to modify an existing base locale end up leaking memory on FreeBSD.
This diff fixes the leak by releasing a reference to the base locale
before returning. This is less efficient than modifying "base"
directly, but is simpler for an initial bug fix. Also, update the man
page to clarify behaviour with respect to "base".
MFC after: 3 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D26522