Memory usage regression due to newlocal() on older FreeBSD releases

[leres]

Note: for general questions and comments, please use the forums at:
https://groups.google.com/forum/#!forum/json-c

Describe the bug
We're using the json-c library under FreeBSD 11.3-RELEASE in a persistent daemon that repeatedly parses some trivial json from a piece of network equipment. This software was developed while the FreeBSD devel/json-c port was at 0.13.1 (or older). The port was updated to 0.14 in May 2020 and to 0.15 in mid August. After the 0.15 update the daemon was rebuilt/installed and after ~12 hours of operation would crash due to memory exhaustion. Eventually we determined that downgrading to 0.13.1 solved the memory growth.

Attached is a simple test program that calls json_tokener_parse() followed by json_object_put() repeatedly and demonstrates the problem. It may well be that this problem is isolated to FreeBSD. Testing shows the problem exists on 11.3-RELEASE, 12-1-RELEASE, and 13.0-CURRENT. When built on FreeBSD, the test program uses the sysctl() to automatically track and report changes to rss and vsz. A binary search of git hashes shows bf29aa0 is the first to exhibit the issue.

jsontest.zip

The search was complicated by the fact that there are many files tracked by git and are also in the .gitignore file and "git clean -f -d" does not remove them.

Steps To Reproduce
List the steps to reproduce the behavior.
If possible, please attach a sample json file and/or a minimal code example.

Version and Platform

• json-c version: bf29aa0
• OS: FreeBSD 11.3-RELEASE-p9
• Custom cmake/build flags (see attached)

[hawicz]

My guess is that this is a difference in how newlocale() works on FreeBSD vs Linux.
https://www.freebsd.org/cgi/man.cgi?query=newlocale&sektion=3&manpath=freebsd-release-ports
"Creates a new locale, inheriting some properties from an existing locale."

https://man7.org/linux/man-pages/man3/newlocale.3.html
"The newlocale() function creates a new locale object, or modifies an existing object..."

Posix says:
https://pubs.opengroup.org/onlinepubs/9699919799/functions/newlocale.html
"The newlocale() function shall create a new locale object or modify an existing one."

so I think this is a FreeBSD bug.

Can you try this program to see if it leaks for you?:

#include <locale.h>
int main()
{
  locale_t oldlocale = uselocale(NULL);
  while(1)
  {
      locale_t duploc = duplocale(oldlocale);
      locale_t newloc = newlocale(LC_NUMERIC_MASK, "C", duploc);
      //freelocale(duploc);  // Should not be needed
      freelocale(newloc);
  }
}

[leres]

Indeed your test program leaks on FreeBSD.

This will be nasty to fix since calling freelocale() on an already freed block dups core (as I would expect) on both FreeBSD and a random linux system I tested.

I really appreciate the quick diagnosis.

[hawicz]

newlocale_test.c.txt

Here's a test program to try to figure out, on any particular platform, whether newlocale() follows the posix behavior or not. Running on Linux (Ubuntu 19.10), it exits with 0, but on your machine I'd expect it to exit with 1. Can you please try it and let me know if it does?

I'm thinking that we can use this, somehow, as part of the cmake config process to determine whether the json_tokener code should include an extra freelocale() call or not.

[leres]

The test program does not detect that FreeBSD's newlocale() is not compliant; I wanted to see if I could fix it but have been too busy. I hope to find some time next week.

Meanwhile a fix is working through the FreeBSD review process: newlocale(3): Fix a memory leak.

[hawicz]

Another relevant FreeBSD bug report: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=239520
The most recent (2023-03-30) comment there says "Fixed in all currently supported releases.", which currently includes FreeBSD 12.4, 13.1 and 13.2

I suspect the newlocale_test.c.txt I mentioned above doesn't detect the issue b/c the freebsd code calls calloc instead of malloc, and that calloc implementation (jemalloc) doesn't call malloc internally.

If we still want to support earlier versions of FreeBSD that still have the problem then we need to either get a test like that working (or ideally w/ some more reliable way to detect "too much" memory usage), or just provide a NEWLOCALE_NEEDS_FREELOCALE cmake setting.

[hawicz]

I added the cmake setting, so I'll calling this good enough for now. If anyone wants to figure out a way to automatically determine whether the bug exists, feel free to open a new PR.