hyperrealm / libconfig

C/C++ library for processing configuration files
https://hyperrealm.github.io/libconfig/
GNU Lesser General Public License v2.1
1.12k stars 366 forks source link

Memory leak in Config::readString() #212

Closed ShadowZzj closed 2 years ago

ShadowZzj commented 3 years ago

I have this test code on Mac BigSur 11.2.3. And it appears that Config::readString() never release the memory it allocated(no exception occurs), the memory continue to raise and never drop down. When I comment out the line "cfg.readString()", the leak disappears. I have tested 1.7.2 and 1.7.3, all versions contain this issue. `

while (1) { int result = 0; Config cfg; cfg.setOptions(Config::OptionFsync | Config::OptionSemicolonSeparators | Config::OptionColonAssignmentForGroups | Config::OptionOpenBraceOnSeparateLine); try { std::string outStr = "version = \"0.1.11\";\nwifi_ssid = \"123\";\n1ACB-4383- : \n{\n filename = \"client.p12\";\n cert_passwd = \"6f8\";\n log_path = \"\";\n sid = \"28\";\n write_log = 0;\n};\n" ; cfg.readString(outStr.c_str()); } catch (const FileIOException &fioex) { result = -2; return result; } catch (const ParseException &pex) { result = -3; return result; } catch (...) { return -4; } }

`

thomastrapp commented 3 years ago

I cannot reproduce the issue on linux (debian testing, g++11.2.0) with libconfig release 1.7.3.

For example:

#include <cstdlib>
#include <string>
#include <iostream>

#include <libconfig.h++>

int main(int argc, char ** argv)
{
  if( argc < 2 )
    return EXIT_FAILURE;

  auto i = std::stoul(argv[1]);

  std::cout << "i = " << i << "\n";

  while( i-- > 0 )
  {
    using namespace libconfig;

    Config cfg;
    cfg.setOptions(
      Config::OptionFsync |
      Config::OptionSemicolonSeparators |
      Config::OptionColonAssignmentForGroups |
      Config::OptionOpenBraceOnSeparateLine);

    try
    {
      std::string outStr = "version = \"0.1.11\";\nwifi_ssid = \"123\";\nACB4383: \n{\n  filename = \"client.p12\";\n  cert_passwd = \"6f8\";\n  log_path = \"\";\n  sid = \"28\";\n  write_log = 0;\n};\n";
      cfg.readString(outStr.c_str());
    }
    catch(const ParseException &pex)
    {
      std::cerr << pex.getError() << "\n";
      return EXIT_FAILURE;
    }
    catch (...)
    {
      return EXIT_FAILURE;
    }
  }

  return EXIT_SUCCESS;
}
g++ -o issue212 -Wall -O3 issue212.cpp -lconfig++

valgrind ./issue212 100000
# => ok, all heap blocks freed

valgrind --tool=massif ./issue212 100000
# => ok, heap usage remains stable at about 73kb

Can you provide a SSCCE exhibiting the memory leak on your Mac?

ShadowZzj commented 3 years ago

I use thie piece of code to test memory leak on my Mac 11.2.3 and Mac 11.6. The Xcode version is 12.3 beta. The version of libconfig I compile is at commit 5ea4462. The compile command I use is "cmake ../" and "make" The memory continue to raise when I use xcode to debug this code. I have also tested the same code on Windows 10 and the vs2019 shows that there is no memory leak.


//test code
#include <libconfig/libconfig.h++>
#include <iostream>
#include <string>
using namespace libconfig;
int main()
{
    while (1)
            {
                using namespace libconfig;

                Config cfg;
                cfg.setOptions(Config::OptionFsync | Config::OptionSemicolonSeparators |
                               Config::OptionColonAssignmentForGroups | Config::OptionOpenBraceOnSeparateLine);

                try
                {
                    std::string outStr =
                        "version = \"0.1.11\";\nwifi_ssid = \"123\";\nACB4383: \n{\n  filename = \"client.p12\";\n  "
                        "cert_passwd = \"6f8\";\n  log_path = \"\";\n  sid = \"28\";\n  write_log = 0;\n};\n";
                    cfg.readString(outStr.c_str());
                }
                catch (const ParseException &pex)
                {
                    std::cerr << pex.getError() << "\n";
                    return EXIT_FAILURE;
                }
                catch (...)
                {
                    return EXIT_FAILURE;
                }
            }
}
thomastrapp commented 3 years ago

