msgpack-c icon indicating copy to clipboard operation
msgpack-c copied to clipboard

bad_cast exception when deserializing vector of nested packed messages

Open benlemond opened this issue 5 years ago • 4 comments

I am having a problem where a larger number of nested "ZoneConfigMsgs" within a std::vector<ZoneConfigMsg> causes a bad_cast exception. In the following code, if I pass a 40 into the SeedData function I get bad_cast exceptions all day on the unpack side. That 40 creates 40 instances of a "Zone". If I pass in 1 or 2, the pack->unpack sequence works pretty well. Anything over two and I see periodic bad_cast exceptions.

Are there limits to the amounts of data that can be send in a packed object?

void TestPacking()
{
        try
        {
           //Pack it
            MsgPackData::ZoneConfigMsg msg = SeedData(40);
            std::stringstream sbuf;
              msgpack::pack(sbuf, msg);
           //UnPack it
            msgpack::object msg_obj = msgpack::unpack(sbuf.str().c_str(), sbuf.str().length()).get();
            auto output = msg_obj.as<MsgPackData::ZoneGroupConfigMsg>();
            LOG.debug("Received ZoneGroupConfigMsg; zoneCount = {0}", output.zones.size());
        }
        catch (const msgpack::type_error& err) {
            LOG.error("MqttSvc::ProcessRequestMsgs() Invalid type error.  Type = {0}", err.what());
        }
        catch (const std::invalid_argument& err) {
            LOG.error("MqttSvc::ProcessRequestMsgs() Invalid argument error.  Type = {0}", err.what());
        }
        catch (const std::exception& err) {
            LOG.error("MqttSvc::ProcessRequestMsgs() Standard error.  Type = {0}", err.what());
        }
        catch (...) {
            LOG.error("MqttSvc::ProcessRequestMsgs() Unknown error.");
        }
}

Seed function

MsgPackData::ZoneGroupConfigMsg SeedData(int number )
{
            MsgPackData::ZoneGroupConfigMsg zg;
            for (int i = 0; i < number; i++) {
                MsgPackData::ZoneConfigMsg z;
                z.zoneType = LocalData::ZONE_TYPE_SHUTDOWN;
                z.zoneApplication = LocalData::ZONE_APPLICATION_EVERYONE;
                z.activeMode = LocalData::ZONE_ACTIVE_ALWAYS;
                z.geometryType = LocalData::ZONE_GEOMETRY_CAMERA_VIEWPORT;
                z.cameraViewports = {0, 1, 2, 3, 4, 5};
                z.label = "Default Shutdown Zone";
                z.id = i;
                z.zonePool = 0;
                z.classList = {"pedestrian"};
                zg.zones.push_back(z);
            }
           return zg;
}

Data Structures

struct MsgBase {
    MsgPackMsgType msg_type {MsgPackMsgType::UNKNOWN};
    std::string version = MSG_PACK_DATA_VERSION;
    uint64_t timestamp {0};
    MSGPACK_DEFINE(msg_type, version, timestamp);
};
struct ZoneConfigMsg{
    int id {0};
    std::string label;
    LocalData::ZoneType zoneType {LocalData::ZONE_TYPE_UNKNOWN};
    LocalData::ZoneApplication zoneApplication {LocalData::ZONE_APPLICATION_NOONE};
    LocalData::ZoneActiveMode activeMode {LocalData::ZONE_ACTIVE_ALWAYS};
    LocalData::ZoneGeometryType geometryType {LocalData::ZONE_GEOMETRY_CAMERA_VIEWPORT};
    int zonePool {0};
    std::vector<CartesianPoint2D> points;
    std::vector<unsigned int> cameraViewports;
    std::vector<std::string> classList;
    MSGPACK_DEFINE(id, label, zoneType, zoneApplication, activeMode, geometryType, zonePool, points,
            cameraViewports, classList);
};
struct CartesianPoint2D{
    float x {0};
    float y {0};
    MSGPACK_DEFINE(x, y);
};

benlemond avatar Feb 09 '19 02:02 benlemond

            msgpack::object msg_obj = msgpack::unpack(sbuf.str().c_str(), sbuf.str().length()).get();

Seems to be wrong. msgpack::unpack(sbuf.str().c_str(), sbuf.str().length()) returns msgpack::object and you need to keep the lifetime of it.

            msgpack::object msg_obj = msgpack::unpack(sbuf.str().c_str(), sbuf.str().length()).get();
            // temporary `msgpack::object::handle` is already been destroyed here

So, you can update as follows:

            msgpack::object_handle oh = msgpack::unpack(sbuf.str().c_str(), sbuf.str().length());
            msgpack::object msg_obj = oh.get();
            auto output = msg_obj.as<MsgPackData::ZoneGroupConfigMsg>();

Or

        auto output = msgpack::unpack(sbuf.str().c_str(), sbuf.str().length()).get().as<MsgPackData::ZoneGroupConfigMsg>();

See msgpack::object_handle description at https://github.com/msgpack/msgpack-c/wiki/v2_0_cpp_object,

redboltz avatar Feb 09 '19 05:02 redboltz

there are a lot of codes file for the same topic how do we understand which one to use? should we combine all of them

KYZRsoze avatar Feb 11 '19 00:02 KYZRsoze

Redboltz, I tried your first suggestion and seemed to have some luck with it. I was already assuming my original error was somehow scope related so that made sense.

I plan to put it through some more rigorous tests tomorrow at work, but I think that will be the answer.

benlemond avatar Feb 11 '19 00:02 benlemond

Important point is keeping lifetime of msgpack::object_handle.

Consider this example that has the same concept.

char const* ptr = std::string("ABC").c_str();

http://www.cplusplus.com/reference/string/string/c_str/

The temporary object std::string("ABC") is over in the line. So ptr points to (could be) freed memory. It causes undefined behavior.

https://wandbox.org/permlink/8KJ5ghiwT0gYnbgA ptr1 points expected string but it is one example of undefined behavior.

In order to avoid the undefined behavior,

auto str = std::string("ABC");
char const* ptr = s.c_str();
// ptr point to valid memory thanks to str's lifetime

or

std::cout << std::string("ABC").c_str() << std::endl;
// do all things **before** temporary object lifetime is over

std::string and c_str() relationship is the same as msgpack::object_handle and obj() relationship.

You can choose any approach. Former and latter are up to you. By the way, msgpack::object_handle provides -> operator. It is similar to std::shared_ptr's one.

See https://wandbox.org/permlink/vt9Tgh06nXVmhuWD

It might be useful to apply the latter approach.

redboltz avatar Feb 11 '19 01:02 redboltz