valgrind reports 'potential memory leaks' from gflags

[GoogleCodeExporter]

What steps will reproduce the problem?
1. compile a program using gflags
2. run it with 'valgrind --leak_check=full'
3. look at all the memory leak reports from gflags

What is the expected output? What do you see instead?

expected: clean memory leak report
instead:  saw lots of potential memory leaks

These are not really memory leaks, but the presence of these leaks makes it 
harder to see real memory leaks.  Note that 'potential' memory leaks are 
sometimes quite dangerous.  For example, when working with Apache HTTPD, a 
large amount of memory allocation is using a pooled allocator.  You can leak 
memory into pools, and valgrind will report that as a 'potential leak'.

The gflags leak reports are not really problems like Apache pools, but the 
presence of them can obscure real problems.

What version of the product are you using? On what operating system?

http://google-gflags.googlecode.com/svn/tags/gflags-1.3/src (head)
OS: Linux Ubtuntu 10.04.

Please provide any additional information below.

Original issue reported on code.google.com by jmara...@google.com on 13 Dec 2010 at 9:23

[GoogleCodeExporter]

I'm ok with adding something like this if it doesn't interfere with normal 
usage (that is, most people can ignore it, and it doesn't complexify 
understanding of the code).  Should be doable for the next release.

Original comment by csilv...@gmail.com on 13 Dec 2010 at 10:30

• Changed state: Started

[GoogleCodeExporter]

A function that frees memory was added in gflags 1.5, just released.

Original comment by csilv...@gmail.com on 25 Jan 2011 at 12:33

• Changed state: Fixed

[GoogleCodeExporter]

Quick question on this.  I tried to update my open-source dependencies to
bring in version 1.5 but I got a 'url is unreachable error' from 'gclient':

Error: This url is unreachable:
http://google-gflags.googlecode.com/svn/tags/gflags-1.5/src@head

I went over to http://code.google.com/p/google-gflags/source/browse/ and
looked at the tags.  I don't see a gflags-1.5 yet. Is that coming?

Thanks!

 Directories

   - svn <http://code.google.com/p/google-gflags/source/browse/>
      - branches<http://code.google.com/p/google-gflags/source/browse/branches>
         - tags <http://code.google.com/p/google-gflags/source/browse/tags>
         - gflags-0.1<http://code.google.com/p/google-gflags/source/browse/#>
         - gflags-0.2<http://code.google.com/p/google-gflags/source/browse/#>
         - gflags-0.3<http://code.google.com/p/google-gflags/source/browse/#>
         - gflags-0.4<http://code.google.com/p/google-gflags/source/browse/#>
         - gflags-0.5<http://code.google.com/p/google-gflags/source/browse/#>
         - gflags-0.6<http://code.google.com/p/google-gflags/source/browse/#>
         - gflags-0.7<http://code.google.com/p/google-gflags/source/browse/#>
         - gflags-0.8<http://code.google.com/p/google-gflags/source/browse/#>
         - gflags-0.9<http://code.google.com/p/google-gflags/source/browse/#>
         - gflags-1.0rc1<http://code.google.com/p/google-gflags/source/browse/#>
         - gflags-1.0rc2<http://code.google.com/p/google-gflags/source/browse/#>
         - gflags-1.1<http://code.google.com/p/google-gflags/source/browse/#>
         - gflags-1.2<http://code.google.com/p/google-gflags/source/browse/#>
         - gflags-1.3<http://code.google.com/p/google-gflags/source/browse/#>
         - gflags-1.4<http://code.google.com/p/google-gflags/source/browse/#>
      - trunk <http://code.google.com/p/google-gflags/source/browse/trunk>

Original comment by jmara...@google.com on 25 Jan 2011 at 12:54

[GoogleCodeExporter]

My mistake -- it looks like my svn tag command failed.  I just ran it again, so 
everything should be fixed now.  Thanks for pointing this out!

Original comment by csilv...@gmail.com on 25 Jan 2011 at 1:32

[GoogleCodeExporter]

It looks like this is still leaking std::strings for DEFINE_string constants.

Is there any way to clean those up too?

