-
-
Notifications
You must be signed in to change notification settings - Fork 143
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Segfault in Handlebars #179
Comments
Hi @matthiasbeyer , from what I can see in the dump, it might be your data structure has recursive reference so when serde_json tries to convert it to
Let me look into the code for further information. |
By the way, I was using |
Can you point out where my code seems to have a recursive reference? |
@matthiasbeyer Just added some debug print to your
When I comment out handlebars rendering code like: fn log(&self, record: &LogRecord) {
println!("entering log");
let mut data = BTreeMap::new();
{
data.insert("level", format!("{}", record.level()));
data.insert("module_path", String::from(record.location().module_path()));
data.insert("file", String::from(record.location().file()));
data.insert("line", format!("{}", record.location().line()));
data.insert("target", String::from(record.target()));
data.insert("message", format!("{}", record.args()));
}
println!("------");
println!("{:?}", data);
// let logtext = self
// .handlebars
// .render(&format!("{}", record.level()), &data)
// .unwrap_or_else(|e| format!("Failed rendering logging data: {:?}\n", e));
// self.module_settings
// .get(record.target())
// .map(|module_setting| {
// let set = module_setting.enabled &&
// module_setting.level.unwrap_or(self.global_loglevel) >= record.level();
// if set {
// let _ = write!(stderr(), "{}", logtext);
// }
// })
// .unwrap_or_else(|| {
// if self.global_loglevel >= record.level() {
// // Yes, we log
// let _ = write!(stderr(), "{}", logtext);
// }
// });
} It seems correct:
So it has something to do with line 641 where I have a debug log I can remove it from handlebars. But I guess this might be an issue you may be interested in. |
Let me know if you can fix it from your code. Otherwise I will publish an update with the debug log removed. |
Thanks for your investigation. I do not see, though, where the error is. Can you explain, please? |
It seems your logger is logging |
Ah, you mean that handlebars uses logging and because of that my logger recurses. Now I understand the problem. Well, then it'd be good if handlebars could add a compiletime feature to disable logging, I guess. Feel free to borrow my solution on this problem from the toml-query crate where I create macros if logging is disabled. |
Maybe we could even author a new crate for exactly this use-case... if there isn't one already! |
@matthiasbeyer by designing the logging API a macro, I believe the author is to eliminate runtime overhead for logging at all if you didn't enable it. So it seems redundant to add another feature toggle for it. I think you should fix it from your codebase or use your own logging macros as you said. |
Sorry, but I don't understand this statement at all.
Then I would need to fork handlebars and maintain a patch on top of my own fork, using my own fork in my imag codebase. Having it upstream in handlebars would eliminate this "maintain overhead" and others can profit from it, too, by disable logging in handlebars at compiletime. |
Ok I'll remove the debug log and release a new version tonight |
I added a workaround to my codebase here but that's not a thing one should need to do when using the handlebars crate in a logger. Thanks for your efforts! 👍 |
Fix released in 0.29.1 |
I just experienced a segfault which seems to be originated in the handlebars code.
As there are ~6000 stack levels, it seems that I ended up crashing into the heap by recursing too deep. The recursion seems to happen in handlebars, as only a few (as in below 10) stack levels are from my code.
The corrosponding code is here, here is the causing call.
The templates can be found here, the registration of the templates and the helpers is here. If you have any questions regarding the code, feel free to ask me, I know the codebase is pretty big and maybe confusing for some people.
To reproduce:
cargo build --manifest-path bin/core/imag-store/Cargo.toml
# takes some timemkdir /tmp/store/
./target/debug/imag-store --rtp /tmp/ --config imagrc.toml create -p test entry -c foo
The last few lines of the stacktrace follow
The text was updated successfully, but these errors were encountered: