Store jailbreak exceptions in constructors, not in LUAI_TRY()

LUAI_TRY() is an internal part of Lua that may not exist forever, and
reliance on overriding it prevents the use of system copies of Lua.

Document in lua_jailbreak_exception this requirement to call
this->store() in derived class constructors.

Also, count the luaW_pcall_internal() recursion depth and store and
rethrow jailbreak exceptions until the recursion depth reaches 0,
because:

 1. luaW_pcall_internal() sometimes runs recursively (C++ calls Lua
    calls C++ calls Lua calls C++), so the middle C++ layer needs to
    rethrow exceptions instead of clearing them.  LUAI_TRY() previously
    stored them each time, but now lua_jailbreak_exception::rethrow()
    needs to know when not to clear().

 2. Jailbreak exceptions can be thrown while no Lua code is running.
    Now that constructors store() all exceptions instead of LUAI_TRY()
    storing only those caught by Lua, lua_jailbreak_exception::store()
    needs to know when not to store them.  Otherwise, for example,
    leaving a game to return to the menu while no Lua code is running
    throws and needlessly stores an exception that isn't cleared, with
    two possible effects:

     a. If another jailbreak exception is thrown and the constructor
        tries to store it, we abort from assert() because one is already
        stored.

     b. Otherwise, if luaW_pcall_internal() runs without a new jailbreak
        exception being thrown, the stored one is rethrown and handled a
        second time (e.g. immediately leaving a new game).

changelog_entries/system_lua.md

