1
0
Fork 0
mirror of https://github.com/luanti-org/luanti.git synced 2025-09-15 18:57:08 +00:00

Avoid signal-unsafe operations in signal handler

TSan reported a signal-unsafe call to `ostream::put` in the POSIX signal
handler. This patch replaces the use of `dstream`, which generated those
calls, with calls to `write` and `strlen`, which are both async-signal
safe in POSIX.

This patch also changes the type of the global kill flag from `bool` to
`volatile std::sig_atomic_t`, to be certain there is no undefined
behavior. TSan did not mention that issue; I am not sure whether that is
something TSan is able to detect.
This commit is contained in:
Josiah VanderZee 2025-05-19 08:19:09 -05:00
parent 30e33d71cc
commit d053d49642
No known key found for this signature in database
GPG key ID: C7BB8573A4ABC4B9
6 changed files with 29 additions and 17 deletions

View file

@ -46,6 +46,8 @@ extern "C" {
#endif #endif
} }
#include <csignal>
#if !defined(__cpp_rtti) || !defined(__cpp_exceptions) #if !defined(__cpp_rtti) || !defined(__cpp_exceptions)
#error Luanti cannot be built without exceptions or RTTI #error Luanti cannot be built without exceptions or RTTI
#endif #endif
@ -1138,7 +1140,7 @@ static bool run_dedicated_server(const GameParams &game_params, const Settings &
return false; return false;
} }
ChatInterface iface; ChatInterface iface;
bool &kill = *porting::signal_handler_killstatus(); volatile std::sig_atomic_t &kill = *porting::signal_handler_killstatus();
try { try {
// Create server // Create server
@ -1181,7 +1183,7 @@ static bool run_dedicated_server(const GameParams &game_params, const Settings &
server.start(); server.start();
// Run server // Run server
bool &kill = *porting::signal_handler_killstatus(); volatile std::sig_atomic_t &kill = *porting::signal_handler_killstatus();
dedicated_server_loop(server, kill); dedicated_server_loop(server, kill);
} catch (const ModError &e) { } catch (const ModError &e) {
@ -1226,7 +1228,7 @@ static bool migrate_map_database(const GameParams &game_params, const Settings &
u32 count = 0; u32 count = 0;
u64 last_update_time = 0; u64 last_update_time = 0;
bool &kill = *porting::signal_handler_killstatus(); volatile std::sig_atomic_t &kill = *porting::signal_handler_killstatus();
std::vector<v3s16> blocks; std::vector<v3s16> blocks;
old_db->listAllLoadableBlocks(blocks); old_db->listAllLoadableBlocks(blocks);
@ -1280,7 +1282,7 @@ static bool recompress_map_database(const GameParams &game_params, const Setting
u32 count = 0; u32 count = 0;
u64 last_update_time = 0; u64 last_update_time = 0;
bool &kill = *porting::signal_handler_killstatus(); volatile std::sig_atomic_t &kill = *porting::signal_handler_killstatus();
const u8 serialize_as_ver = SER_FMT_VER_HIGHEST_WRITE; const u8 serialize_as_ver = SER_FMT_VER_HIGHEST_WRITE;
const s16 map_compression_level = rangelim(g_settings->getS16("map_compression_level_disk"), -1, 9); const s16 map_compression_level = rangelim(g_settings->getS16("map_compression_level_disk"), -1, 9);

View file

@ -60,6 +60,7 @@
#include "util/string.h" #include "util/string.h"
#include "util/tracy_wrapper.h" #include "util/tracy_wrapper.h"
#include <vector> #include <vector>
#include <csignal>
#include <cstdarg> #include <cstdarg>
#include <cstdio> #include <cstdio>
#include <signal.h> #include <signal.h>
@ -81,24 +82,27 @@ namespace porting
Signal handler (grabs Ctrl-C on POSIX systems) Signal handler (grabs Ctrl-C on POSIX systems)
*/ */
static bool g_killed = false; volatile static std::sig_atomic_t g_killed = false;
bool *signal_handler_killstatus() volatile std::sig_atomic_t *signal_handler_killstatus()
{ {
return &g_killed; return &g_killed;
} }
#if !defined(_WIN32) // POSIX #if !defined(_WIN32) // POSIX
#define STDOUT 1
static void signal_handler(int sig) static void signal_handler(int sig)
{ {
if (!g_killed) { if (!g_killed) {
if (sig == SIGINT) { if (sig == SIGINT) {
dstream << "INFO: signal_handler(): " const char *dbg_text{"INFO: signal_handler(): "
<< "Ctrl-C pressed, shutting down." << std::endl; "Ctrl-C pressed, shutting down.\n"};
write(STDOUT, dbg_text, strlen(dbg_text) + 1);
} else if (sig == SIGTERM) { } else if (sig == SIGTERM) {
dstream << "INFO: signal_handler(): " const char *dbg_text{"INFO: signal_handler(): "
<< "got SIGTERM, shutting down." << std::endl; "got SIGTERM, shutting down.\n"};
write(STDOUT, dbg_text, strlen(dbg_text) + 1);
} }
// Comment out for less clutter when testing scripts // Comment out for less clutter when testing scripts

View file

@ -13,6 +13,7 @@
#endif #endif
// Be mindful of what you include here! // Be mindful of what you include here!
#include <csignal>
#include <string> #include <string>
#include "config.h" #include "config.h"
#include "irrlichttypes.h" // u64 #include "irrlichttypes.h" // u64
@ -77,7 +78,7 @@ namespace porting
void signal_handler_init(); void signal_handler_init();
// Returns a pointer to a bool. // Returns a pointer to a bool.
// When the bool is true, program should quit. // When the bool is true, program should quit.
[[nodiscard]] bool *signal_handler_killstatus(); [[nodiscard]] volatile std::sig_atomic_t *signal_handler_killstatus();
/* /*
Path of static data directory. Path of static data directory.

View file

@ -65,6 +65,8 @@
#include "gettext.h" #include "gettext.h"
#include "util/tracy_wrapper.h" #include "util/tracy_wrapper.h"
#include <csignal>
class ClientNotFoundException : public BaseException class ClientNotFoundException : public BaseException
{ {
public: public:
@ -4105,7 +4107,7 @@ std::unique_ptr<PlayerSAO> Server::emergePlayer(const char *name, session_t peer
return playersao; return playersao;
} }
void dedicated_server_loop(Server &server, bool &kill) void dedicated_server_loop(Server &server, volatile std::sig_atomic_t &kill)
{ {
verbosestream<<"dedicated_server_loop()"<<std::endl; verbosestream<<"dedicated_server_loop()"<<std::endl;

View file

@ -25,6 +25,7 @@
#include "translation.h" #include "translation.h"
#include "script/common/c_types.h" // LuaError #include "script/common/c_types.h" // LuaError
#include <atomic> #include <atomic>
#include <csignal>
#include <string> #include <string>
#include <list> #include <list>
#include <map> #include <map>
@ -794,4 +795,4 @@ private:
Shuts down when kill is set to true. Shuts down when kill is set to true.
*/ */
void dedicated_server_loop(Server &server, bool &kill); void dedicated_server_loop(Server &server, volatile std::sig_atomic_t &kill);

View file

@ -9,6 +9,8 @@
#include "util/container.h" #include "util/container.h"
#include "log.h" #include "log.h"
#include "log_internal.h" #include "log_internal.h"
#include <csignal>
#include <set> #include <set>
#include <sstream> #include <sstream>
@ -45,7 +47,7 @@ public:
void setup( void setup(
ChatInterface *iface, ChatInterface *iface,
bool *kill_requested, volatile std::sig_atomic_t *kill_requested,
const std::string &nick) const std::string &nick)
{ {
m_nick = nick; m_nick = nick;
@ -96,9 +98,9 @@ private:
int m_rows; int m_rows;
bool m_can_draw_text; bool m_can_draw_text;
bool *m_kill_requested = nullptr; volatile std::sig_atomic_t *m_kill_requested = nullptr;
ChatBackend m_chat_backend; ChatBackend m_chat_backend;
ChatInterface *m_chat_interface; ChatInterface *m_chat_interface;
TermLogOutput m_log_output; TermLogOutput m_log_output;