Skip to content

Commit

Permalink
Mostly a refactor of tests (#513)
Browse files Browse the repository at this point in the history
  • Loading branch information
KjellKod authored Dec 6, 2023
1 parent 055f5e4 commit 626191a
Show file tree
Hide file tree
Showing 11 changed files with 467 additions and 329 deletions.
6 changes: 2 additions & 4 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,9 @@
"name": "(gdb) Start",
"type": "cppdbg",
"request": "launch",
"program": "${workspaceFolder}/build/g3log-FATAL-contract",
"program": "${workspaceFolder}/build/test_signal",
"MIMode": "gdb",
"cwd": "${workspaceFolder}/build"
"setupCommands": [
"cwd": "${workspaceFolder}/build""setupCommands": [
{
"description": "Enable pretty-printing for gdb",
"text": "-enable-pretty-printing",
Expand All @@ -28,6 +27,5 @@
}
]
}

]
}
7 changes: 5 additions & 2 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
{
"cmake.configureOnOpen": false,
"editor.formatOnSave": true
}
"editor.formatOnSave": true,
"files.associations": {
"ostream": "cpp"
}
}
27 changes: 26 additions & 1 deletion src/crashhandler_unix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,30 @@ namespace g3 {
exit(signal_number);
}

// This function is intended to be async-signal-safe. Using write and STDERR_FILENO should be
// safe in a signal handler. ref: http://pubs.opengroup.org/onlinepubs/009695399/functions/write.html

// This function is intended to be async-signal-safe.
// It writes an error message to stderr using the write system call,
// which is listed as async-signal-safe by POSIX.
size_t writeErrorMessage(const char* message) {
if (message == nullptr) {
return 0;
}

// Calculate the length of the message without using std library strlen or similar
// this is to ensure async-signal-safe by POSIX
size_t length = 0;
for (const char* p = message; *p != '\0'; ++p) {
++length;
}

// Write the message to STDERR_FILENO in a single call.
// This assumes that the message is not too large for a single write.
auto bytes_written = write(STDERR_FILENO, message, length);
return bytes_written;
}

