Skip to content

Commit

Permalink
Do not use localtime in our spdlog usage
Browse files Browse the repository at this point in the history
This flag is `true` by default and makes the spdlog library to call
[`localtime_r()`](https://github.com/gabime/spdlog/blob/v1.15.0/include/spdlog/pattern_formatter-inl.h#L1003), which is not safe to call within a signal handler.

In Fedora and RHEL we are getting a `SIGSEGV` in the `signals_test`
testsuite because of that. In Ubuntu the tests pass but it is undefined
behavior.

Setting it to `false` fixes the issue and has no side effects. We are
[ignoring the `std::tm` argument in the `custom_flag_formatter` sub
classes](https://github.com/envoyproxy/envoy/blob/v1.32.1/source/common/common/logger.cc#L430).

References:
- https://sourceware.org/bugzilla/show_bug.cgi?id=1390#c4
- https://man7.org/linux/man-pages/man7/signal-safety.7.html

Signed-off-by: Jonh Wendell <[email protected]>
  • Loading branch information
jwendell committed Nov 14, 2024
1 parent 3b293f1 commit 4b982b8
Showing 1 changed file with 1 addition and 0 deletions.
1 change: 1 addition & 0 deletions source/common/common/logger.cc
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,7 @@ void setLogFormatForLogger(spdlog::logger& logger, const std::string& log_format
CustomFlagFormatter::ExtractedMessage::Placeholder)
.set_pattern(log_format);

formatter->need_localtime(false);
logger.set_formatter(std::move(formatter));
}

Expand Down

0 comments on commit 4b982b8

Please sign in to comment.