Fix race conditions and deadlocks in Channels (hopefully) (issue #554)

Also, this is a giant commit because of the official Code Style (tm) was applied.

--HG--
branch : minor
This commit is contained in:
Bart van Strien
2013-01-30 17:15:12 +01:00
parent be2363fc00
commit c7c5d0ed6b
15 changed files with 414 additions and 367 deletions
+52 -27
View File
@@ -24,55 +24,63 @@
namespace
{
union uslong
{
unsigned long u;
long i;
};
union uslong
{
unsigned long u;
long i;
};
// target <= current, but semi-wrapsafe, one wrap, anyway
inline bool past(unsigned int target, unsigned int current)
{
if (target > current)
return false;
if (target == current)
return true;
// target <= current, but semi-wrapsafe, one wrap, anyway
inline bool past(unsigned int target, unsigned int current)
{
if (target > current)
return false;
if (target == current)
return true;
uslong t, c;
t.u = target;
c.u = current;
uslong t, c;
t.u = target;
c.u = current;
return !(t.i < 0 && c.i > 0);
}
return !(t.i < 0 && c.i > 0);
}
}
namespace love
{
namespace thread
{
static std::map<std::string, Channel*> namedChannels;
static std::map<std::string, Channel *> namedChannels;
static Mutex *namedChannelMutex;
Channel *Channel::getChannel(const std::string &name)
{
if (!namedChannelMutex)
namedChannelMutex = newMutex();
Lock l(namedChannelMutex);
if (!namedChannels.count(name))
namedChannels[name] = new Channel(name);
else
namedChannels[name]->retain();
return namedChannels[name];
}
Channel::Channel()
: named(false), sent(0), received(0)
: named(false)
, sent(0)
, received(0)
{
mutex = newMutex();
cond = newConditional();
}
Channel::Channel(const std::string &name)
: named(true), name(name), sent(0), received(0)
: named(true)
, name(name)
, sent(0)
, received(0)
{
mutex = newMutex();
cond = newConditional();
@@ -88,6 +96,7 @@ Channel::~Channel()
delete mutex;
delete cond;
if (named)
namedChannels.erase(name);
}
@@ -96,12 +105,15 @@ unsigned long Channel::push(Variant *var)
{
if (!var)
return 0;
Lock l(mutex);
var->retain();
// Keep a reference to ourselves
// if we're non-empty and named.
if (named && queue.empty())
retain();
queue.push(var);
cond->broadcast();
@@ -113,13 +125,12 @@ void Channel::supply(Variant *var)
if (!var)
return;
mutex->lock();
unsigned long id = push(var);
mutex->lock();
while (!past(id, received))
{
cond->wait(mutex);
}
mutex->unlock();
}
@@ -139,18 +150,18 @@ Variant *Channel::pop()
// if we're empty and named.
if (named && queue.empty())
release();
return var;
} // NOTE: Returns a retained Variant
Variant *Channel::demand()
{
Variant *var;
mutex->lock();
while (!(var = pop()))
{
mutex->lock();
cond->wait(mutex);
mutex->unlock();
}
mutex->unlock();
return var;
}
@@ -185,12 +196,26 @@ void Channel::clear()
queue.pop();
}
// Finish all the supply waits
received = sent;
cond->broadcast();
// Once again, release our own
// reference if we're named.
if (named)
release();
}
void Channel::lockMutex()
{
mutex->lock();
}
void Channel::unlockMutex()
{
mutex->unlock();
}
void Channel::retain()
{
if (named)
+17 -10
View File
@@ -35,16 +35,9 @@ namespace thread
{
class Channel : public love::Object
{
private:
Mutex *mutex;
Conditional *cond;
std::queue<Variant*> queue;
bool named;
std::string name;
Channel(const std::string &name);
unsigned long sent;
unsigned long received;
// FOR WRAPPER USE ONLY
friend void retainVariant(Channel *, Variant *);
friend void releaseVariant(Channel *, Variant *);
public:
Channel();
@@ -61,6 +54,20 @@ public:
void retain();
void release();
private:
Channel(const std::string &name);
void lockMutex();
void unlockMutex();
Mutex *mutex;
Conditional *cond;
std::queue<Variant *> queue;
bool named;
std::string name;
unsigned long sent;
unsigned long received;
}; // Channel
} // thread
} // love
+30 -30
View File
@@ -30,39 +30,39 @@ namespace love
{
namespace thread
{
LuaThread::LuaThread(const std::string &name, love::Data *code)
: name(name), code(code)
{
code->retain();
}
LuaThread::LuaThread(const std::string &name, love::Data *code)
: name(name), code(code)
{
code->retain();
}
LuaThread::~LuaThread()
{
code->release();
}
LuaThread::~LuaThread()
{
code->release();
}
void LuaThread::threadFunction()
{
this->retain();
lua_State * L = lua_open();
luaL_openlibs(L);
#ifdef LOVE_BUILD_STANDALONE
love::luax_preload(L, luaopen_love, "love");
luaopen_love(L);
#endif // LOVE_BUILD_STANDALONE
luaopen_love_thread(L);
if (luaL_loadbuffer(L, (const char*) code->getData(), code->getSize(), name.c_str()) != 0)
void LuaThread::threadFunction()
{
this->retain();
lua_State * L = lua_open();
luaL_openlibs(L);
#ifdef LOVE_BUILD_STANDALONE
love::luax_preload(L, luaopen_love, "love");
luaopen_love(L);
#endif // LOVE_BUILD_STANDALONE
luaopen_love_thread(L);
if (luaL_loadbuffer(L, (const char *) code->getData(), code->getSize(), name.c_str()) != 0)
error = luax_tostring(L, -1);
else
if (lua_pcall(L, 0, 0, 0) != 0)
error = luax_tostring(L, -1);
else
if (lua_pcall(L, 0, 0, 0) != 0)
error = luax_tostring(L, -1);
lua_close(L);
this->release();
}
lua_close(L);
this->release();
}
const std::string &LuaThread::getError()
{
return error;
}
const std::string &LuaThread::getError()
{
return error;
}
} // thread
} // love
+12 -12
View File
@@ -33,19 +33,19 @@ namespace love
{
namespace thread
{
class LuaThread : public love::Object, public Threadable
{
private:
love::Data *code;
std::string name;
std::string error;
class LuaThread : public love::Object, public Threadable
{
public:
LuaThread(const std::string &name, love::Data *code);
~LuaThread();
void threadFunction();
const std::string &getError();
public:
LuaThread(const std::string &name, love::Data *code);
~LuaThread();
void threadFunction();
const std::string &getError();
};
private:
love::Data *code;
std::string name;
std::string error;
};
} // thread
} // love
+17 -18
View File
@@ -24,26 +24,25 @@ namespace love
{
namespace thread
{
const char *ThreadModule::getName() const
{
return "love.thread.sdl";
}
const char *ThreadModule::getName() const
{
return "love.thread.sdl";
}
LuaThread *ThreadModule::newThread(const std::string &name, love::Data *data)
{
LuaThread *lt = new LuaThread(name, data);
return lt;
}
LuaThread *ThreadModule::newThread(const std::string &name, love::Data *data)
{
LuaThread *lt = new LuaThread(name, data);
return lt;
}
Channel *ThreadModule::newChannel()
{
return new Channel();
}
Channel *ThreadModule::getChannel(const std::string &name)
{
return Channel::getChannel(name);
}
Channel *ThreadModule::newChannel()
{
return new Channel();
}
Channel *ThreadModule::getChannel(const std::string &name)
{
return Channel::getChannel(name);
}
} // thread
} // love
+9 -9
View File
@@ -36,16 +36,16 @@ namespace love
{
namespace thread
{
class ThreadModule : public love::Module
{
public:
virtual ~ThreadModule() {}
LuaThread *newThread(const std::string &name, love::Data *data);
Channel *newChannel();
Channel *getChannel(const std::string &name);
class ThreadModule : public love::Module
{
public:
virtual ~ThreadModule() {}
LuaThread *newThread(const std::string &name, love::Data *data);
Channel *newChannel();
Channel *getChannel(const std::string &name);
const char *getName() const;
}; // ThreadModule
const char *getName() const;
}; // ThreadModule
} // thread
} // love
+56 -54
View File
@@ -26,65 +26,67 @@ namespace thread
{
namespace sdl
{
Thread::Thread(Threadable *t)
: t(t), running(false), thread(0)
{
}
Thread::Thread(Threadable *t)
: t(t)
, running(false)
, thread(0)
{
}
Thread::~Thread()
{
if (!running) // Clean up handle
wait();
/*
if (running)
wait();
FIXME: Needed for proper thread cleanup
*/
}
Thread::~Thread()
{
if (!running) // Clean up handle
wait();
/*
if (running)
wait();
FIXME: Needed for proper thread cleanup
*/
}
bool Thread::start()
{
Lock l(mutex);
if (running)
return false;
if (thread) // Clean old handle up
SDL_WaitThread(thread, 0);
thread = SDL_CreateThread(thread_runner, this);
running = (thread != 0);
return running;
}
void Thread::wait()
{
mutex.lock();
if (!thread)
return;
mutex.unlock();
bool Thread::start()
{
Lock l(mutex);
if (running)
return false;
if (thread) // Clean old handle up
SDL_WaitThread(thread, 0);
Lock l(mutex);
running = false;
thread = 0;
}
thread = SDL_CreateThread(thread_runner, this);
running = (thread != 0);
return running;
}
void Thread::kill()
{
Lock l(mutex);
if (!running)
return;
SDL_KillThread(thread);
SDL_WaitThread(thread, 0);
running = false;
thread = 0;
}
void Thread::wait()
{
mutex.lock();
if (!thread)
return;
mutex.unlock();
SDL_WaitThread(thread, 0);
Lock l(mutex);
running = false;
thread = 0;
}
int Thread::thread_runner(void *data)
{
Thread *self = (Thread*) data; // some compilers don't like 'this'
self->t->threadFunction();
Lock l(self->mutex);
self->running = false;
return 0;
}
void Thread::kill()
{
Lock l(mutex);
if (!running)
return;
SDL_KillThread(thread);
SDL_WaitThread(thread, 0);
running = false;
thread = 0;
}
int Thread::thread_runner(void *data)
{
Thread *self = (Thread *) data; // some compilers don't like 'this'
self->t->threadFunction();
Lock l(self->mutex);
self->running = false;
return 0;
}
} // sdl
} // thread
} // love
+15 -15
View File
@@ -34,22 +34,22 @@ namespace thread
{
namespace sdl
{
class Thread : public thread::Thread
{
private:
Threadable *t;
bool running;
SDL_Thread *thread;
static int thread_runner(void *data);
Mutex mutex;
class Thread : public thread::Thread
{
private:
Threadable *t;
bool running;
SDL_Thread *thread;
static int thread_runner(void *data);
Mutex mutex;
public:
Thread(Threadable *t);
~Thread();
bool start();
void kill();
void wait();
}; // Thread
public:
Thread(Threadable *t);
~Thread();
bool start();
void kill();
void wait();
}; // Thread
} // sdl
} // thread
} // love
+1 -1
View File
@@ -72,7 +72,7 @@ bool Conditional::wait(thread::Mutex *_mutex, int timeout)
// Yes, I realise this can be dangerous,
// however, you're asking for it if you're
// mixing thread implementations.
Mutex *mutex = (Mutex*) _mutex;
Mutex *mutex = (Mutex *) _mutex;
if (timeout < 0)
return !SDL_CondWait(cond, mutex->mutex);
else
+24 -24
View File
@@ -32,37 +32,37 @@ namespace thread
{
namespace sdl
{
class Conditional;
class Conditional;
class Mutex : public thread::Mutex
{
public:
Mutex();
~Mutex();
class Mutex : public thread::Mutex
{
public:
Mutex();
~Mutex();
void lock();
void unlock();
void lock();
void unlock();
private:
SDL_mutex* mutex;
Mutex(const Mutex&/* mutex*/) {}
private:
SDL_mutex *mutex;
Mutex(const Mutex&/* mutex*/) {}
friend class Conditional;
};
friend class Conditional;
};
class Conditional : public thread::Conditional
{
public:
Conditional();
~Conditional();
class Conditional : public thread::Conditional
{
public:
Conditional();
~Conditional();
void signal();
void broadcast();
bool wait(thread::Mutex* mutex, int timeout=-1);
void signal();
void broadcast();
bool wait(thread::Mutex *mutex, int timeout=-1);
private:
SDL_cond* cond;
};
private:
SDL_cond *cond;
};
} // sdl
} // thread
+3 -3
View File
@@ -45,14 +45,14 @@ public:
virtual void signal() = 0;
virtual void broadcast() = 0;
virtual bool wait(Mutex* mutex, int timeout=-1) = 0;
virtual bool wait(Mutex *mutex, int timeout=-1) = 0;
};
class Lock
{
public:
Lock(Mutex* m);
Lock(Mutex& m);
Lock(Mutex *m);
Lock(Mutex &m);
~Lock();
private:
+89 -75
View File
@@ -24,94 +24,108 @@ namespace love
{
namespace thread
{
Channel *luax_checkchannel(lua_State *L, int idx)
{
return luax_checktype<Channel>(L, idx, "Channel", THREAD_CHANNEL_T);
}
void retainVariant(Channel *c, Variant *v)
{
c->lockMutex();
v->retain();
c->unlockMutex();
}
int w_Channel_push(lua_State *L)
{
Channel *c = luax_checkchannel(L, 1);
Variant *var = Variant::fromLua(L, 2);
c->push(var);
var->release();
return 0;
}
void releaseVariant(Channel *c, Variant *v)
{
c->lockMutex();
v->release();
c->unlockMutex();
}
int w_Channel_supply(lua_State *L)
{
Channel *c = luax_checkchannel(L, 1);
Variant *var = Variant::fromLua(L, 2);
c->supply(var);
var->release();
return 0;
}
Channel *luax_checkchannel(lua_State *L, int idx)
{
return luax_checktype<Channel>(L, idx, "Channel", THREAD_CHANNEL_T);
}
int w_Channel_pop(lua_State *L)
{
Channel *c = luax_checkchannel(L, 1);
Variant *var = c->pop();
if (var)
{
var->toLua(L);
var->release();
}
else
lua_pushnil(L);
return 1;
}
int w_Channel_push(lua_State *L)
{
Channel *c = luax_checkchannel(L, 1);
Variant *var = Variant::fromLua(L, 2);
c->push(var);
releaseVariant(c, var);
return 0;
}
int w_Channel_demand(lua_State *L)
int w_Channel_supply(lua_State *L)
{
Channel *c = luax_checkchannel(L, 1);
Variant *var = Variant::fromLua(L, 2);
c->supply(var);
releaseVariant(c, var);
return 0;
}
int w_Channel_pop(lua_State *L)
{
Channel *c = luax_checkchannel(L, 1);
Variant *var = c->pop();
if (var)
{
Channel *c = luax_checkchannel(L, 1);
Variant *var = c->demand();
var->toLua(L);
var->release();
return 1;
releaseVariant(c, var);
}
else
lua_pushnil(L);
return 1;
}
int w_Channel_peek(lua_State *L)
int w_Channel_demand(lua_State *L)
{
Channel *c = luax_checkchannel(L, 1);
Variant *var = c->demand();
var->toLua(L);
releaseVariant(c, var);
return 1;
}
int w_Channel_peek(lua_State *L)
{
Channel *c = luax_checkchannel(L, 1);
Variant *var = c->peek();
if (var)
{
Channel *c = luax_checkchannel(L, 1);
Variant *var = c->peek();
if (var)
{
var->toLua(L);
var->release();
}
else
lua_pushnil(L);
return 1;
var->toLua(L);
releaseVariant(c, var);
}
else
lua_pushnil(L);
return 1;
}
int w_Channel_count(lua_State *L)
{
Channel *c = luax_checkchannel(L, 1);
lua_pushnumber(L, c->count());
return 1;
}
int w_Channel_count(lua_State *L)
{
Channel *c = luax_checkchannel(L, 1);
lua_pushnumber(L, c->count());
return 1;
}
int w_Channel_clear(lua_State *L)
{
Channel *c = luax_checkchannel(L, 1);
c->clear();
return 0;
}
int w_Channel_clear(lua_State *L)
{
Channel *c = luax_checkchannel(L, 1);
c->clear();
return 0;
}
static const luaL_Reg type_functions[] = {
{ "push", w_Channel_push },
{ "supply", w_Channel_supply },
{ "pop", w_Channel_pop },
{ "demand", w_Channel_demand },
{ "peek", w_Channel_peek },
{ "count", w_Channel_count },
{ "clear", w_Channel_clear },
{ 0, 0 }
};
static const luaL_Reg type_functions[] = {
{ "push", w_Channel_push },
{ "supply", w_Channel_supply },
{ "pop", w_Channel_pop },
{ "demand", w_Channel_demand },
{ "peek", w_Channel_peek },
{ "count", w_Channel_count },
{ "clear", w_Channel_clear },
{ 0, 0 }
};
extern "C" int luaopen_channel(lua_State *L)
{
return luax_register_type(L, "Channel", type_functions);
}
extern "C" int luaopen_channel(lua_State *L)
{
return luax_register_type(L, "Channel", type_functions);
}
}
}
+9 -9
View File
@@ -28,16 +28,16 @@ namespace love
{
namespace thread
{
Channel *luax_checkchannel(lua_State *L, int idx);
int w_Channel_push(lua_State *L);
int w_Channel_supply(lua_State *L);
int w_Channel_pop(lua_State *L);
int w_Channel_demand(lua_State *L);
int w_Channel_peek(lua_State *L);
int w_Channel_count(lua_State *L);
int w_Channel_clear(lua_State *L);
Channel *luax_checkchannel(lua_State *L, int idx);
int w_Channel_push(lua_State *L);
int w_Channel_supply(lua_State *L);
int w_Channel_pop(lua_State *L);
int w_Channel_demand(lua_State *L);
int w_Channel_peek(lua_State *L);
int w_Channel_count(lua_State *L);
int w_Channel_clear(lua_State *L);
extern "C" int luaopen_channel(lua_State *L);
extern "C" int luaopen_channel(lua_State *L);
} // thread
} // love
+76 -76
View File
@@ -28,99 +28,99 @@ namespace love
{
namespace thread
{
static ThreadModule *instance = 0;
static ThreadModule *instance = 0;
int w_newThread(lua_State *L)
int w_newThread(lua_State *L)
{
std::string name = "Thread code";
love::Data *data;
if (lua_isstring(L, 1))
luax_convobj(L, 1, "filesystem", "newFile");
if (luax_istype(L, 1, FILESYSTEM_FILE_T))
{
std::string name = "Thread code";
love::Data *data;
if (lua_isstring(L, 1))
luax_convobj(L, 1, "filesystem", "newFile");
if (luax_istype(L, 1, FILESYSTEM_FILE_T))
try
{
try
{
love::filesystem::File * file = luax_checktype<love::filesystem::File>(L, 1, "File", FILESYSTEM_FILE_T);
name = std::string("@") + file->getFilename();
data = file->read();
}
catch (love::Exception & e)
{
return luaL_error(L, "%s", e.what());
}
love::filesystem::File * file = luax_checktype<love::filesystem::File>(L, 1, "File", FILESYSTEM_FILE_T);
name = std::string("@") + file->getFilename();
data = file->read();
}
else if (luax_istype(L, 1, FILESYSTEM_FILE_DATA_T))
catch (love::Exception & e)
{
love::filesystem::FileData * fdata = luax_checktype<love::filesystem::FileData>(L, 1, "FileData", FILESYSTEM_FILE_DATA_T);
name = std::string("@") + fdata->getFilename();
data = fdata;
data->retain();
return luaL_error(L, "%s", e.what());
}
else
{
data = luax_checktype<love::Data>(L, 1, "Data", DATA_T);
data->retain();
}
LuaThread *t = instance->newThread(name, data);
// do not worry, file->read() returns retained data
data->release();
luax_newtype(L, "Thread", THREAD_THREAD_T, (void*)t);
return 1;
}
int w_newChannel(lua_State *L)
else if (luax_istype(L, 1, FILESYSTEM_FILE_DATA_T))
{
Channel *c = instance->newChannel();
luax_newtype(L, "Channel", THREAD_CHANNEL_T, (void*)c);
return 1;
love::filesystem::FileData * fdata = luax_checktype<love::filesystem::FileData>(L, 1, "FileData", FILESYSTEM_FILE_DATA_T);
name = std::string("@") + fdata->getFilename();
data = fdata;
data->retain();
}
int w_getChannel(lua_State *L)
else
{
std::string name = luax_checkstring(L, 1);
Channel *c = instance->getChannel(name);
luax_newtype(L, "Channel", THREAD_CHANNEL_T, (void*)c);
return 1;
data = luax_checktype<love::Data>(L, 1, "Data", DATA_T);
data->retain();
}
LuaThread *t = instance->newThread(name, data);
// do not worry, file->read() returns retained data
data->release();
luax_newtype(L, "Thread", THREAD_THREAD_T, (void *)t);
return 1;
}
// List of functions to wrap.
static const luaL_Reg module_functions[] = {
{ "newThread", w_newThread },
{ "newChannel", w_newChannel },
{ "getChannel", w_getChannel },
{ 0, 0 }
};
int w_newChannel(lua_State *L)
{
Channel *c = instance->newChannel();
luax_newtype(L, "Channel", THREAD_CHANNEL_T, (void *)c);
return 1;
}
static const lua_CFunction types[] = {
luaopen_thread,
luaopen_channel,
0
};
int w_getChannel(lua_State *L)
{
std::string name = luax_checkstring(L, 1);
Channel *c = instance->getChannel(name);
luax_newtype(L, "Channel", THREAD_CHANNEL_T, (void *)c);
return 1;
}
extern "C" int luaopen_love_thread(lua_State *L)
// List of functions to wrap.
static const luaL_Reg module_functions[] = {
{ "newThread", w_newThread },
{ "newChannel", w_newChannel },
{ "getChannel", w_getChannel },
{ 0, 0 }
};
static const lua_CFunction types[] = {
luaopen_thread,
luaopen_channel,
0
};
extern "C" int luaopen_love_thread(lua_State *L)
{
if (instance == 0)
{
if (instance == 0)
try
{
try
{
instance = new ThreadModule();
}
catch (Exception & e)
{
return luaL_error(L, "%s", e.what());
}
instance = new ThreadModule();
}
catch (Exception & e)
{
return luaL_error(L, "%s", e.what());
}
else
instance->retain();
WrappedModule w;
w.module = instance;
w.name = "thread";
w.flags = MODULE_T;
w.functions = module_functions;
w.types = types;
return luax_register_module(L, w);
}
else
instance->retain();
WrappedModule w;
w.module = instance;
w.name = "thread";
w.flags = MODULE_T;
w.functions = module_functions;
w.types = types;
return luax_register_module(L, w);
}
}
}
+4 -4
View File
@@ -28,11 +28,11 @@ namespace love
{
namespace thread
{
int w_newThread(lua_State *L);
int w_newChannel(lua_State *L);
int w_getChannel(lua_State *L);
int w_newThread(lua_State *L);
int w_newChannel(lua_State *L);
int w_getChannel(lua_State *L);
extern "C" LOVE_EXPORT int luaopen_love_thread(lua_State * L);
extern "C" LOVE_EXPORT int luaopen_love_thread(lua_State * L);
} // thread
} // love