Bug 239520 - Possible memory leak in newlocale/uselocale
Summary: Possible memory leak in newlocale/uselocale
Status: New
Alias: None
Product: Base System
Classification: Unclassified
Component: misc (show other bugs)
Version: 12.0-RELEASE
Hardware: amd64 Any
: --- Affects Some People
Assignee: freebsd-bugs mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-07-30 07:00 UTC by Alexander Shikov
Modified: 2019-07-31 08:00 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Shikov 2019-07-30 07:00:55 UTC
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
Comment 1 Baggio Kwok 2019-07-31 07:39:37 UTC
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 }
Comment 2 Baggio Kwok 2019-07-31 07:53:45 UTC
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 }
Comment 3 Baggio Kwok 2019-07-31 08:00:33 UTC
Ok, a quick check shows atomic_fetchadd_long(_, -1) [2] should works properly, both linuxkpi and nginx do it like that.