Starting with github hash bf29aa0 and in between the 0.13.1 and 0.14 releases, json-c uses newlocale(3) which appears to cause increased memory usage on each call when base is supplied. We have 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 json-c 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. I wrote a test program that calls json_tokener_parse() followed by json_object_put() in a loop; rss slowly increases for every loop. I opened this upstream ticket: https://github.com/json-c/json-c/issues/668 Eric Hawicz responded, suggesting that FreeBSD's implementation of newlocale() might be causing the issue. His helpful test program confirms this. Looking at the first version of json-c to has the issue: https://github.com/json-c/json-c/commit/bf29aa0 HAVE_USELOCALE is defined when locale.h is present. This results in a different codepath in json_tokener.c where newlocale() is used instead of setlocale(). I'll submit a seperate PR for the newlocale() issue. The attached patch comments out HAVE_USELOCALE in post-configure to avoid the use of newlocale(). Also attached is a json-c test program which uses sysctl() to automatically track and report changes to rss and vsz and clearly shows the issue.
Created attachment 218034 [details] patch
Created attachment 218035 [details] test program
Created attachment 218036 [details] test program Makefile
Build info is available at https://gitlab.com/swills/freebsd-ports/pipelines/197164210
A commit references this bug: Author: leres Date: Wed Oct 7 22:11:30 UTC 2020 New revision: 551672 URL: https://svnweb.freebsd.org/changeset/ports/551672 Log: devel/json-c: Avoid use of newlocale(3) that results in increasing memory usage The json-c distribution began using newlocale(3) starting with 0.14. Unfortunately the FreeBSD implementation is not posix compliant and when called with a base does not modify and return it nor does it free it; it always allocates and returns a new locale, leaking the base locale. See the PR for a test program that demonstrates the json-c issue. Here is the upstream github issue: https://github.com/json-c/json-c/issues/668 The fix to the port is to comment out HAVE_USELOCALE in post-configure and avoid the use newlocale() for now. A fix for newlocale(3) is in progress: https://reviews.freebsd.org/D26522 So it is likely this problem will be solved in time for 12.3-RELEASE. PR: 249412 Approved by: sunpoet (maintainer timeout, 3 weeks) Changes: head/devel/json-c/Makefile
I committed the patch.
Sorry, I thought I replied. I prefer a patch file instead of doing it in configure stage. Could you please check this one [1]? Thanks! --- old/config.h +++ new/config.h @@ -126,7 +126,7 @@ #define HAVE_STRNCASECMP 1 /* Define to 1 if you have the `uselocale' function. */ -// #define HAVE_USELOCALE +/* #undef HAVE_USELOCALE */ /* Define to 1 if you have the `vasprintf' function. */ #define HAVE_VASPRINTF [1] https://people.FreeBSD.org/~sunpoet/patch/devel-json-c.txt
(In reply to Sunpoet Po-Chuan Hsieh from comment #7) I guess I was thinking an advantage of making the change it in the port Makefile is it can be wrapped with: .if ${OPSYS} == FreeBSD && ${OSVERSION} >= 1203000 ... .endif or whatever after this is fixed in libc. I tested your patch and it allows the test program to run without leaks.
(In reply to Craig Leres from comment #8) I agree with you that it should be conditional when newlocale() is fixed. At that time, we could move the patch file to an extra patch (EXTRA_PATCHES) and wrap it with OSVERSION check.
A commit references this bug: Author: sunpoet Date: Sat Oct 10 07:44:14 UTC 2020 New revision: 551865 URL: https://svnweb.freebsd.org/changeset/ports/551865 Log: Update r551672 Use a patch file instead doing it in configure stage. PR: 249412 Submitted by: sunpoet (myself) Tested by: leres Changes: head/devel/json-c/Makefile head/devel/json-c/files/ head/devel/json-c/files/patch-CMakeLists.txt