Avoid signal-unsafe operations in POSIX signal handler (#16160)

src/client/clientlauncher.cpp

@@ -147,8 +147,8 @@ bool ClientLauncher::run(GameStartData &start_data, const Settings &cmd_args)
 	/*
 		Menu-game loop
 	*/
-	bool retval = true;
-	bool *kill = porting::signal_handler_killstatus();
+	bool retval         = true;
+	volatile auto *kill = porting::signal_handler_killstatus();
 
 	while (m_rendering_engine->run() && !*kill &&
 		!g_gamecallback->shutdown_requested) {
@@ -540,9 +540,9 @@ bool ClientLauncher::launch_game(std::string &error_message,
 
 void ClientLauncher::main_menu(MainMenuData *menudata)
 {
-	bool *kill = porting::signal_handler_killstatus();
+	volatile auto       *kill   = porting::signal_handler_killstatus();
 	video::IVideoDriver *driver = m_rendering_engine->get_video_driver();
-	auto *device = m_rendering_engine->get_raw_device();
+	auto                *device = m_rendering_engine->get_raw_device();
 
 	// Wait until app is in foreground because of #15883
 	infostream << "Waiting for app to be in foreground" << std::endl;

src/client/game.cpp

@@ -65,6 +65,8 @@
 	#include "client/sound/sound_openal.h"
 #endif
 
+#include <csignal>
+
 class NodeDugEvent : public MtEvent
 {
 public:
@@ -561,7 +563,7 @@ public:
 	Game();
 	~Game();
 
-	bool startup(bool *kill,
+	bool startup(volatile std::sig_atomic_t *kill,
 			InputHandler *input,
 			RenderingEngine *rendering_engine,
 			const GameStartData &game_params,
@@ -793,14 +795,14 @@ private:
 	   This class does take ownership/responsibily for cleaning up etc of any of
 	   these items (e.g. device)
 	*/
-	IrrlichtDevice *device;
-	RenderingEngine *m_rendering_engine;
-	video::IVideoDriver *driver;
-	scene::ISceneManager *smgr;
-	bool *kill;
-	std::string *error_message;
-	bool *reconnect_requested;
-	PausedNodesList paused_animated_nodes;
+	IrrlichtDevice             *device;
+	RenderingEngine            *m_rendering_engine;
+	video::IVideoDriver        *driver;
+	scene::ISceneManager       *smgr;
+	volatile std::sig_atomic_t *kill;
+	std::string                *error_message;
+	bool                       *reconnect_requested;
+	PausedNodesList             paused_animated_nodes;
 
 	bool simple_singleplayer_mode;
 	/* End 'cache' */
@@ -932,7 +934,7 @@ Game::~Game()
 		m_rendering_engine->finalize();
 }
 
-bool Game::startup(bool *kill,
+bool Game::startup(volatile std::sig_atomic_t *kill,
 		InputHandler *input,
 		RenderingEngine *rendering_engine,
 		const GameStartData &start_data,
@@ -4235,7 +4237,7 @@ void Game::readSettings()
  ****************************************************************************/
 /****************************************************************************/
 
-void the_game(bool *kill,
+void the_game(volatile std::sig_atomic_t *kill,
 		InputHandler *input,
 		RenderingEngine *rendering_engine,
 		const GameStartData &start_data,

src/client/game.h

@@ -6,6 +6,7 @@
 
 #include "irrlichttypes.h"
 #include "config.h"
+#include <csignal>
 #include <string>
 
 #if !IS_CLIENT_BUILD
@@ -36,7 +37,7 @@ struct CameraOrientation {
 #define GAME_FALLBACK_TIMEOUT 1.8f
 #define GAME_CONNECTION_TIMEOUT 10.0f
 
-void the_game(bool *kill,
+void the_game(volatile std::sig_atomic_t *kill,
 		InputHandler *input,
 		RenderingEngine *rendering_engine,
 		const GameStartData &start_data,

src/gui/guiEngine.cpp

@@ -33,6 +33,8 @@
 	#include "client/sound/sound_openal.h"
 #endif
 
+#include <csignal>
+
 
 /******************************************************************************/
 void TextDestGuiEngine::gotText(const StringMap &fields)
@@ -104,7 +106,7 @@ GUIEngine::GUIEngine(JoystickController *joystick,
 		RenderingEngine *rendering_engine,
 		IMenuManager *menumgr,
 		MainMenuData *data,
-		bool &kill) :
+		volatile std::sig_atomic_t &kill) :
 	m_rendering_engine(rendering_engine),
 	m_parent(parent),
 	m_menumanager(menumgr),

src/gui/guiEngine.h

@@ -14,6 +14,8 @@
 #include "util/enriched_string.h"
 #include "translation.h"
 
+#include <csignal>
+
 /******************************************************************************/
 /* Structs and macros                                                         */
 /******************************************************************************/
@@ -124,7 +126,7 @@ public:
 			RenderingEngine *rendering_engine,
 			IMenuManager *menumgr,
 			MainMenuData *data,
-			bool &kill);
+			volatile std::sig_atomic_t &kill);
 
 	/** default destructor */
 	virtual ~GUIEngine();
@@ -193,7 +195,7 @@ private:
 	irr_ptr<GUIFormSpecMenu>              m_menu;
 
 	/** reference to kill variable managed by SIGINT handler */
-	bool                                 &m_kill;
+	volatile std::sig_atomic_t           &m_kill;
 
 	/** variable used to abort menu and return back to main game handling */
 	bool                                  m_startgame = false;

src/main.cpp

@@ -1138,7 +1138,7 @@ static bool run_dedicated_server(const GameParams &game_params, const Settings &
 			return false;
 		}
 		ChatInterface iface;
-		bool &kill = *porting::signal_handler_killstatus();
+		volatile auto &kill = *porting::signal_handler_killstatus();
 
 		try {
 			// Create server
@@ -1181,7 +1181,7 @@ static bool run_dedicated_server(const GameParams &game_params, const Settings &
 			server.start();
 
 			// Run server
-			bool &kill = *porting::signal_handler_killstatus();
+			volatile auto &kill = *porting::signal_handler_killstatus();
 			dedicated_server_loop(server, kill);
 
 		} catch (const ModError &e) {
@@ -1226,7 +1226,7 @@ static bool migrate_map_database(const GameParams &game_params, const Settings &
 
 	u32 count = 0;
 	u64 last_update_time = 0;
-	bool &kill = *porting::signal_handler_killstatus();
+	volatile auto &kill = *porting::signal_handler_killstatus();
 
 	std::vector<v3s16> blocks;
 	old_db->listAllLoadableBlocks(blocks);
@@ -1280,7 +1280,7 @@ static bool recompress_map_database(const GameParams &game_params, const Setting
 
 	u32 count = 0;
 	u64 last_update_time = 0;
-	bool &kill = *porting::signal_handler_killstatus();
+	volatile auto &kill = *porting::signal_handler_killstatus();
 	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);
 

src/porting.cpp

@@ -60,6 +60,7 @@
 #include "util/string.h"
 #include "util/tracy_wrapper.h"
 #include <vector>
+#include <csignal>
 #include <cstdarg>
 #include <cstdio>
 #include <signal.h>
@@ -81,31 +82,28 @@ namespace porting
 	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;
 }
 
 #if !defined(_WIN32) // POSIX
+#define STDERR_FILENO 2
 
 static void signal_handler(int sig)
 {
 	if (!g_killed) {
 		if (sig == SIGINT) {
-			dstream << "INFO: signal_handler(): "
-				<< "Ctrl-C pressed, shutting down." << std::endl;
+			const char *dbg_text{"INFO: signal_handler(): "
+				"Ctrl-C pressed, shutting down.\n"};
+			write(STDERR_FILENO, dbg_text, strlen(dbg_text));
 		} else if (sig == SIGTERM) {
-			dstream << "INFO: signal_handler(): "
-				<< "got SIGTERM, shutting down." << std::endl;
+			const char *dbg_text{"INFO: signal_handler(): "
+				"got SIGTERM, shutting down.\n"};
+			write(STDERR_FILENO, dbg_text, strlen(dbg_text));
 		}
-
-		// Comment out for less clutter when testing scripts
-		/*dstream << "INFO: sigint_handler(): "
-				<< "Printing debug stacks" << std::endl;
-		debug_stacks_print();*/
-
 		g_killed = true;
 	} else {
 		(void)signal(sig, SIG_DFL);

src/porting.h

@@ -13,6 +13,7 @@
 #endif
 
 // Be mindful of what you include here!
+#include <csignal>
 #include <string>
 #include "config.h"
 #include "irrlichttypes.h" // u64
@@ -77,7 +78,7 @@ namespace porting
 void signal_handler_init();
 // Returns a pointer to a bool.
 // 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.

src/server.cpp

@@ -65,6 +65,8 @@
 #include "gettext.h"
 #include "util/tracy_wrapper.h"
 
+#include <csignal>
+
 class ClientNotFoundException : public BaseException
 {
 public:
@@ -4114,7 +4116,7 @@ std::unique_ptr<PlayerSAO> Server::emergePlayer(const char *name, session_t peer
 	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;
 

src/server.h

@@ -25,6 +25,7 @@
 #include "translation.h"
 #include "script/common/c_types.h" // LuaError
 #include <atomic>
+#include <csignal>
 #include <string>
 #include <list>
 #include <map>
@@ -799,4 +800,4 @@ private:
 
 	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);

src/terminal_chat_console.h

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