fix lua_thread to call lua_resume from outside TL state, and to never call the callback directly lua-threads
authorTero Marttila <terom@fixme.fi>
Thu, 21 May 2009 16:22:57 +0300
branchlua-threads
changeset 204 7dfc3d7bd92b
parent 203 ffdf53fd0337
child 205 c13d2fc7b480
fix lua_thread to call lua_resume from outside TL state, and to never call the callback directly
src/lua_thread.c
src/lua_thread.h
--- a/src/lua_thread.c	Wed May 20 22:53:05 2009 +0300
+++ b/src/lua_thread.c	Thu May 21 16:22:57 2009 +0300
@@ -22,8 +22,6 @@
  */
 static void lua_thread_done (lua_State *L, struct lua_thread *thread, error_t *err)
 {
-    assert(L == thread->L);
-
     // remove from registry so thread can be GC'd
     lua_pushthread(L);
     lua_pushnil(L);
@@ -33,33 +31,33 @@
     thread->L = NULL;
 
     // call
-    thread->cb_func(thread, err, thread->cb_arg);
+    thread->cb_func(err, thread->cb_arg);
 }
 
 /**
- * Execute the lua_resume with zero arguments, invoking the callback on succesful completion, and returning any
- * error.
+ * Execute the lua_resume with zero arguments. If the thread finished, this returns zero, if it yielded, >0. For
+ * errors, returns -err_t.
  */
-static err_t lua_thread_resume (lua_State *L, struct lua_thread *thread, error_t *err)
+static int lua_thread_resume (lua_State *L, struct lua_thread *thread, error_t *err)
 {
     int ret;
 
+    (void) thread;
+    
     // invoke it
     switch ((ret = lua_resume(L, 0))) {
         case LUA_YIELD:
             // ok, running, wait
-            return SUCCESS;
+            return 1;
 
         case 0:
-            // done, notify user
-            lua_thread_done(L, thread, NULL);
-
-            return SUCCESS;
+            // done
+            return 0;
 
         default:
             // let caller handle
             // XXX: backtrace...
-            return nexus_lua_error(L, ret, err);
+            return -nexus_lua_error(L, ret, err);
     }
 }
 
@@ -69,16 +67,15 @@
 };
 
 /**
- * Setup the given newthread state, context, and load/run the given chunk
+ * Setup the given newthread's state
  */
 static int lua_thread_setup (lua_State *L)
 {
     struct lua_thread_start_ctx *ctx;
-    error_t err;
 
     // read the ctx argument off the stack
     if ((ctx = lua_touserdata(L, 1)) == NULL)
-        luaL_error(L, "lua_touserdata");
+        return luaL_error(L, "lua_touserdata");
 
     // push the thread state as the key
     lua_pushthread(L);
@@ -92,50 +89,67 @@
     // clean up the stack
     lua_pop(L, 1);
     
-    // load the chunk as a lua function into the thread's state
-    if (nexus_lua_error(L, luaL_loadstring(L, ctx->chunk), &err))
-        luaL_error(L, "%s", error_msg(&err));
-    
-    // initial resume on the loaded chunk
-    if (lua_thread_resume(L, ctx->thread, &err))
-        luaL_error(L, "%s", error_msg(&err));
-
     // ok
     return 0;
 }
 