@@ -2,3 +2,4 @@
    * Fix delayed handling of Lua jailbreak exceptions (quit to menu or desktop, wesnothd connection errors, etc.) thrown during Lua `pcall()` and `xpcall()`, which could accumulate and cause wesnoth to abort. (PR #8234)
      * This bug has existed since jailbreak exceptions were introduced in version 1.9.5.
    * Fix strict mode not catching Lua errors without exception strings (bug since 1.9.5) or with exception strings containing "~lua:" (bug since 1.9.0). (PR #8234)
+   * Store Lua jailbreak exceptions in derived class constructors. This reduces the reliance on `LUAI_TRY()`, an internal part of Lua that may not exist forever. (PR #8234)

src/game_end_exceptions.hpp

@@ -42,6 +42,7 @@ public:
 		: lua_jailbreak_exception()
 		, std::exception()
 	{
+		this->store();
 	}
 	const char * what() const noexcept { return "return_to_play_side_exception"; }
 private:
@@ -59,6 +60,7 @@ public:
 		: lua_jailbreak_exception()
 		, std::exception()
 	{
+		this->store();
 	}
 	const char * what() const noexcept { return "quit_game_exception"; }
 private:

src/gui/auxiliary/iterator/exception.hpp

@@ -40,6 +40,7 @@ public:
 		: std::logic_error("GUI2 ITERATOR: " + message)
 		, lua_jailbreak_exception()
 	{
+		this->store();
 	}
 
 private:
@@ -58,6 +59,7 @@ public:
 		: std::range_error("GUI2 ITERATOR: " + message)
 		, lua_jailbreak_exception()
 	{
+		this->store();
 	}
 
 private:

src/image_modifications.cpp

@@ -120,11 +120,13 @@ std::unique_ptr<modification> decode_modification(const std::string& encoded_mod
 modification::imod_exception::imod_exception(const std::stringstream& message_stream)
 	: message(message_stream.str())
 {
+	this->store();
 }
 
 modification::imod_exception::imod_exception(const std::string& message)
 	: message(message)
 {
+	this->store();
 }
 
 /** Decodes the modification string

src/lua_jailbreak_exception.cpp

@@ -17,6 +17,7 @@
 
 #include <cassert>
 
+int lua_jailbreak_exception::jail_depth = 0;
 lua_jailbreak_exception *lua_jailbreak_exception::jailbreak_exception = nullptr;
 
 void lua_jailbreak_exception::store() const noexcept
@@ -27,6 +28,9 @@ void lua_jailbreak_exception::store() const noexcept
 	 * lua_jailbreak_exception::rethrow() or a logic error in the code.
 	 */
 	assert(!jailbreak_exception);
+	if (jail_depth == 0) {
+		return;
+	}
 
 	jailbreak_exception = this->clone();
 }
@@ -47,7 +51,9 @@ void lua_jailbreak_exception::rethrow()
 	try {
 		jailbreak_exception->execute();
 	} catch(...) {
-		clear();
+		if (jail_depth == 0) {
+			clear();
+		}
 		throw;
 	}
 

src/lua_jailbreak_exception.hpp

@@ -18,14 +18,19 @@
 /**
  * Base class for exceptions that want to be thrown 'through' lua.
  *
- * Classes inheriting from this class need to use the @ref
- * IMPLEMENT_LUA_JAILBREAK_EXCEPTION macro in the class definition.
+ * Classes derived from this class <b>must call `this->store()` in all of their
+ * constructors</b> and use the @ref IMPLEMENT_LUA_JAILBREAK_EXCEPTION macro
+ * in the class definition. No other classes may derive from classes derived
+ * from this class.
  */
 class lua_jailbreak_exception
 {
 public:
 	virtual ~lua_jailbreak_exception() noexcept {}
 
+	/** Depth of recursive luaW_pcall_internal() function calls. */
+	static int jail_depth;
+
 	/** Stores a copy the current exception to be rethrown. */
 	void store() const noexcept;
 

src/menu_events.hpp

@@ -40,6 +40,11 @@ class mouse_handler;
 
 struct fallback_ai_to_human_exception final : public lua_jailbreak_exception
 {
+	fallback_ai_to_human_exception() : lua_jailbreak_exception()
+	{
+		this->store();
+	}
+
 	IMPLEMENT_LUA_JAILBREAK_EXCEPTION(fallback_ai_to_human_exception)
 };
 

src/playsingle_controller.hpp

@@ -31,7 +31,10 @@ class saved_game;
 
 struct reset_gamestate_exception final : public lua_jailbreak_exception, public std::exception
 {
-	reset_gamestate_exception(std::shared_ptr<config> l, std::shared_ptr<config> stats, bool s = true) : level(l), stats_(stats), start_replay(s) {}
+	reset_gamestate_exception(std::shared_ptr<config> l, std::shared_ptr<config> stats, bool s = true) : level(l), stats_(stats), start_replay(s)
+	{
+		this->store();
+	}
 	std::shared_ptr<config> level;
 	std::shared_ptr<config> stats_;
 	bool start_replay;

src/savegame.hpp

@@ -88,6 +88,7 @@ public:
 		: lua_jailbreak_exception()
 		, data_(data)
 	{
+		this->store();
 	}
 	load_game_metadata data_;
 private:

src/scripting/lua_common.cpp

@@ -1116,9 +1116,12 @@ int luaW_pcall_internal(lua_State *L, int nArgs, int nRets)
 
 	int error_handler_index = lua_gettop(L) - nArgs - 1;
 
+	++lua_jailbreak_exception::jail_depth;
+
 	// Call the function.
 	int errcode = lua_pcall(L, nArgs, nRets, -2 - nArgs);
 
+	--lua_jailbreak_exception::jail_depth;
 	lua_jailbreak_exception::rethrow();
 
 	// Remove the error handler.

src/video.hpp

@@ -320,6 +320,7 @@ public:
 	quit()
 		: lua_jailbreak_exception()
 	{
+		this->store();
 	}
 
 private:

src/wesnoth_lua_config.h

@@ -56,12 +56,8 @@
 #include <string.h>
 #define strcoll(a,b) strcmp(a,b)
 
-/*  We need to rethrow exceptions.
- *
- *  The stock Lua catch(...) consumes the exception. We need to re-throw
- *  it so. This allows the inner function (in C++ -> Lua -> C++) to pass
- *  back information about the exception, instead of reclassifying all
- *  exceptions to a single Lua status code.
+/*  Push std::exception::what() strings onto the Lua stack for use by
+ *  luaW_pcall().
  */
 
 #include <cassert>
@@ -75,8 +71,7 @@
 	try { \
 		try { \
 			a \
-		} catch(const lua_jailbreak_exception &e) { \
-			e.store(); \
+		} catch(const lua_jailbreak_exception &) { \
 			throw; \
 		} catch(const std::exception &e) { \
 			lua_pushstring(L, e.what()); \

src/wesnothd_connection_error.hpp

@@ -51,6 +51,7 @@ struct ingame_wesnothd_error final : public wesnothd_error, public lua_jailbreak
 	ingame_wesnothd_error(const std::string& error)
 		: wesnothd_error(error)
 	{
+		this->store();
 	}
 
 	IMPLEMENT_LUA_JAILBREAK_EXCEPTION(ingame_wesnothd_error)
@@ -61,6 +62,7 @@ struct leavegame_wesnothd_error final : public wesnothd_error, public lua_jailbr
 	leavegame_wesnothd_error(const std::string& error)
 		: wesnothd_error(error)
 	{
+		this->store();
 	}
 
 	IMPLEMENT_LUA_JAILBREAK_EXCEPTION(leavegame_wesnothd_error)
@@ -76,6 +78,7 @@ struct wesnothd_connection_error final : public wesnothd_error, public lua_jailb
 		: wesnothd_error(error.message())
 		, user_message(msg)
 	{
+		this->store();
 	}
 
 	/** User-friendly and potentially translated message for use in the UI. */

src/wml_exception.hpp

@@ -103,6 +103,7 @@ struct wml_exception final
 		: user_message(user_msg)
 		, dev_message(dev_msg)
 	{
+		this->store();
 	}
 
 	~wml_exception() noexcept {}