Rapidjson: modifying document creating object and removing member causes assert when writing document

[mikealeonetti]

I have a bit of code where I parse a json file, edit it, and save it again. The code works fine as long as I don't create an object and then remove a member. Doing those two steps causes an assertion in the object where the member was removed when writing again. I've compiled a small example to show what I mean:

#define RAPIDJSON_HAS_STDSTRING 1

#include "rapidjson/document.h"
#include "rapidjson/prettywriter.h"
#include "rapidjson/istreamwrapper.h"

#include <iostream>
#include <fstream>


int main( int argc, char *argv[] )
{
    rapidjson::Document document;

    {
        std::ifstream file( "test.json" );
        rapidjson::IStreamWrapper file_wrapper( file );


        // Now load it up
        if( document.ParseStream( file_wrapper ).HasParseError() )
        {
            std::cout << "Configuration: Error parsing JSON." << std::endl;
            return( EXIT_FAILURE );
        }
    }

    // Now move some things around
    {
        auto &allocator = document.GetAllocator();

        // Shorthand
        auto &one = document[ "one" ];

        // Move each
        if( one.HasMember( "fruit" ) and one[ "fruit" ].IsString() )
        {
            const std::string &s = one[ "fruit" ].GetString();

            document.AddMember( "fruit", s, allocator );

            one.RemoveMember( "fruit" );
        }
        if( one.HasMember( "pear" ) and one[ "pear" ].IsString() )
        {
            std::string s = one[ "pear" ].GetString();

            rapidjson::Value value( rapidjson::kObjectType );

            value.AddMember( "pear", s, allocator );

            document.AddMember( "sugar", std::move( value ), allocator );

            one.RemoveMember( "pear" );
        }
        if( one.HasMember( "apple" ) and one[ "apple" ].IsString() )
        {
            const std::string &s = one[ "apple" ].GetString();

            document.AddMember( "apple", s, allocator );

            one.RemoveMember( "apple" );
        }
    }

    // Write it out
    rapidjson::StringBuffer sb;
    rapidjson::PrettyWriter<rapidjson::StringBuffer> writer( sb );

    // Write from the document
    document.Accept( writer );

    // Open the file
    std::ofstream file( "test-out.json" );

    // Now write
    file << sb.GetString();
}

Parsing this test.json file:

{
    "one" : {
        "fruit": "box",
        "apple": "plane",
        "pear" : "quincy"
    },
    "two" : {
        "french" : "pub"
    }
}

Will produce this output

