Skip to content
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

Mostly a refactor of tests #513

Merged
merged 15 commits into from
Dec 6, 2023
Prev Previous commit
Next Next commit
updated with improved POSIX fatal handling
  • Loading branch information
KjellKod committed Dec 6, 2023
commit ce22cde482f2abe5680cb757960811edbbbba341
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"
}
}
30 changes: 26 additions & 4 deletions 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,10 +298,8 @@ namespace g3 {

if (sigaction(signal_number, &(old_action_it->second), nullptr) < 0) {
auto signalname = std::string("sigaction - ") + signalToStr(signal_number);
// https://man7.org/linux/man-pages/man7/signal-safety.7.html (see signal-safety)
// perror is not async-signal-safe
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: 17 additions & 11 deletions src/crashhandler_windows.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,12 @@ 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.
void writeErrorMessage(const char* message) {
perror(message)
KjellKod marked this conversation as resolved.
Show resolved Hide resolved
}

// Restore back to default fatal event handling
void restoreFatalHandlingToDefault() {
#if !(defined(DISABLE_FATAL_SIGNALHANDLING))
Expand All @@ -181,19 +187,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 @@ -207,20 +213,20 @@ namespace g3 {
/// on Windows. This is automatically done if you do at least one LOG(...) call
/// you can also use this function call, per thread so make sure these three
/// fatal signals are covered in your thread (even if you don't do a LOG(...) call
void installSignalHandlerForThread() {
size_t installSignalHandlerForThread() {
#if !(defined(DISABLE_FATAL_SIGNALHANDLING))
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::writeErrorMessag("signal - SIGILL");
}
#endif
}
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 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_fatal 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