// restores the signal handler back to default
void restoreFatalHandlingToDefault() {
#if !(defined(DISABLE_FATAL_SIGNALHANDLING))
Expand Down Expand Up @@ -274,7 +298,8 @@ namespace g3 {

if (sigaction(signal_number, &(old_action_it->second), nullptr) < 0) {
auto signalname = std::string("sigaction - ") + signalToStr(signal_number);
perror(signalname.c_str());
// https://man7.org/linux/man-pages/man7/signal-safety.7.html (see signal-safety)
internal::writeErrorMessage(signalname.c_str());
}

gSavedSigActions.erase(old_action_it);
Expand Down
28 changes: 18 additions & 10 deletions src/crashhandler_windows.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <windows.h>
#include <atomic>
#include <csignal>
#include <cstring>
#include <sstream>
#include "g3log/crashhandler.hpp"
#include "g3log/g3log.hpp"
Expand Down Expand Up @@ -171,6 +172,13 @@ namespace g3 {
raise(signal_number);
}

// FYI: Concept of async-signal-safe operations does not exist on windows
// we stick to perror for lack of better alternatives.
size_t writeErrorMessage(const char* message) {
perror(message);
return std::strlen(message);
}

// Restore back to default fatal event handling
void restoreFatalHandlingToDefault() {
#if !(defined(DISABLE_FATAL_SIGNALHANDLING))
Expand All @@ -181,19 +189,19 @@ namespace g3 {
#endif

if (SIG_ERR == signal(SIGABRT, SIG_DFL))
perror("signal - SIGABRT");
internal::writeErrorMessage("signal - SIGABRT");

if (SIG_ERR == signal(SIGFPE, SIG_DFL))
perror("signal - SIGABRT");
internal::writeErrorMessage("signal - SIGABRT");

if (SIG_ERR == signal(SIGSEGV, SIG_DFL))
perror("signal - SIGABRT");
internal::writeErrorMessage("signal - SIGABRT");

if (SIG_ERR == signal(SIGILL, SIG_DFL))
perror("signal - SIGABRT");
internal::writeErrorMessage("signal - SIGABRT");

if (SIG_ERR == signal(SIGTERM, SIG_DFL))
perror("signal - SIGABRT");
internal::writeErrorMessage("signal - SIGABRT");
#endif
}

Expand All @@ -212,15 +220,15 @@ namespace g3 {
if (!g_installed_thread_signal_handler) {
g_installed_thread_signal_handler = true;
if (SIG_ERR == signal(SIGTERM, signalHandler))
perror("signal - SIGTERM");
internal::writeErrorMessage("signal - SIGTERM");
if (SIG_ERR == signal(SIGABRT, signalHandler))
perror("signal - SIGABRT");
internal::writeErrorMessage("signal - SIGABRT");
if (SIG_ERR == signal(SIGFPE, signalHandler))
perror("signal - SIGFPE");
internal::writeErrorMessage("signal - SIGFPE");
if (SIG_ERR == signal(SIGSEGV, signalHandler))
perror("signal - SIGSEGV");
internal::writeErrorMessage("signal - SIGSEGV");
if (SIG_ERR == signal(SIGILL, signalHandler))
perror("signal - SIGILL");
internal::writeErrorMessage("signal - SIGILL");
}
#endif
}
Expand Down
4 changes: 2 additions & 2 deletions src/g3log.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,13 +148,13 @@ namespace g3 {
message.get()->setExpression(boolean_expression);

if (internal::wasFatal(level)) {
saveFatalMessage(level, stack_trace, message, fatal_signal);
saveFatalMessage(stack_trace, message, fatal_signal);
} else {
pushMessageToLogger(message);
}
}

void saveFatalMessage(const LEVELS& level, const char* stack_trace, g3::LogMessagePtr& message, int& fatal_signal) {
void saveFatalMessage(const char* stack_trace, g3::LogMessagePtr& message, int& fatal_signal) {
auto fatalhook = g_fatal_pre_logging_hook;
// In case the fatal_pre logging actually will cause a crash in its turn
// let's not do recursive crashing!
Expand Down
1 change: 1 addition & 0 deletions src/g3log/crashhandler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,5 +77,6 @@ namespace g3 {
* This is an internal only function. Do not use it elsewhere. It is triggered
* from g3log, g3LogWorker after flushing messages to file */
void exitWithDefaultSignalHandler(const LEVELS& level, g3::SignalType signal_number);
size_t writeErrorMessage(const char* message);
} // namespace internal
} // namespace g3
2 changes: 1 addition & 1 deletion src/g3log/g3log.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ namespace g3 {
void saveMessage(const char* message, const char* file, int line, const char* function, const LEVELS& level,
const char* boolean_expression, int fatal_signal, const char* stack_trace);

void saveFatalMessage(const LEVELS& level, const char* stack_trace, g3::LogMessagePtr& message, int& fatal_signal);
void saveFatalMessage(const char* stack_trace, g3::LogMessagePtr& message, int& fatal_signal);

// forwards the message to all sinks
void pushMessageToLogger(LogMessagePtr log_entry);
Expand Down
2 changes: 1 addition & 1 deletion test_unit/Test.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
SET(OS_SPECIFIC_TEST test_crashhandler_windows)
ENDIF(MSVC OR MINGW)

SET(tests_to_run test_message test_filechange test_io test_cpp_future_concepts test_concept_sink test_sink ${OS_SPECIFIC_TEST})
SET(tests_to_run test_message test_filechange test_io test_fatal test_signal test_cpp_future_concepts test_concept_sink test_sink ${OS_SPECIFIC_TEST})
SET(helper ${DIR_UNIT_TEST}/testing_helpers.h ${DIR_UNIT_TEST}/testing_helpers.cpp)
include_directories(${DIR_UNIT_TEST})

Expand Down
Loading

0 comments on commit 626191a

Please sign in to comment.