Signal handling
m-pilia opened this issue · 5 comments
I was considering to implement a signal handler for our stack tracer that could be used like this, and thinking about it rang a bell in my mind. So I checked how it is implemented in the library, and indeed it is not AS safe, I think it may create a deadlock if a signal is risen within a malloc. Indeed, at best of my knowledge, I think it is impossible to print anything safely from within a signal handler.
Should we remove the signal handler from our code, or can we accept the risk? In the second case, I would add an additional CMake option to toggle the signal handler, which should be used only for debugging purpose.
Hmm, tricky. I saw some solutions where they checked whether malloc is a part of the callstack or not before printing. Another solution is to make sure to only use static memory for the logging of stacktraces. However, both are quite tricky to get right I think. For static logging we would had to make some custom stream object I think.
They know about the issue but that was 1 year ago so we probably shouldn't wait for them to finish it. Maybe having a option to enable it would make sense.
An additional problem, at least with the current implementation, is that functions like __cxa_demangle and backtrace_symbols are not AS safe as well, so we would need to change the implementation of the stack tracer too.
Another solution is to make sure to only use static memory for the logging of stacktraces.
To be truly sure we do not have undefined behaviour lurking around, the solution would be to only call functions that are marked as AS-safe in the POSIX standard. Which is even more limiting.
I saw some solutions where they checked whether malloc is a part of the callstack or not before printing
I was thinking, that may not even be enough in a multi-threaded application. I think it depends upon the implementation of the stdlib whether allocation is concurrent or not.
Seems like a lot of trouble, but given that it's quite useful to have I guess some opt-in thing makes sense.
I agree. I am adding a separate opt-in in CMake, and I will try to add also a custom handler for our tracer (as safe-ish as possible).