billyquith / ponder

C++ reflection library with Lua binding, and JSON and XML serialisation.
http://billyquith.github.io/ponder/
Other
642 stars 95 forks source link

Problems with ponder::Value and std::map #15

Closed iwbnwif closed 8 years ago

iwbnwif commented 8 years ago

I am using TDM MinGW64 on Windows 10.

There is a problem when a std::map containing ponder::Value objects goes out of focus. It calls the dtor for the Value objects, but the dtor seems to have problems freeing the memory.

Here is a simple test case to reproduce the problem:

    try
    {
        std::map<std::string, ponder::Value> testmap;

        ponder::Value value = "Hello";

        testmap["1"] = value;

        std::cout << "Testing: " << testmap["1"] << std::endl;

        // Error when testmap goes out of focus here.
    }
    catch (ponder::Error e)
    {
        std::cout << "Error: " << e.what() << std::endl;
    }

And here is the back trace that leads to the SIGTRAP

0  0x00007ffda889e69c  ntdll!RtlpNtMakeTemporaryKey    
1  0x00007ffda88a0d86  ntdll!RtlpNtMakeTemporaryKey    
2  0x00007ffda8854b9a  ntdll!RtlRaiseStatus    
3  0x00007ffda87d08f9  ntdll!RtlFreeHeap    
4  0x00007ffda6549b9c  msvcrt!free    
5  0x0000000000439bc3  mapbox::util::variant_helper<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, ponder::EnumObject, ponder::UserObject>::destroy  C:\\Users\\iwbnwif\\Development\\general_tests\\ponder\\ponder\\include\\ponder\\detail\\variant.hpp  211
6  0x0000000000439cf7  mapbox::util::variant_helper<double, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, ponder::EnumObject, ponder::UserObject>::destroy  C:\\Users\\iwbnwif\\Development\\general_tests\\ponder\\ponder\\include\\ponder\\detail\\variant.hpp  215
7  0x0000000000439d87  mapbox::util::variant_helper<long, double, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, ponder::EnumObject, ponder::UserObject>::destroy  C:\\Users\\iwbnwif\\Development\\general_tests\\ponder\\ponder\\include\\ponder\\detail\\variant.hpp  215
8  0x0000000000439c67  mapbox::util::variant_helper<bool, long, double, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, ponder::EnumObject, ponder::UserObject>::destroy  C:\\Users\\iwbnwif\\Development\\general_tests\\ponder\\ponder\\include\\ponder\\detail\\variant.hpp  215
9  0x0000000000439b07  mapbox::util::variant_helper<ponder::NoType, bool, long, double, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, ponder::EnumObject, ponder::UserObject>::destroy  C:\\Users\\iwbnwif\\Development\\general_tests\\ponder\\ponder\\include\\ponder\\detail\\variant.hpp  215
10  0x000000000043f143  mapbox::util::variant<ponder::NoType, bool, long, double, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, ponder::EnumObject, ponder::UserObject>::~variant  C:\\Users\\iwbnwif\\Development\\general_tests\\ponder\\ponder\\include\\ponder\\detail\\variant.hpp  794
11  0x00000000004506c8  ponder::Value::~Value  C:\\Users\\iwbnwif\\Development\\general_tests\\ponder\\ponder\\include\\ponder\\value.hpp  74
12  0x00000000004ca1dc  std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, ponder::Value>::~pair  C:\\TDM-GCC-64\\lib\\gcc\\x86_64-w64-mingw32\\5.1.0\\include\\c++\\bits\\stl_pair.h  96

Therefore I think that this is actually a mapbox::variant problem and not specific to Ponder.

For reference, the following works as expected:

    try
    {
        std::unordered_map<std::string, ponder::Value> testmap;

        ponder::Value value = 5; //"Hello";

        testmap["1"] = value;

        std::cout << "Testing: " << testmap["1"] << std::endl;

        // Works fine
    }
    catch (ponder::Error e)
    {
        std::cout << "Error: " << e.what() << std::endl;
    }
iwbnwif commented 8 years ago

The fix for this appears to be as simple as copying the latest mapbox::variant files variant.hpp and recursive_wrapper.hpp over the existing ones the Ponder include/ponder/detail folder.

Note: The recursive_wrapper.hpp seems to be renamed variant_recursive_wrapper.hpp in Ponder.

billyquith commented 8 years ago

I added a test based on your code:

TEST_CASE("Values can be held in containers")
{
    SECTION("a std::map")
    {
        std::map<std::string, ponder::Value> testmap;
        ponder::Value value = "Hello";
        testmap["1"] = value;
        REQUIRE(testmap["1"] == std::string("Hello"));
    }
}

This passes on all of the test platforms, so possibly this is a problem with your compiler or the library it uses? I will update the Variant code though.

billyquith commented 8 years ago

Current version 1.1.0 of Mapbox variant added. See https://github.com/billyquith/ponder/commit/2099d627ee516467147a1a65e4f07a6ed50c7f85. Hopefully this should fix your issue. Cannot replicate.

iwbnwif commented 8 years ago

Thank you, I can confirm that this is now fixed on my TDM-GCC64 setup.