I can add valgrind suppression for that, but it still gives me errors if I when 
I change the default flag value (as shown here 
http://google-gflags.googlecode.com/svn/trunk/doc/gflags.html#default), which 
are tedious to have to go through and individually suppress.

Original comment by alastair.maw on 13 Sep 2012 at 2:08

[GoogleCodeExporter]

Yes.  You need to clean up the constants on exit, which you can do by calling 
ShutDownCommandLineFlags().

Original comment by jmara...@google.com on 13 Sep 2012 at 2:12

[GoogleCodeExporter]

I'm already calling this on shutdown, but I still get valgrind (3.6.1) 
complaining. That said, I've committed the cardinal sin of whinging on a bug 
but having a slightly old version of both valgrind and gflags. I'll update both 
and try again, and get back to you with a better report. Thanks.

Original comment by alastair.maw on 14 Sep 2012 at 9:27

[GoogleCodeExporter]

OK, with the latest valgrind and gflags this is still an issue, even with the 
most basic possible program:

#include <gflags/gflags.h>

DEFINE_bool(foo, false, "Test flag");
DEFINE_string(bar, "bar", "bar flag");

int main(int argc, char** argv)
{
    google::ShutDownCommandLineFlags();
    return 0;
}

If I remove the DEFINE_string() then I get no warnings.

It seems that the std::string that DEFINE_string is allocating is leaking (I 
guess in line 530 of gflags.h, where it's allocated, or 551 where it's kept but 
never freed, depending how you look at it):

==18700== HEAP SUMMARY:
==18700==     in use at exit: 28 bytes in 1 blocks
==18700==   total heap usage: 82 allocs, 81 frees, 3,116 bytes allocated
==18700==
==18700== 28 bytes in 1 blocks are possibly lost in loss record 1 of 1
==18700==    at 0x4A0867F: operator new(unsigned long) (vg_replace_malloc.c:298)
==18700==    by 0x3A5E89B860: std::string::_Rep::_S_create(unsigned long, 
unsigned long, std::allocator<char> const&) (in /usr/lib64/libstdc++.so.6.0.8)
==18700==    by 0x3A5E89C364: ??? (in /usr/lib64/libstdc++.so.6.0.8)
==18700==    by 0x3A5E89C511: std::basic_string<char, std::char_traits<char>, 
std::allocator<char> >::basic_string(char const*, std::allocator<char> const&) 
(in /usr/lib64/libstdc++.so.6.0.8)
==18700==    by 0x400AFF: fLS::dont_pass0toDEFINE_string(char*, char const*) 
(in /home/[...]/foo)
==18700==    by 0x4009C7: __static_initialization_and_destruction_0(int, int) 
(in /home/[...]/foo)
==18700==    by 0x400A85: global constructors keyed to fLB::FLAGS_foo (in 
/home/[...]/foo)
==18700==    by 0x400C25: ??? (in /home/[...]/foo)
==18700==    by 0x4007BA: ??? (in /home/[...]/foo)
==18700==    by 0x3A5EAEF0FF: ???
==18700==    by 0x400BA6: __libc_csu_init (in /home/[...]/foo)
==18700==    by 0x3A5CC1D92D: (below main) (in /lib64/libc-2.5.so)

Original comment by alastair.maw on 18 Sep 2012 at 10:58

[GoogleCodeExporter]

OK, thanks for the report and nice example !

There are two uses of placement-new, one at line 530 and the other at 556 of 
gflags.h. In both cases, the strings are passed on to FlagValue(), which never 
takes over ownership of the memory and thus will not free it upon 
ShutDownCommandLineFlags(). So I am not clear yet why only the first would 
cause a valgrind warning...

I have compiled your example and run it through valgrind 3.6.1. I get the same, 
so I reopen this bug and will do some debugging and see if we can get rid of 
this last valgrind warning.

Original comment by andreas....@gmail.com on 18 Sep 2012 at 1:27

• Changed state: Started

[GoogleCodeExporter]

OK, I figured it out. What is missing is the explicit destructor call for the 
string objects that were created via placement-new. See also 
http://www.stroustrup.com/bs_faq2.html#placement-delete . Adding these manually 
to the example made the valgrind warning disappear.

Now that I know what is missing and how to close this leak, I just need to 
figure how best to patch the code...

Original comment by andreas....@gmail.com on 19 Sep 2012 at 7:51

[GoogleCodeExporter]

I added the placement delete template function from 
http://www.stroustrup.com/bs_faq2.html#placement-delete to gflags.cc and 
modified the destructor of FlagValue to call it for string objects created via 
placement new. This, however, given your provided example code, did not help to 
resolve this issue. Only when I add a call to the placement delete for both 
string objects created by DEFINE_string for the current and default value after 
ShutDownCommandLineFlags() in main(), the valgrind warnings disappear.

It seems to somewhat matter in which module the destructor of the object is 
called. If called in the main module, i.e., the one containing the 
corresponding DEFINE_string, it seems to work. If called within gflags.cc, 
however not... (linking statically to gflags library).

Also, given the issue at hand, it is surprising that their are no warnings for 
those string flags defined in gflags_reporting.cc, even without explicit call 
of the destructor of placement new-created string objects.

Maybe somebody can enlighten me ? I will keep looking for a solution, but feel 
like I'm missing something important yet.

Original comment by andreas....@gmail.com on 20 Nov 2012 at 11:25

[GoogleCodeExporter]

Is this issue still open?

I experienced that valgrind warnings only appear, when a default value for the 
string is given:

DEFINE_string(foo, "c", "foo");//valgrind warning reports "..possibly lost.."
DEFINE_string(foo, "", "foo");//no valgrind report

Hopefully this helps to get closer to the fix.

Original comment by thomasel...@googlemail.com on 31 Jul 2013 at 11:37

[GoogleCodeExporter]

Bump.

Any news on this?

Original comment by thomasel...@googlemail.com on 20 Aug 2013 at 9:17

[GoogleCodeExporter]

Hello, I replied by mail a few weeks ago wrongly assuming the reply would get 
attached here. It wasn't. I decided to just dump below my original message, 
dated 31 Jul.

- - - - - (in reply to message 31 Jul) - - - - -
Yes and no.
Looks like both me and Andreas forgot to add a note about this.

There's a branch (bugfix-40-memory-leaks) which fixed this behaviour.
Howerer, due to compatibility concerns, it has not been merged with master, 
default version.

I've implemented the fix myself but Andreas took it one step further by seeing 
the real meaning in what I did. In the end looks like none of us were 
sufficiently close to language's low level plumbings dealing with memory 
initialization. The question is: can this memory be safely reclaimed at all?

The current approach in the fixed branch is to reclaim memory - thus breaking 
compatibility in some extreme usage cases - while leaving string initialization 
order and processing as is. Andreas noticed this appeared to be the same of 
just using std::string objects directly but we weren't sure of the details.

I know this probably does not help much but I'm afraid it's the best we can do 
without extensive testing nor contacting someone with special knowledge on the 
subject.

Massimo Del Zotto

Original comment by Massimo...@gmail.com on 20 Aug 2013 at 9:21

[GoogleCodeExporter]

After the release of v2.1 (which mainly contains minor bug fixes and in 
particular the migration to CMake), I would like to eventually resolve this 
issue (and others) as part of v2.2.

Original comment by andreas....@gmail.com on 20 Mar 2014 at 3:32

• Added labels: Milestone-v2.2