+/**
+ * Create a new thread, set it up, and run the initial resume, returning the boolean result of that
+ * (true for yielded, false for done).
+ */
 static int _lua_thread_start (lua_State *L)
 {
     struct lua_thread_start_ctx *ctx;
     lua_State *TL;
     error_t err;
+    int ret;
 
     // read the ctx argument off the stack
     if ((ctx = lua_touserdata(L, 1)) == NULL)
-        luaL_error(L, "lua_touserdata");
+        return luaL_error(L, "lua_touserdata");
+    else
+        lua_pop(L, 1);
     
+    // check
+    assert(!ctx->thread->L);
+
     // create new thread
     TL = lua_newthread(L);
-
+    
     // create thread and do setup on it
-    if (nexus_lua_error(TL, lua_cpcall(TL, lua_thread_setup, ctx), &err)) {
-        // cleanup stack (ctx, TL, error message)
-        lua_pop(L, 3);
+    if (nexus_lua_error(TL, lua_cpcall(TL, lua_thread_setup, ctx), &err))
+        goto error;
 
-        // pass on error
-        luaL_error(L, "lua_thread_setup: %s", error_msg(&err));
-    }
-        
-    // drop the thread for GC, we don't need it, and also our ctx arg
-    lua_pop(L, 2);
+    // load the chunk as a lua function into the thread's state
+    if (nexus_lua_error(TL, luaL_loadstring(TL, ctx->chunk), &err))
+        goto error;
+
+    // initial resume on the loaded chunk
+    if ((ret = lua_thread_resume(TL, ctx->thread, &err)) < 0)
+        goto error;
+
+    // yielded?
+    if (ret) 
+        // store
+        ctx->thread->L = TL;
     
-    // store
-    ctx->thread->L = TL;
+    // pop thread to release for GC
+    lua_pop(L, 1);
 
     return 0;
+        
+error:
+    // pop thread to release for GC
+    lua_pop(L, 1);
+
+    // drop ref
+    ctx->thread->L = NULL;
+
+    // pass on error
+    return luaL_error(L, "%s", error_msg(&err));
 }
 
-err_t lua_thread_start (struct lua_thread *thread, const char *chunk, error_t *err)
+int lua_thread_start (struct lua_thread *thread, const char *chunk, error_t *err)
 {
     struct lua_thread_start_ctx ctx = { thread, chunk };
 
@@ -144,16 +158,17 @@
 
     // use protected mode to setup
     if (nexus_lua_error(thread->lua->st, lua_cpcall(thread->lua->st, _lua_thread_start, &ctx), err))
-        return ERROR_CODE(err);
+        return -ERROR_CODE(err);
 
-    // ok
-    return SUCCESS;
+    // return true if yielded, false if done
+    return (thread->L != NULL);
 }
 
 void lua_thread_resume_state (lua_State *L)
 {
     struct lua_thread *thread;
     error_t err;
+    int ret;
 
     // lookup the thread's context
     lua_pushthread(L);
@@ -164,8 +179,12 @@
         luaL_error(L, "lua_thread_resume_state: lua_touserdata");
 
     // handle the resume
-    if (lua_thread_resume(L, thread, &err))
+    if ((ret = lua_thread_resume(L, thread, &err)) < 0)
         // notify user
-        lua_thread_done(thread->L, thread, &err);
+        return lua_thread_done(thread->L, thread, &err);
+
+    // finished?
+    if (ret == 0)
+        return lua_thread_done(thread->L, thread, NULL);
 }
 
--- a/src/lua_thread.h	Wed May 20 22:53:05 2009 +0300
+++ b/src/lua_thread.h	Thu May 21 16:22:57 2009 +0300
@@ -15,11 +15,13 @@
 struct lua_thread;
 
 /**
- * Callback for async execution completion.
+ * Callback for async execution completion. The thread will be cleaned up for re-use before this is called.
  *
  * Called with err == NULL for success, error code otherwise.
+ *
+ * XXX: also return execution results (as a lua_State arg)?
  */
-typedef void (*lua_thread_cb) (struct lua_thread *thread, const error_t *err, void *arg);
+typedef void (*lua_thread_cb) (const error_t *err, void *arg);
 
 /**
  * A thread of execution
@@ -51,12 +53,13 @@
  * This should be called from unprotected mode, and the thread must currently not be active.
  *
  * This will load the chunk, create a new lua thread state off the lua_nexus, and then do the initial 'lua_resume' call
- * on the loaded chunk.
+ * on the loaded chunk, to execute up to the first yield or final return.
  *
- * If this process raises an error, it will be returned directly. Otherwise, the thread's callback function will
+ * If this process raises an error, it will be returned directly. If the called chunk simply returns without yielding,
+ * this returns 0, and the callback is *NOT* called. Otherwise, this returns >0 and the thread's callback function will
  * eventually be called once the thread either errors out or finishes execution.
  */
-err_t lua_thread_start (struct lua_thread *thread, const char *chunk, error_t *err);
+int lua_thread_start (struct lua_thread *thread, const char *chunk, error_t *err);
 
 /**
  * Protected-mode function to resume execution of the given lua_State, which must be a lua_State created with