From 52e5b513ed9dc143c967c733423fe751e1b663d1 Mon Sep 17 00:00:00 2001
From: kwolekr <kwolekr@minetest.net>
Date: Sat, 31 Oct 2015 16:31:43 -0400
Subject: Fix Lua scripting synchronization

For several years now, the lua script lock has been completely broken.
This commit fixes the main issue (creation of a temporary rather than
scoped object), and fixes a subsequent deadlock issue caused by
nested script API calls by adding support for recursive mutexes.
---
 src/script/cpp_api/s_base.cpp   | 10 ++++++++--
 src/script/cpp_api/s_base.h     |  4 +++-
 src/script/cpp_api/s_internal.h | 42 +++++++++++++++++++++++++++++++----------
 3 files changed, 43 insertions(+), 13 deletions(-)

(limited to 'src/script')

diff --git a/src/script/cpp_api/s_base.cpp b/src/script/cpp_api/s_base.cpp
index b40d31533..71369e3d7 100644
--- a/src/script/cpp_api/s_base.cpp
+++ b/src/script/cpp_api/s_base.cpp
@@ -67,10 +67,11 @@ public:
 	ScriptApiBase
 */
 
-ScriptApiBase::ScriptApiBase()
+ScriptApiBase::ScriptApiBase() :
+	m_luastackmutex(true)
 {
 #ifdef SCRIPTAPI_LOCK_DEBUG
-	m_locked = false;
+	m_lock_recursion_count = 0;
 #endif
 
 	m_luastack = luaL_newstate();
@@ -157,9 +158,14 @@ void ScriptApiBase::loadScript(const std::string &script_path)
 // - runs the callbacks
 // - replaces the table and arguments with the return value,
 //     computed depending on mode
+// This function must only be called with scriptlock held (i.e. inside of a
+// code block with SCRIPTAPI_PRECHECKHEADER declared)
 void ScriptApiBase::runCallbacksRaw(int nargs,
 		RunCallbacksMode mode, const char *fxn)
 {
+#ifdef SCRIPTAPI_LOCK_DEBUG
+	assert(m_lock_recursion_count > 0);
+#endif
 	lua_State *L = getStack();
 	FATAL_ERROR_IF(lua_gettop(L) < nargs + 1, "Not enough arguments");
 
diff --git a/src/script/cpp_api/s_base.h b/src/script/cpp_api/s_base.h
index 20f4bc11b..d30373ce1 100644
--- a/src/script/cpp_api/s_base.h
+++ b/src/script/cpp_api/s_base.h
@@ -28,6 +28,7 @@ extern "C" {
 }
 
 #include "irrlichttypes.h"
+#include "threads.h"
 #include "threading/mutex.h"
 #include "threading/mutex_auto_lock.h"
 #include "common/c_types.h"
@@ -111,7 +112,8 @@ protected:
 	std::string     m_last_run_mod;
 	bool            m_secure;
 #ifdef SCRIPTAPI_LOCK_DEBUG
-	bool            m_locked;
+	int             m_lock_recursion_count;
+	threadid_t      m_owning_thread;
 #endif
 
 private:
diff --git a/src/script/cpp_api/s_internal.h b/src/script/cpp_api/s_internal.h
index 7daf8c03f..651fed95f 100644
--- a/src/script/cpp_api/s_internal.h
+++ b/src/script/cpp_api/s_internal.h
@@ -32,28 +32,50 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 
 #ifdef SCRIPTAPI_LOCK_DEBUG
 #include "debug.h" // assert()
+
 class LockChecker {
 public:
-	LockChecker(bool *variable) {
-		assert(*variable == false);
+	LockChecker(int *recursion_counter, threadid_t *owning_thread)
+	{
+		m_lock_recursion_counter = recursion_counter;
+		m_owning_thread          = owning_thread;
+		m_original_level         = *recursion_counter;
+
+		if (*m_lock_recursion_counter > 0)
+			assert(thr_is_current_thread(*m_owning_thread));
+		else
+			*m_owning_thread = thr_get_current_thread_id();
 
-		m_variable = variable;
-		*m_variable = true;
+		(*m_lock_recursion_counter)++;
 	}
-	~LockChecker() {
-		*m_variable = false;
+
+	~LockChecker()
+	{
+		assert(thr_is_current_thread(*m_owning_thread));
+		assert(*m_lock_recursion_counter > 0);
+
+		(*m_lock_recursion_counter)--;
+
+		assert(*m_lock_recursion_counter == m_original_level);
 	}
+
 private:
-	bool *m_variable;
+	int *m_lock_recursion_counter;
+	int m_original_level;
+	threadid_t *m_owning_thread;
 };
 
-#define SCRIPTAPI_LOCK_CHECK LockChecker(&(this->m_locked))
+#define SCRIPTAPI_LOCK_CHECK           \
+	LockChecker scriptlock_checker(    \
+		&this->m_lock_recursion_count, \
+		&this->m_owning_thread)
+
 #else
-#define SCRIPTAPI_LOCK_CHECK while(0)
+	#define SCRIPTAPI_LOCK_CHECK while(0)
 #endif
 
 #define SCRIPTAPI_PRECHECKHEADER                                               \
-		MutexAutoLock(this->m_luastackmutex);                                 \
+		MutexAutoLock scriptlock(this->m_luastackmutex);                       \
 		SCRIPTAPI_LOCK_CHECK;                                                  \
 		realityCheck();                                                        \
 		lua_State *L = getStack();                                             \
-- 
cgit v1.2.3