Comments (3)
Thanks for the bug report. It should be fixed now. The writer will clean up properly when destroyed with an open builder. I don't have great tests for it at the moment (or for any of the builder code for that matter.) This will hopefully improve later. Thanks for using the builder, and please report any more bugs you find.
If you run your test with the latest code, the call to mpack_writer_destroy()
returns mpack_error_bug
after cleaning everything up. This indicates there is a bug in the calling code, in this case the missing call to mpack_complete_map()
. You can print the error like this:
// finish writing
mpack_error_t error = mpack_writer_destroy(&writer);
if (error != mpack_ok) {
fprintf(stderr, "An error occurred encoding the data! %s (%i)\n",
mpack_error_to_string(error), error);
return 1;
}
This will now print:
An error occurred encoding the data! mpack_error_bug (8)
Valgrind will report no leaks or other problems.
If you additionally build with -DDEBUG
, the write tracking will know that you have an unclosed map so it will assert in the call to mpack_writer_destroy()
and print:
mpack breakpoint hit at src/mpack/mpack-common.c:583
unclosed mpack_type_map
Valgrind will print the abort and may also print leaks since it doesn't clean up when it aborts.
You can build a new amalgamation package from the latest source with tools/amalgamate.sh
. I might do a point release at some point but I'm not in a rush to do so because this bug can only occur due to another bug in the calling code.
from mpack.
I might do a point release at some point but I'm not in a rush to do so because this bug can only occur due to another bug in the calling code.
Thanks for fixing this. I'd just push back a bit on this description. IIUC, an error can occur doing the serialization (e.g. an I/O error), that will short-circuit mpack_complete_map
even if it's subsequently called and in that case the call to destroy
would still issue an invalid free
.
from mpack.
Hmm. You're partially right. An error can't happen from I/O; it can only happen from a malloc failure. When a builder is open all writes are diverted to a buffer because we can't output anything until we figure out the size of the container being built. If a malloc does fail though, it will indeed flag an error and short circuit subsequent calls to mpack_complete_map()
, leaking the builder and causing the growable writer to crash. I am intending to fully support malloc failures so this is a serious bug; I'll need to make a patch release soon.
There is another bug that can occur though if the caller is using longjmp for error handling. The I/O happens in mpack_complete_map()
(via mpack_builder_resolve()
) when closing the final container. This can flag an error. Nothing in mpack_builder_resolve()
is short circuited on an error though, so it will still walk through all of its builder pages, attempt to write them (ignored due to error), then free them... unless the error handler longjmps out. This is also something I am intending to support properly.
I'm not sure of the proper fix here yet, but most likely I will just defer the callback until after the writer has walked all its pages. The reader does something similar when reading into an allocation.
In any case I am reopening this issue. I need to fix this longjmp bug, plus I need a lot more unit tests to make sure these bugs don't crop up again. At the very least the builder needs tests to make sure it handles any malloc failure properly.
from mpack.
Related Issues (20)
- Stream reader with manual memory management? HOT 2
- comparison with libmpack HOT 1
- Warning about incompatible int types in mpack_snprintf calls HOT 1
- ESP32-C3 rebooting when executing functions from the MPack library
- Stream parser that doesn't buffer the entire message
- Usage in Linux kernel modules HOT 8
- How to use the protocol on both client side and server side HOT 1
- Copy element from node API to write API HOT 1
- New release? HOT 3
- Suggestions for warnings? HOT 3
- Node API for read+write? HOT 1
- How to change the data with node API? Or how to write fast when I know the data offset or giving node? HOT 2
- get nothing if prev node is nil HOT 3
- Make it possible to use a dynamic buffer with mpack writer HOT 2
- builder page allocation leak and segfault HOT 6
- Missing Symbolic Link HOT 1
- Should NULL data be allowed for mpack_write_str() with count of 0? HOT 1
- Error handler segfault HOT 1
- How can I get it header only? HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from mpack.