Corrupt output, possibly due to lost ownership of rapidjson::Document::Allocator

Question

I have some trouble with, what I think is, object ownership. My goal is to create a JSON-RPC message. What I want to achieve on a lower level is an array of objects, each of which has 1 object error. Everything goes fine, except for that inner error object.

Below you can find the output of my code. As you can see error.message has its first 8 characters replaced by null chars. The expected output should be Internal error: Don't know what happened.

[{"error":{"code":-32603,"message":"\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000 error: Don't know what happened"},"jsonrpc":"2.0","id":null}]

Since every object is created and added to the array inside a loop, the scope is lost after every iteration, so I have the idea it has to do with ownership. Possibly something like this? I tried about everything I could think of: creating a Document and AllocatorType for the error objects inside the loop, copy instead of move construct all objects, and more, but I'm a bit stuck here. Can anyone help me in managing the ownership of the inner error objects?

Thanks in advance!

My code:

void MessageCenter::write_rpc_response(network::Session& session,
        const std::vector<RPCResponse>& rpc_responses)
{
    rapidjson::Document json_responses;
    rapidjson::Document::AllocatorType& alloc = json_responses.GetAllocator();
    json_responses.SetArray();

    std::vector<RPCResponse>::const_iterator iter;
    for (iter = rpc_responses.begin(); iter != rpc_responses.end(); iter++)
    {
        rapidjson::Document resp;
        resp.SetObject();

        // Create error object, based on std::map
        struct { int64_t code = 0; std::string message = ""; } error;
        error.code = -32603;
        error.message = "Internal error: Don't know what happened";

        // Create JSON object, based on the error object
        rapidjson::Value data_object(rapidjson::kObjectType);

        data_object.AddMember("code",
                rapidjson::Value(error.code).Move(), alloc);
        rapidjson::Value error_message(error.message.c_str(),
                error.message.length());
        data_object.AddMember("message", error_message.Move(), alloc);

        resp.AddMember("error", data_object.Move(), alloc);

        // Add JSON-RPC version
        rapidjson::Value jsonrpc_version(iter->jsonrpc.c_str(),
                iter->jsonrpc.length());
        resp.AddMember("jsonrpc", jsonrpc_version.Move(), alloc);

        // Add id: null if 0, int otherwise
        if (iter->id == 0) {
            resp.AddMember("id", rapidjson::Value().Move(), alloc);
        }
        else {
            resp.AddMember("id", rapidjson::Value(iter->id).Move(), alloc);
        }
        json_responses.PushBack(resp, alloc);
    }

    // Write to the session
    rapidjson::StringBuffer buffer;
    rapidjson::Writer<rapidjson::StringBuffer> writer(buffer);
    json_responses.Accept(writer);
    session.Write(buffer.GetString());
}

All the `c_str()` calls extract pointers to the internals of objects that then go out of scope. Also, do you need that anonymous `struct`? — Ulrich Eckhardt, Mar 28 '16 at 14:57
@UlrichEckhardt Thanks a lot, it were indeed the extracted pointers from `c_str()` that went out of scope. I added some allocators and it works fine again. :-) No, I don't necessarily need that struct, but this code is a simplified version of my real code and in the more complex code I thought it might come in handy to structure the data. May I ask why? — , Mar 28 '16 at 15:22
The reason I ask is that people are supposed to first extract a minimal example before asking here. Hence I was wondering if it was somehow essential to demonstrating the problem. — Ulrich Eckhardt, Mar 28 '16 at 19:08

score 1 · Accepted Answer · answered Mar 29 '16 at 09:49

There are several APIs for constructing strings:

//! Constructor for constant string (i.e. do not make a copy of string)
GenericValue(const Ch* s, SizeType length) RAPIDJSON_NOEXCEPT : data_() { SetStringRaw(StringRef(s, length)); }

//! Constructor for constant string (i.e. do not make a copy of string)
explicit GenericValue(StringRefType s) RAPIDJSON_NOEXCEPT : data_() { SetStringRaw(s); }

//! Constructor for copy-string (i.e. do make a copy of string)
GenericValue(const Ch* s, SizeType length, Allocator& allocator) : data_() { SetStringRaw(StringRef(s, length), allocator); }

//! Constructor for copy-string (i.e. do make a copy of string)
GenericValue(const Ch*s, Allocator& allocator) : data_() { SetStringRaw(StringRef(s), allocator); }

The following code used the constant string version:

    rapidjson::Value error_message(error.message.c_str(),
            error.message.length());

But it is clearly error will go out of scope. You should use the copy-string API:

    rapidjson::Value error_message(error.message.c_str(),
            error.message.length(), alloc);

By the way, resp does not need to be a Document. It can be just a Value. This does not generate bugs if you always use the outer allocator, though.

Thanks for your elaborate answer, Milo! I understand it much better now. Keep up the good work with RapidJSON, it's much appreciated. :-) — , Mar 29 '16 at 16:30

Corrupt output, possibly due to lost ownership of rapidjson::Document::Allocator

1 Answers1