From 369dde36c38c1eac6ce535ffe8c7522fb02f7280 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Mon, 23 Nov 2015 15:50:53 -0500 Subject: Load thread's start routine from separate shared library Prior to this change, if Lua unloads the threads library before the child threads terminate, the program wil segfault. This could happen if an error is triggered during an endcallback, preventing the child thread from exiting. This places the thread's start routine in a separate shared library: libthreadsmain, which is loaded once and never unloaded. --- .gitignore | 2 ++ CMakeLists.txt | 8 +++++++- lib/THThread.c | 40 +++++++++++++----------------------- lib/THThread.h | 6 +++++- lib/thread-main.c | 42 ++++++++++++++++++++++++++++++++++++++ lib/threads.c | 49 +++++++++++++++++---------------------------- test/test-threads-error.lua | 17 ++++++++++++++++ 7 files changed, 105 insertions(+), 59 deletions(-) create mode 100644 lib/thread-main.c create mode 100644 test/test-threads-error.lua diff --git a/.gitignore b/.gitignore index b08795f..7d07faa 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,5 @@ install_manifest.txt # Build output libthreads.a libthreads.so +libthreadsmain.dylib +libthreadsmain.so diff --git a/CMakeLists.txt b/CMakeLists.txt index d6b2fcf..a583a6f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -20,9 +20,15 @@ set(luasrc ) add_torch_package(threads "${src}" "${luasrc}" "Threads") - target_link_libraries(threads luaT TH) +ADD_LIBRARY(threadsmain MODULE lib/thread-main.c) +IF(APPLE) + SET_TARGET_PROPERTIES(threadsmain PROPERTIES + LINK_FLAGS "-undefined dynamic_lookup") +ENDIF() +INSTALL(TARGETS threadsmain LIBRARY DESTINATION "${Torch_INSTALL_LIB_SUBDIR}") + if(WIN32) add_definitions(-DUSE_WIN_THREADS=1) else() diff --git a/lib/THThread.c b/lib/THThread.c index 1a680c6..a273600 100644 --- a/lib/THThread.c +++ b/lib/THThread.c @@ -1,6 +1,3 @@ -#ifndef TH_THREAD_INC -#define TH_THREAD_INC - #include #include @@ -85,39 +82,32 @@ int pthread_cond_signal(pthread_cond_t *cond) #error no thread system available #endif -typedef struct THThread_ { +struct THThread_ { pthread_t id; int (*func)(void*); - void* data; - int status; -} THThread; + THThreadState state; +}; -typedef struct THMutex_{ +struct THMutex_{ pthread_mutex_t id; int refcount; -} THMutex; +}; -typedef struct THCondition_ { +struct THCondition_ { pthread_cond_t id; int refcount; -} THCondition; - -static void* thread_closure(void *data) -{ - THThread *thread = data; - thread->status = thread->func(thread->data); - return NULL; -} +}; -THThread* THThread_new(int (*func)(void*), void *data) +THThread* THThread_new(void* (*func)(void*), void *data) { THThread *self = malloc(sizeof(THThread)); - self->func = func; - self->data = data; - self->status = 0; if(!self) return NULL; - if(pthread_create(&self->id, NULL, thread_closure, self)) { + + self->state.data = data; + self->state.status = 0; + + if(pthread_create(&self->id, NULL, func, &self->state)) { free(self); return NULL; } @@ -135,7 +125,7 @@ int THThread_free(THThread *self) if(self) { if(pthread_join(self->id, NULL)) return 1; - status = self->status; + status = self->state.status; free(self); } return status; @@ -238,5 +228,3 @@ void THCondition_free(THCondition *self) } } } - -#endif diff --git a/lib/THThread.h b/lib/THThread.h index 0b086fb..008223d 100644 --- a/lib/THThread.h +++ b/lib/THThread.h @@ -4,8 +4,12 @@ typedef struct THThread_ THThread; typedef struct THMutex_ THMutex; typedef struct THCondition_ THCondition; +typedef struct THThreadState_ { + void* data; + int status; +} THThreadState; -THThread* THThread_new(int (*closure)(void*), void *data); +THThread* THThread_new(void* (*closure)(void*), void *data); long THThread_id(THThread *self); int THThread_free(THThread *self); diff --git a/lib/thread-main.c b/lib/thread-main.c new file mode 100644 index 0000000..344e679 --- /dev/null +++ b/lib/thread-main.c @@ -0,0 +1,42 @@ +#include +#include +#include +#include +#include +#include "THThread.h" + +static int runthread(void *code_) +{ + char *code = code_; + lua_State *L = luaL_newstate(); + + if(!L) { + printf("THREAD FATAL ERROR: could not create lua state\n"); + return -1; + } + luaL_openlibs(L); + + if(luaL_loadstring(L, code)) { + printf("FATAL THREAD PANIC: (loadstring) %s\n", lua_tolstring(L, -1, NULL)); + free(code); + lua_close(L); + return -1; + } + + free(code); + if(lua_pcall(L, 0, 0, 0)) { + printf("FATAL THREAD PANIC: (pcall) %s\n", lua_tolstring(L, -1, NULL)); + lua_close(L); + return -1; + } + + lua_close(L); + return 0; +} + +void* THThread_main(void *arg) +{ + THThreadState* state = arg; + state->status = runthread(state->data); + return NULL; +} diff --git a/lib/threads.c b/lib/threads.c index 5f379fe..fbb8d60 100644 --- a/lib/threads.c +++ b/lib/threads.c @@ -2,6 +2,7 @@ #include #include #include +#include #include "THThread.h" #include "luaTHRD.h" @@ -9,34 +10,6 @@ #include #include -static int newthread(void *code_) -{ - char *code = code_; - lua_State *L = luaL_newstate(); - - if(!L) { - printf("THREAD FATAL ERROR: could not create lua state\n"); - return -1; - } - luaL_openlibs(L); - - if(luaL_loadstring(L, code)) { - printf("FATAL THREAD PANIC: (loadstring) %s\n", lua_tolstring(L, -1, NULL)); - free(code); - lua_close(L); - return -1; - } - free(code); - if(lua_pcall(L, 0, 0, 0)) { - printf("FATAL THREAD PANIC: (pcall) %s\n", lua_tolstring(L, -1, NULL)); - lua_close(L); - return -1; - } - - lua_close(L); - return 0; -} - static int thread_new(lua_State *L) { THThread *thread = NULL; @@ -47,11 +20,25 @@ static int thread_new(lua_State *L) luaL_error(L, "threads: out of memory"); memcpy(code_dup, code, len+1); - thread = THThread_new(newthread, (void*)code_dup); - if(!thread) + void* lib = dlopen("libthreadsmain.so", RTLD_LAZY|RTLD_LOCAL|RTLD_NODELETE); + if (!lib) { + free(code_dup); + luaL_error(L, "threads: dlopen: %s", dlerror()); + } + + void* (*thread_main)(void*) = dlsym(lib, "THThread_main"); + if (!thread_main) { + free(code_dup); + luaL_error(L, "threads: dlsym: %s", dlerror()); + } + + thread = THThread_new(thread_main, (void*)code_dup); + if(!thread) { + free(code_dup); luaL_error(L, "threads: thread new failed"); - luaTHRD_pushudata(L, thread, "threads.Thread"); + } + luaTHRD_pushudata(L, thread, "threads.Thread"); return 1; } diff --git a/test/test-threads-error.lua b/test/test-threads-error.lua new file mode 100644 index 0000000..01bf4dd --- /dev/null +++ b/test/test-threads-error.lua @@ -0,0 +1,17 @@ +local threads = require 'threads' + +local t = threads.Threads(1, function() + sys = require 'sys' +end) + +-- Trigger an error in an endcallback. The callback is run during lua_close +-- when the Threads:synchronize method is called from the __gc metamethod. +-- The error may prevent the thread from terminating before the threads module +-- and libthreads.so is unloaded. In previous versions of threads this would +-- cause a segfault. + +t:addjob(function() + sys.sleep(0.1) +end, function() + error('error from callback') +end) -- cgit v1.2.3