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
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
Loading