search

naphaso / cbor-cpp

CBOR C++ serialization library
75 stars 29 forks source link

memory issues with the decoder #8

Open bijwaard opened 8 years ago

bijwaard commented 8 years ago

There are some memory issues in decoder.run() according to valgrind/memcheck. I looked at the decoder source and spotted two new unsigned char[_currentLength] statements (for string and bytes) that are not freed. The char array for std::string copies the data twice which would not be necessary (e.g. add a _in->get_string(str,_currentLength), for a byte array it is probably best freed after calling listener.on_bytes(), returning a const pointer within cbor_input memory may be a bit dangerous. I tested "delete data" after calling the listener callback in decoder.cpp (i.e. after _listener->on_bytes and _listener->on_string), and this resolved the memory leak for both cases.

Reducing copying of input data may bring the CBOR-cpp performance closer to BSONcxx (some early performance tests show about 9% speed difference for the same JSON data { "postcode":10000, "temperature":27, "relhumidity":35 } received over zeromq and averaging the temperature of 100000 messages like in the zeromq weather service example).

zoowii commented 6 years ago

https://github.com/zoowii/cbor-cpp I fix this bug in my fork(listener/visitor pattern is also changed). The pull request is not merged yet.

bijwaard commented 6 years ago

Thanks for your work on this bug. For my own usage I wrote a simple encoder/decoder that uses pre-allocated memory to store/parse the CBOR message contents in my internal data structure. That certainly boosts performance.