Bug 249416 - Each call to newlocale(3) with a non-null base argument results in additional memory usage
Summary: Each call to newlocale(3) with a non-null base argument results in additional...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 12.1-RELEASE
Hardware: Any Any
: --- Affects Only Me
Assignee: Mark Johnston
Depends on:
Reported: 2020-09-18 01:33 UTC by Craig Leres
Modified: 2020-11-13 17:42 UTC (History)
1 user (show)

See Also:

newlocale() test program (3.83 KB, text/plain)
2020-09-18 01:33 UTC, Craig Leres
no flags Details
proposed patch (801 bytes, patch)
2020-09-19 18:09 UTC, Mark Johnston
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Craig Leres freebsd_committer 2020-09-18 01:33:46 UTC
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.
Comment 1 Mark Johnston freebsd_committer 2020-09-19 18:03:44 UTC
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.
Comment 2 Mark Johnston freebsd_committer 2020-09-19 18:09:03 UTC
Created attachment 218084 [details]
proposed patch

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.
Comment 3 Craig Leres freebsd_committer 2020-09-21 17:55:56 UTC
(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.
Comment 4 Mark Johnston freebsd_committer 2020-09-22 16:47:03 UTC
Comment 5 Mark Johnston freebsd_committer 2020-09-22 16:47:32 UTC
(In reply to Craig Leres from comment #3)
Thanks.  Are you able to test it with the original application?
Comment 6 commit-hook freebsd_committer 2020-10-02 18:36:05 UTC
A commit references this bug:

Author: markj
Date: Fri Oct  2 18:35:56 UTC 2020
New revision: 366375
URL: https://svnweb.freebsd.org/changeset/base/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".

  PR:		249416
  MFC after:	3 weeks
  Sponsored by:	The FreeBSD Foundation
  Differential Revision:	https://reviews.freebsd.org/D26522

Comment 7 commit-hook freebsd_committer 2020-10-23 14:48:27 UTC
A commit references this bug:

Author: markj
Date: Fri Oct 23 14:47:33 UTC 2020
New revision: 366971
URL: https://svnweb.freebsd.org/changeset/base/366971

  MFC r366375:
  newlocale(3): Fix a memory leak.

  PR:	249416

_U  stable/12/
Comment 8 Mark Johnston freebsd_committer 2020-10-23 14:59:38 UTC
So I suspect we'll want to issue an EN for this once 12.2 is released.  I'll keep this PR open for now until after the release goes out.
Comment 9 Craig Leres freebsd_committer 2020-10-23 18:06:33 UTC
Is it safe to assume this fix will be in 12.3? If so then we could wrap the workaround for devel/json-c with:

    .if ${OPSYS} == FreeBSD && ${OSVERSION} < 1203000

As described in the json-c PR:

Comment 10 Mark Johnston freebsd_committer 2020-10-23 21:06:34 UTC
(In reply to Craig Leres from comment #9)
Yes it will definitely be in 12.3.  The change is already in the stable/12 branch, where the __FreeBSD_version value is currently 1202503, so you could perhaps apply the workaround only if ${OSVERSION} <= 1202503.
Comment 11 Mark Johnston freebsd_committer 2020-11-13 17:42:23 UTC
I see that a workaround was applied to devel/json-c so I'm just going to go ahead and close this PR.  If there's some demand for an EN, we can issue one.