report: rapidjson/document.h:1878: bool rapidjson::GenericValue<Encoding, Allocator>::Accept(Handler&) const [with Handler = rapidjson::PrettyWriter<rapidjson::GenericStringBuffer<rapidjson::UTF8<> > >; Encoding = rapidjson::UTF8<>; Allocator = rapidjson::MemoryPoolAllocator<>]: Assertion `m->name.IsString()' failed.
Aborted

If I take out all of the one.RemoveMember( ... ) lines the code will work. If I remove the block about adding an object it will work. RemoveMember and adding an object can't seem to co-exist.

How can this be sorted out?

Here is a backtrace from gdb:

__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50      ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007ffff7a65535 in __GI_abort () at abort.c:79
#2  0x00007ffff7a6540f in __assert_fail_base (fmt=0x7ffff7bdeea8 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=0x555555562181 "m->name.IsString()", file=0x55555556212c "rapidjson/document.h", line=1878, 
    function=<optimized out>) at assert.c:92
#3  0x00007ffff7a73852 in __GI___assert_fail (assertion=0x555555562181 "m->name.IsString()", file=0x55555556212c "rapidjson/document.h", line=1878, 
    function=0x555555564ec0 <rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >::Accept<rapidjson::PrettyWriter<rapidjson::GenericStringBuffer<rapidjson::UTF8<char>, rapidjson::CrtAllocator>, rapidjson::UTF8<char>, rapidjson::UTF8<char>, rapidjson::CrtAllocator, 0u> >(rapidjson::PrettyWriter<rapidjson::GenericStringBuffer<rapidjson::UTF8<char>, rapidjson::CrtAllocator>, rapidjson::UTF8<char>, rapidjson::UTF8<char>, rapidjson::CrtAllocator, 0u>&) const::__PRETTY_FUNCTION__> "bool rapidjson::GenericValue<Encoding, Allocator>::Accept(Handler&) const [with Handler = rapidjson::PrettyWriter<rapidjson::GenericStringBuffer<rapidjson::UTF8<> > >; Encoding = rapidjson::UTF8<>; Al"...) at assert.c:101
#4  0x000055555555a22d in bool rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >::Accept<rapidjson::PrettyWriter<rapidjson::GenericStringBuffer<rapidjson::UTF8<char>, rapidjson::CrtAllocator>, rapidjson::UTF8<char>, rapidjson::UTF8<char>, rapidjson::CrtAllocator, 0u> >(rapidjson::PrettyWriter<rapidjson::GenericStringBuffer<rapidjson::UTF8<char>, rapidjson::CrtAllocator>, rapidjson::UTF8<char>, rapidjson::UTF8<char>, rapidjson::CrtAllocator, 0u>&) const ()
#5  0x000055555555a2c5 in bool rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >::Accept<rapidjson::PrettyWriter<rapidjson::GenericStringBuffer<rapidjson::UTF8<char>, rapidjson::CrtAllocator>, rapidjson::UTF8<char>, rapidjson::UTF8<char>, rapidjson::CrtAllocator, 0u> >(rapidjson::PrettyWriter<rapidjson::GenericStringBuffer<rapidjson::UTF8<char>, rapidjson::CrtAllocator>, rapidjson::UTF8<char>, rapidjson::UTF8<char>, rapidjson::CrtAllocator, 0u>&) const ()
#6  0x00005555555568df in main ()

[miloyip]

I traced the code above and found the answer.
After calling document.AddMember(), one becomes an invalid reference.
This behavior is similiar to C++ containers:

~cpp
std::map m;
m.insert({ "apple", 1 });
auto &a = m["apple"];
m.insert({ "orange", 2}); // a becomes invalid reference
a = 3; // error
~

[jcmonnin]

Invalidating iterators and references when altering a container is a sensible choice and done in many places of the C++ standard library, but your example is a bit unhandy, given that std::map doesn't invalidate references when doing insert/emplace:

https://stackoverflow.com/questions/47280380/can-a-reference-to-an-element-inside-an-stdmap-be-invalidated

The same point can be made by changing the example to std::unordered_map

[mikealeonetti]

Hello @miloyip,

Thanks for checking into it.

Agree with @jcmonnin on this. Even your example works fine when compiled. I've expanded your example a bit also to:

#include <map>

#include <iostream>

int main( int argc, char *argv[] )
{
    std::map<std::string, int> m;
    m.insert({ "apple", 1 });
    auto &a = m["apple"];

    // Add some
    for( auto i=0; i<5000; ++i )
        m.insert({ "orange"+std::to_string( i ), 2}); // a becomes invalid reference

    // Erase some
    for( auto i=1000; i<2000; ++i )
        m.erase( m.find( "orange"+std::to_string( i ) ) );

    std::cout << a << std::endl;

    a = 3; // error

    std::cout << a << std::endl;
}

It will indeed print 1 and then 3 without segfaulting.

But what you're saying is that AddMember invalidates iterators?

[miloyip]

Yes, my fault. Sorry for the confusion. The behavior is more similiar to std::vector.
Anyway, the reference or iterator will become invalid when modifying a value/document in RapidJSON.

[piratf]

// Now move some things around
{
    auto &allocator = document.GetAllocator();

    // Shorthand
    auto &one = document[ "one" ];

    // Move each
    if( one.HasMember( "pear" ) and one[ "pear" ].IsString() )
    {
        std::string s1 = one[ "pear" ].GetString();

        rapidjson::Value value( rapidjson::kObjectType );

        value.AddMember( "pear", s1, allocator );

        document.AddMember( "sugar", s1, allocator );

        one.RemoveMember( "pear" );
    }

    one = document[ "one" ]; // you should do this.

    if( one.HasMember( "fruit" ) and one[ "fruit" ].IsString() )
    {
        const std::string &s = one[ "fruit" ].GetString();

        document.AddMember( "fruit", s, allocator );

        one.RemoveMember( "fruit" );
    }

    one = document[ "one" ]; // you should do this.

    if( one.HasMember( "apple" ) and one[ "apple" ].IsString() )
    {
        const std::string &s = one[ "apple" ].GetString();

        document.AddMember( "apple", s, allocator );

        one.RemoveMember( "apple" );
    }
}

Because the multiple call of AddMember action on the same allocator will trigger realloc, new chunks been alloced and memorys been moved to another place.

In this case, the reference one been used even after it's dangling, so the third if will write data to the wrong place.

The vector should have the same realloc behavior, be aware of it, take good care of the references.

[mikealeonetti]

Got it. Thank you!