16.12.2021, 9:00 - 11:00: Due to updates GitLab may be unavailable for some minutes between 09:00 and 11:00.

Commit a337b188 authored by Artur Grunau's avatar Artur Grunau
Browse files

Guard accesses to Lua state with a mutex

Now that support for signals and slots has been added to Lua pipelines,
each Lua state has to be protected with a mutex because it's not
thread-safe and different threads (running LuaPipeline or a processor
attached to it) may try to access it simultaneously.

The mutex needs to be recursive due to the fact that Lua code can
trigger the emission (or copying) of signals that have slots defined it
Lua connected to them. This in turn causes the state to be accessed from
a thread that, unbeknownst to it, already holds a lock on the mutex.

A pointer to the mutex is stored in the state's registry; this way code
that accesses Lua state directly (e.g. connections between sigslot's
signals and slots defined in Lua) can retrieve and lock it when
necessary.

References #1
parent 8f1dd0c9
......@@ -26,6 +26,7 @@ namespace campvis {
}
void GlobalLuaTable::pushField(const std::string& name) {
LuaStateMutexType::scoped_lock lock(_luaVmState.getMutex());
lua_getglobal(_luaVmState.rawState(), name.c_str());
}
}
......@@ -12,16 +12,28 @@ extern "C" {
namespace campvis {
LuaVmState::LuaVmState(bool loadStdLibs /*= true*/) : _luaState(0)
LuaVmState::LuaVmState(bool loadStdLibs /*= true*/)
: _luaState(0)
, _luaStateMutex()
{
_luaState = luaL_newstate();
// load standard Lua libraries
if (loadStdLibs)
luaL_openlibs(_luaState);
/*
* Store a pointer to the mutex guarding access to _luaState in the state's registry; this
* way code that accesses Lua state directly (e.g. connections between sigslot's signals and
* slots defined in Lua) has access to it and can lock it when necessary.
*/
lua_pushlightuserdata(_luaState, static_cast<void*>(_luaState));
lua_pushlightuserdata(_luaState, static_cast<void*>(&_luaStateMutex));
lua_settable(_luaState, LUA_REGISTRYINDEX);
}
LuaVmState::~LuaVmState() {
LuaStateMutexType::scoped_lock lock(_luaStateMutex);
lua_close(_luaState);
}
......@@ -37,6 +49,8 @@ namespace campvis {
}
bool LuaVmState::execFile(const std::string& scriptPath) {
LuaStateMutexType::scoped_lock lock(_luaStateMutex);
// run a Lua script here; true is returned if there were errors
if (luaL_dofile(_luaState, scriptPath.c_str())) {
this->logLuaError();
......@@ -47,6 +61,8 @@ namespace campvis {
}
bool LuaVmState::execString(const std::string& scriptString) {
LuaStateMutexType::scoped_lock lock(_luaStateMutex);
if (luaL_dostring(_luaState, scriptString.c_str())) {
this->logLuaError();
return false;
......@@ -59,11 +75,17 @@ namespace campvis {
return std::shared_ptr<GlobalLuaTable>(new GlobalLuaTable(*this));
}
lua_State *LuaVmState::rawState() const {
lua_State* LuaVmState::rawState() const {
return _luaState;
}
LuaStateMutexType& LuaVmState::getMutex() {
return _luaStateMutex;
}
void LuaVmState::callLuaFunc(int nargs, int nresults) {
LuaStateMutexType::scoped_lock lock(_luaStateMutex);
if (lua_pcall(_luaState, nargs, nresults, 0) != LUA_OK) {
this->logLuaError();
}
......
......@@ -4,6 +4,7 @@
#include <memory>
#include <string>
#include "swigluarun.h"
#include "tbb/recursive_mutex.h"
extern "C" {
#include "lua.h"
......@@ -11,13 +12,23 @@ extern "C" {
#include "lauxlib.h"
}
struct lua_State;
namespace campvis {
class GlobalLuaTable;
/**
* The Lua state managed by LuaVmState has to be protected with a mutex because it's not
* thread-safe and different threads (running LuaPipeline or a processor attached to it) may try
* to access it simultaneously.
*
* The mutex needs to be recursive due to the fact that Lua code can trigger the emission (or
* copying) of signals that have slots defined it Lua connected to them. This in turn causes the
* state to be accessed from a thread that, unbeknownst to it, already holds a lock on the
* mutex.
*/
typedef tbb::recursive_mutex LuaStateMutexType;
class LuaVmState
{
public:
......@@ -31,7 +42,19 @@ namespace campvis {
template<typename T>
bool injectObjectPointer(T* objPointer, const std::string& typeName, const std::string& luaVarName);
struct lua_State* rawState() const;
/**
* Return the Lua state managed by LuaVmState.
*
* \note The mutex returned by getMutex() must be locked before accessing the state in any
* way.
*/
lua_State* rawState() const;
/**
* Return the mutex guarding access to the Lua state managed by LuaVmState.
*/
LuaStateMutexType& getMutex();
/**
* Call the Lua function that's at the top of the stack.
......@@ -41,11 +64,13 @@ namespace campvis {
private:
void logLuaError();
struct lua_State* _luaState; ///< Lua state managed by LuaVmState
lua_State* _luaState; ///< Lua state managed by LuaVmState
LuaStateMutexType _luaStateMutex; ///< Mutex guarding access to the above Lua state
};
template<typename T>
bool LuaVmState::injectObjectPointer(T* objPointer, const std::string& typeName, const std::string& luaVarName) {
LuaStateMutexType::scoped_lock lock(_luaStateMutex);
swig_type_info* objTypeInfo = SWIG_TypeQuery(_luaState, typeName.c_str());
if (objTypeInfo == nullptr) {
......
......@@ -15,11 +15,15 @@ namespace campvis {
bool result = false;
_parent->pushField(_name);
if (lua_istable(_luaVmState.rawState(), -1) == 1)
result = true;
{
LuaStateMutexType::scoped_lock lock(_luaVmState.getMutex());
// Pop the table
lua_pop(_luaVmState.rawState(), 1);
if (lua_istable(_luaVmState.rawState(), -1) == 1)
result = true;
// Pop the table
lua_pop(_luaVmState.rawState(), 1);
}
return result;
}
......@@ -30,16 +34,28 @@ namespace campvis {
void RegularLuaTable::callInstanceMethod(const std::string& name) {
_parent->pushField(_name);
lua_getfield(_luaVmState.rawState(), -1, name.c_str());
{
LuaStateMutexType::scoped_lock lock(_luaVmState.getMutex());
lua_getfield(_luaVmState.rawState(), -1, name.c_str());
}
_parent->pushField(_name);
_luaVmState.callLuaFunc(1, 0);
// Pop the table
lua_pop(_luaVmState.rawState(), 1);
{
LuaStateMutexType::scoped_lock lock(_luaVmState.getMutex());
// Pop the table
lua_pop(_luaVmState.rawState(), 1);
}
}
void RegularLuaTable::pushField(const std::string& name) {
_parent->pushField(_name);
lua_getfield(_luaVmState.rawState(), -1, name.c_str());
{
LuaStateMutexType::scoped_lock lock(_luaVmState.getMutex());
lua_getfield(_luaVmState.rawState(), -1, name.c_str());
}
}
}
......@@ -3,6 +3,7 @@
%include lua_fnptr.i
%{
#include "tbb/recursive_mutex.h"
#include "ext/sigslot/sigslot.h"
%}
......@@ -27,6 +28,8 @@ namespace sigslot {
template<typename T>
struct LuaConnectionArgTraits {};
typedef tbb::recursive_mutex LuaStateMutexType;
/**
* Custom signal-slot connection type that accepts Lua functions as slots.
*
......@@ -37,8 +40,25 @@ namespace sigslot {
class _lua_connection1 : public _connection_base1<arg1_type, mt_policy>
{
public:
_lua_connection1() : _slot_fn(), _dummy_dest(nullptr) {}
_lua_connection1(SWIGLUA_REF slot_fn) : _slot_fn(slot_fn), _dummy_dest(nullptr) {}
_lua_connection1(SWIGLUA_REF slot_fn)
: _slot_fn(slot_fn)
, _dummy_dest(nullptr)
{
// Retrieve from the registry the mutex associated with the function's Lua state
lua_pushlightuserdata(slot_fn.L, static_cast<void*>(slot_fn.L));
lua_gettable(slot_fn.L, LUA_REGISTRYINDEX);
// The mutex should be stored as light userdata
assert(lua_islightuserdata(slot_fn.L, -1));
_lua_state_mutex = static_cast<LuaStateMutexType*>(lua_touserdata(slot_fn.L, -1));
lua_pop(slot_fn.L, 1);
}
_lua_connection1(SWIGLUA_REF slot_fn, LuaStateMutexType* lua_state_mutex)
: _slot_fn(slot_fn)
, _dummy_dest(nullptr)
, _lua_state_mutex(lua_state_mutex)
{}
virtual ~_lua_connection1() {
swiglua_ref_clear(&_slot_fn);
......@@ -50,11 +70,15 @@ namespace sigslot {
virtual _connection_base1<arg1_type, mt_policy>* clone() {
SWIGLUA_REF slot_fn;
swiglua_ref_get(&_slot_fn);
swiglua_ref_set(&slot_fn, _slot_fn.L, -1);
lua_pop(_slot_fn.L, 1);
{
LuaStateMutexType::scoped_lock lock(*_lua_state_mutex);
swiglua_ref_get(&_slot_fn);
swiglua_ref_set(&slot_fn, _slot_fn.L, -1);
lua_pop(_slot_fn.L, 1);
}
return new _lua_connection1(slot_fn);
return new _lua_connection1(slot_fn, _lua_state_mutex);
}
virtual _connection_base1<arg1_type, mt_policy>* duplicate(sigslot::has_slots<mt_policy>* pnewdest) {
......@@ -75,19 +99,23 @@ namespace sigslot {
return;
}
// Put this connection's slot and all arguments on Lua's stack
swiglua_ref_get(&_slot_fn);
SWIG_NewPointerObj(_slot_fn.L, a1, argTypeInfo, 0);
{
LuaStateMutexType::scoped_lock lock(*_lua_state_mutex);
if (lua_pcall(_slot_fn.L, 1, 0, 0) != LUA_OK) {
const char* errorMsg = lua_tostring(_slot_fn.L, -1);
// Put this connection's slot and all arguments on Lua's stack
swiglua_ref_get(&_slot_fn);
SWIG_NewPointerObj(_slot_fn.L, a1, argTypeInfo, 0);
if (errorMsg == nullptr)
std::cerr << "(error object is not a string)" << std::endl;
else
std::cerr << "An error occured while calling a Lua slot function: " << errorMsg << std::endl;
if (lua_pcall(_slot_fn.L, 1, 0, 0) != LUA_OK) {
const char* errorMsg = lua_tostring(_slot_fn.L, -1);
lua_pop(_slot_fn.L, 1);
if (errorMsg == nullptr)
std::cerr << "(error object is not a string)" << std::endl;
else
std::cerr << "An error occured while calling a Lua slot function: " << errorMsg << std::endl;
lua_pop(_slot_fn.L, 1);
}
}
}
......@@ -105,6 +133,7 @@ namespace sigslot {
private:
SWIGLUA_REF _slot_fn; ///< Reference to a Lua function acting as a slot
mutable has_slots<mt_policy>* _dummy_dest; ///< Dummy destination object needed to support getdest()
LuaStateMutexType* _lua_state_mutex; ///< Mutex guarding access to the above function's Lua state
};
}
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment