Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Push error handler afresh each time lua_pcall is used
Fixes "double fault" / "error in error handling" messages
(issue #1423) and instead shows a complete backtrace.
  • Loading branch information
kahrl authored and est31 committed Aug 26, 2015
1 parent 8658c8d commit 3304e1e
Show file tree
Hide file tree
Showing 16 changed files with 188 additions and 92 deletions.
14 changes: 7 additions & 7 deletions src/script/common/c_internal.cpp
Expand Up @@ -136,28 +136,28 @@ void script_run_callbacks_f(lua_State *L, int nargs,
FATAL_ERROR_IF(lua_gettop(L) < nargs + 1, "Not enough arguments");

// Insert error handler
lua_pushcfunction(L, script_error_handler);
int errorhandler = lua_gettop(L) - nargs - 1;
lua_insert(L, errorhandler);
PUSH_ERROR_HANDLER(L);
int error_handler = lua_gettop(L) - nargs - 1;
lua_insert(L, error_handler);

// Insert run_callbacks between error handler and table
lua_getglobal(L, "core");
lua_getfield(L, -1, "run_callbacks");
lua_remove(L, -2);
lua_insert(L, errorhandler + 1);
lua_insert(L, error_handler + 1);

// Insert mode after table
lua_pushnumber(L, (int) mode);
lua_insert(L, errorhandler + 3);
lua_insert(L, error_handler + 3);

// Stack now looks like this:
// ... <error handler> <run_callbacks> <table> <mode> <arg#1> <arg#2> ... <arg#n>

int result = lua_pcall(L, nargs + 2, 1, errorhandler);
int result = lua_pcall(L, nargs + 2, 1, error_handler);
if (result != 0)
script_error(L, result, NULL, fxn);

lua_remove(L, -2); // Remove error handler
lua_remove(L, error_handler);
}

void log_deprecated(lua_State *L, const std::string &message)
Expand Down
4 changes: 4 additions & 0 deletions src/script/common/c_internal.h
Expand Up @@ -53,7 +53,11 @@ extern "C" {
#define CUSTOM_RIDX_SCRIPTAPI (CUSTOM_RIDX_BASE)
#define CUSTOM_RIDX_GLOBALS_BACKUP (CUSTOM_RIDX_BASE + 1)
#define CUSTOM_RIDX_CURRENT_MOD_NAME (CUSTOM_RIDX_BASE + 2)
#define CUSTOM_RIDX_ERROR_HANDLER (CUSTOM_RIDX_BASE + 3)

// Pushes the error handler onto the stack and returns its index
#define PUSH_ERROR_HANDLER(L) \
(lua_rawgeti((L), LUA_REGISTRYINDEX, CUSTOM_RIDX_ERROR_HANDLER), lua_gettop((L)))

#define PCALL_RESL(L, RES) do { \
int result_ = (RES); \
Expand Down
13 changes: 8 additions & 5 deletions src/script/cpp_api/s_async.cpp
Expand Up @@ -144,8 +144,9 @@ void AsyncEngine::putJobResult(LuaJobInfo result)
}

/******************************************************************************/
void AsyncEngine::step(lua_State *L, int errorhandler)
void AsyncEngine::step(lua_State *L)
{
int error_handler = PUSH_ERROR_HANDLER(L);
lua_getglobal(L, "core");
resultQueueMutex.lock();
while (!resultQueue.empty()) {
Expand All @@ -164,10 +165,10 @@ void AsyncEngine::step(lua_State *L, int errorhandler)
lua_pushlstring(L, jobDone.serializedResult.data(),
jobDone.serializedResult.size());

PCALL_RESL(L, lua_pcall(L, 2, 0, errorhandler));
PCALL_RESL(L, lua_pcall(L, 2, 0, error_handler));
}
resultQueueMutex.unlock();
lua_pop(L, 1); // Pop core
lua_pop(L, 2); // Pop core and error handler
}

/******************************************************************************/
Expand Down Expand Up @@ -248,6 +249,8 @@ void* AsyncWorkerThread::run()
abort();
}

int error_handler = PUSH_ERROR_HANDLER(L);

lua_getglobal(L, "core");
if (lua_isnil(L, -1)) {
errorstream << "Unable to find core within async environment!";
Expand Down Expand Up @@ -279,7 +282,7 @@ void* AsyncWorkerThread::run()
toProcess.serializedParams.data(),
toProcess.serializedParams.size());

int result = lua_pcall(L, 2, 1, m_errorhandler);
int result = lua_pcall(L, 2, 1, error_handler);
if (result) {
PCALL_RES(result);
toProcess.serializedResult = "";
Expand All @@ -296,7 +299,7 @@ void* AsyncWorkerThread::run()
jobDispatcher->putJobResult(toProcess);
}