Does Xcode report a memory leak? Does it report a specific location in code where memory was lost? I think it has a tool that is able to detect memory leaks. I am not familiar with xcode.

Are you on ARM64?

ShadowZzj commented 3 years ago

I have use the xcode leak profile tool. It says the leak stack as below:

libconfig::Config::readString(char const*) __config_read __config_locale_override newlocale _duplocate _malloc_zone_malloc

ps: I am on Intel Code I7

thomastrapp commented 3 years ago

I can confirm the memory leak: A locale is allocated here but never freed, because HAVE_USELOCALE and HAVE_FREELOCALE are undefined here.

I think this only applies to cmake builds of libconfig (and libconfig++) on Mac OS.

I'll create a pull request with a fix.

ShadowZzj commented 3 years ago

OK. Thanks for your in time reply.

thomastrapp commented 3 years ago

Pull request

diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt
index fb7f095..709a1b5 100644
--- a/lib/CMakeLists.txt
+++ b/lib/CMakeLists.txt
@@ -58,10 +58,6 @@ set_target_properties(${libname}++
         VERSION "${libconfig_VERSION}"
         PUBLIC_HEADER "${libinc_cpp}")

-#check_symbol_exists(uselocale "locale.h" HAVE_USELOCALE)
-#check_symbol_exists(newlocale "locale.h" HAVE_NEWLOCALE)
-#check_symbol_exists(freelocale "locale.h" HAVE_FREELOCALE)
-
 if(BUILD_SHARED_LIBS)
     target_compile_definitions(${libname}++ PRIVATE LIBCONFIG_STATIC)
 else()
@@ -69,19 +65,35 @@ else()
     target_compile_definitions(${libname}++ PUBLIC LIBCONFIGXX_STATIC)
 endif()

+if(APPLE)
+    check_symbol_exists(uselocale "xlocale.h" HAVE_USELOCALE)
+    check_symbol_exists(newlocale "xlocale.h" HAVE_NEWLOCALE)
+    check_symbol_exists(freelocale "xlocale.h" HAVE_FREELOCALE)
+else()
+    check_symbol_exists(uselocale "locale.h" HAVE_USELOCALE)
+    check_symbol_exists(newlocale "locale.h" HAVE_NEWLOCALE)
+    check_symbol_exists(freelocale "locale.h" HAVE_FREELOCALE)
+endif()
+
 if(HAVE_USELOCALE)
-target_compile_definitions(${libname}
-    PRIVATE "HAVE_USELOCALE")
+    target_compile_definitions(${libname}
+        PRIVATE "HAVE_USELOCALE")
+    target_compile_definitions(${libname}++
+        PRIVATE "HAVE_USELOCALE")
 endif()

 if(HAVE_NEWLOCALE)
-target_compile_definitions(${libname}
-    PRIVATE "HAVE_NEWLOCALE")
+    target_compile_definitions(${libname}
+        PRIVATE "HAVE_NEWLOCALE")
+    target_compile_definitions(${libname}++
+        PRIVATE "HAVE_NEWLOCALE")
 endif()

 if(HAVE_FREELOCALE)
-target_compile_definitions(${libname}
-    PRIVATE "HAVE_FREELOCALE")
+    target_compile_definitions(${libname}
+        PRIVATE "HAVE_FREELOCALE")
+    target_compile_definitions(${libname}++
+        PRIVATE "HAVE_FREELOCALE")
 endif()

 if(MSVC)

Thank you for the bug report

thomastrapp commented 3 years ago

The fix was merged into master. Affected builds need a rerun of cmake, make, etc.

@ShadowZzj Can you confirm that the issue is resolved on your end? Thanks

ShadowZzj commented 3 years ago

I have pull the newest commit and compile the newest libconfig++. The leak doesn't appear anymore.