lua_pop(L, 1); // Pop core
lua_pop(L, 2); // Pop core and error handler

return 0;
}
Expand Down
3 changes: 1 addition & 2 deletions src/script/cpp_api/s_async.h
Expand Up @@ -95,9 +95,8 @@ class AsyncEngine {
* Engine step to process finished jobs
* the engine step is one way to pass events back, PushFinishedJobs another
* @param L The Lua stack
* @param errorhandler Stack index of the Lua error handler
*/
void step(lua_State *L, int errorhandler);
void step(lua_State *L);

/**
* Push a list of finished jobs onto the stack
Expand Down
30 changes: 16 additions & 14 deletions src/script/cpp_api/s_base.cpp
Expand Up @@ -78,14 +78,14 @@ ScriptApiBase::ScriptApiBase()

luaL_openlibs(m_luastack);

// Add and save an error handler
lua_pushcfunction(m_luastack, script_error_handler);
m_errorhandler = lua_gettop(m_luastack);

// Make the ScriptApiBase* accessible to ModApiBase
lua_pushlightuserdata(m_luastack, this);
lua_rawseti(m_luastack, LUA_REGISTRYINDEX, CUSTOM_RIDX_SCRIPTAPI);

// Add and save an error handler
lua_pushcfunction(m_luastack, script_error_handler);
lua_rawseti(m_luastack, LUA_REGISTRYINDEX, CUSTOM_RIDX_ERROR_HANDLER);

// If we are using LuaJIT add a C++ wrapper function to catch
// exceptions thrown in Lua -> C++ calls
#if USE_LUAJIT
Expand Down Expand Up @@ -133,13 +133,15 @@ bool ScriptApiBase::loadScript(const std::string &script_path, std::string *erro

lua_State *L = getStack();

int error_handler = PUSH_ERROR_HANDLER(L);

bool ok;
if (m_secure) {
ok = ScriptApiSecurity::safeLoadFile(L, script_path.c_str());
} else {
ok = !luaL_loadfile(L, script_path.c_str());
}
ok = ok && !lua_pcall(L, 0, 0, m_errorhandler);
ok = ok && !lua_pcall(L, 0, 0, error_handler);
if (!ok) {
std::string error_msg = lua_tostring(L, -1);
if (error)
Expand All @@ -150,9 +152,9 @@ bool ScriptApiBase::loadScript(const std::string &script_path, std::string *erro
<< error_msg << std::endl << std::endl
<< "======= END OF ERROR FROM LUA ========" << std::endl;
lua_pop(L, 1); // Pop error message from stack
return false;
}
return true;
lua_pop(L, 1); // Pop error handler
return ok;
}

// Push the list of callbacks (a lua table).
Expand All @@ -168,28 +170,28 @@ void ScriptApiBase::runCallbacksRaw(int nargs,
FATAL_ERROR_IF(lua_gettop(L) < nargs + 1, "Not enough arguments");

// Insert error handler
lua_pushcfunction(L, script_error_handler);
int errorhandler = lua_gettop(L) - nargs - 1;
lua_insert(L, errorhandler);
PUSH_ERROR_HANDLER(L);
int error_handler = lua_gettop(L) - nargs - 1;
lua_insert(L, error_handler);

// Insert run_callbacks between error handler and table
lua_getglobal(L, "core");
lua_getfield(L, -1, "run_callbacks");
lua_remove(L, -2);
lua_insert(L, errorhandler + 1);
lua_insert(L, error_handler + 1);

// Insert mode after table
lua_pushnumber(L, (int)mode);
lua_insert(L, errorhandler + 3);
lua_insert(L, error_handler + 3);

// Stack now looks like this:
// ... <error handler> <run_callbacks> <table> <mode> <arg#1> <arg#2> ... <arg#n>

int result = lua_pcall(L, nargs + 2, 1, errorhandler);
int result = lua_pcall(L, nargs + 2, 1, error_handler);
if (result != 0)
scriptError(result, fxn);

lua_remove(L, -2); // Remove error handler
lua_remove(L, error_handler);
}

void ScriptApiBase::realityCheck()
Expand Down
2 changes: 0 additions & 2 deletions src/script/cpp_api/s_base.h
Expand Up @@ -109,8 +109,6 @@ class ScriptApiBase {

Mutex m_luastackmutex;
std::string m_last_run_mod;
// Stack index of Lua error handler
int m_errorhandler;
bool m_secure;
#ifdef SCRIPTAPI_LOCK_DEBUG
bool m_locked;
Expand Down
31 changes: 21 additions & 10 deletions src/script/cpp_api/s_entity.cpp
Expand Up @@ -80,6 +80,8 @@ void ScriptApiEntity::luaentity_Activate(u16 id,

verbosestream << "scriptapi_luaentity_activate: id=" << id << std::endl;

int error_handler = PUSH_ERROR_HANDLER(L);

// Get core.luaentities[id]
luaentity_get(L, id);
int object = lua_gettop(L);
Expand All @@ -93,11 +95,11 @@ void ScriptApiEntity::luaentity_Activate(u16 id,
lua_pushinteger(L, dtime_s);

setOriginFromTable(object);
PCALL_RES(lua_pcall(L, 3, 0, m_errorhandler));
PCALL_RES(lua_pcall(L, 3, 0, error_handler));
} else {
lua_pop(L, 1);
}
lua_pop(L, 1); // Pop object
lua_pop(L, 2); // Pop object and error handler
}

void ScriptApiEntity::luaentity_Remove(u16 id)
Expand Down Expand Up @@ -126,6 +128,8 @@ std::string ScriptApiEntity::luaentity_GetStaticdata(u16 id)

//infostream<<"scriptapi_luaentity_get_staticdata: id="<<id<<std::endl;

int error_handler = PUSH_ERROR_HANDLER(L);

// Get core.luaentities[id]
luaentity_get(L, id);
int object = lua_gettop(L);
Expand All @@ -140,9 +144,10 @@ std::string ScriptApiEntity::luaentity_GetStaticdata(u16 id)
lua_pushvalue(L, object); // self

setOriginFromTable(object);
PCALL_RES(lua_pcall(L, 1, 1, m_errorhandler));
PCALL_RES(lua_pcall(L, 1, 1, error_handler));

lua_remove(L, object); // Remove object
lua_remove(L, object);
lua_remove(L, error_handler);

size_t len = 0;
const char *s = lua_tolstring(L, -1, &len);
Expand Down Expand Up @@ -196,6 +201,8 @@ void ScriptApiEntity::luaentity_Step(u16 id, float dtime)

//infostream<<"scriptapi_luaentity_step: id="<<id<<std::endl;

int error_handler = PUSH_ERROR_HANDLER(L);

// Get core.luaentities[id]
luaentity_get(L, id);
int object = lua_gettop(L);
Expand All @@ -211,9 +218,9 @@ void ScriptApiEntity::luaentity_Step(u16 id, float dtime)
lua_pushnumber(L, dtime); // dtime

setOriginFromTable(object);
PCALL_RES(lua_pcall(L, 2, 0, m_errorhandler));
PCALL_RES(lua_pcall(L, 2, 0, error_handler));

lua_pop(L, 1); // Pop object
lua_pop(L, 2); // Pop object and error handler
}

// Calls entity:on_punch(ObjectRef puncher, time_from_last_punch,
Expand All @@ -226,6 +233,8 @@ void ScriptApiEntity::luaentity_Punch(u16 id,

//infostream<<"scriptapi_luaentity_step: id="<<id<<std::endl;

int error_handler = PUSH_ERROR_HANDLER(L);

// Get core.luaentities[id]
luaentity_get(L,id);
int object = lua_gettop(L);
Expand All @@ -244,9 +253,9 @@ void ScriptApiEntity::luaentity_Punch(u16 id,
push_v3f(L, dir);

setOriginFromTable(object);
PCALL_RES(lua_pcall(L, 5, 0, m_errorhandler));
PCALL_RES(lua_pcall(L, 5, 0, error_handler));

lua_pop(L, 1); // Pop object
lua_pop(L, 2); // Pop object and error handler
}

// Calls entity:on_rightclick(ObjectRef clicker)
Expand All @@ -257,6 +266,8 @@ void ScriptApiEntity::luaentity_Rightclick(u16 id,

//infostream<<"scriptapi_luaentity_step: id="<<id<<std::endl;

int error_handler = PUSH_ERROR_HANDLER(L);

// Get core.luaentities[id]
luaentity_get(L, id);
int object = lua_gettop(L);
Expand All @@ -272,8 +283,8 @@ void ScriptApiEntity::luaentity_Rightclick(u16 id,
objectrefGetOrCreate(L, clicker); // Clicker reference

setOriginFromTable(object);
PCALL_RES(lua_pcall(L, 2, 0, m_errorhandler));
PCALL_RES(lua_pcall(L, 2, 0, error_handler));

lua_pop(L, 1); // Pop object
lua_pop(L, 2); // Pop object and error handler
}

0 comments on commit 3304e1e

Please sign in to comment.