Merge pull request #335 from AndrewTsao/master

Hold coroutine when handler created in coroutine, fix #319
luvit · Aug 30, 2012 · 1f9c272 · 1f9c272
2 parents f6fccfc + d016e25
commit 1f9c272
Show file tree

Hide file tree

Showing 3 changed files with 114 additions and 1 deletion.
diff --git a/src/utils.c b/src/utils.c
@@ -180,7 +180,7 @@ uv_loop_t* luv_get_loop(lua_State *L) {
 
 /* Initialize a new lhandle and push the new userdata on the stack. */
 luv_handle_t* luv_handle_create(lua_State* L, size_t size, const char* type) {
-
+  lua_State* mainthread;
   /* Create the userdata and set it's metatable */
   luv_handle_t* lhandle = (luv_handle_t*)lua_newuserdata(L, sizeof(luv_handle_t));
 
@@ -197,6 +197,15 @@ luv_handle_t* luv_handle_create(lua_State* L, size_t size, const char* type) {
   lhandle->handle->data = lhandle; /* Point back to lhandle from handle */
   lhandle->refCount = 0;
   lhandle->L = L;
+
+  /* if handle create in a coroutine, we need hold the coroutine */
+  mainthread = luv_get_main_thread(L);
+  if (L != mainthread) { 
+    lua_pushthread(L);
+    lhandle->threadref = luaL_ref(L, LUA_REGISTRYINDEX);
+  } else {
+    lhandle->threadref = LUA_NOREF;
+  }
   lhandle->ref = LUA_NOREF;
   lhandle->type = type;
   return lhandle;
@@ -243,6 +252,10 @@ void luv_handle_unref(lua_State* L, luv_handle_t* lhandle) {
   /* If it's now inactive, clear the ref */
   if (!lhandle->refCount) {
     luaL_unref(L, LUA_REGISTRYINDEX, lhandle->ref);
+    if (lhandle->threadref != LUA_NOREF) {
+      luaL_unref(L, LUA_REGISTRYINDEX, lhandle->threadref);
+      lhandle->threadref = LUA_NOREF;
+    }
     lhandle->ref = LUA_NOREF;
 /*    printf("makeWeak\t%s lhandle=%p handle=%p\n", lhandle->type, lhandle, lhandle->handle);*/
   }
@@ -271,3 +284,11 @@ ares_channel luv_get_ares_channel(lua_State *L) {
   lua_pop(L, 1);
   return channel;
 }
+
+lua_State* luv_get_main_thread(lua_State *L) {
+  lua_State *main_thread;
+  lua_getfield(L, LUA_REGISTRYINDEX, "main_thread");
+  main_thread = lua_tothread(L, -1);
+  lua_pop(L, 1);
+  return main_thread;
+}
diff --git a/src/utils.h b/src/utils.h
@@ -38,6 +38,7 @@ uv_loop_t* luv_get_loop(lua_State *L);
 
 void luv_set_ares_channel(lua_State *L, ares_channel channel);
 ares_channel luv_get_ares_channel(lua_State *L);
+lua_State* luv_get_main_thread(lua_State *L);
 
 
 void luv_push_async_error(lua_State* L, uv_err_t err, const char* source, const char* path);
@@ -58,6 +59,13 @@ typedef struct {
   uv_handle_t* handle; /* The actual uv handle. memory managed by luv */
   int refCount;        /* a count of all pending request to know strength */
   lua_State* L;        /* L and ref together form a reference to the userdata */
+  int threadref;       /* if handle is created in a coroutine(not main thread), threadref is
+                          the reference to the coroutine in the Lua registery. 
+                          we release the reference when handle closed. 
+                          if handle is created in the main thread, threadref is LUA_NOREF.
+                          we must hold the coroutine, because in some cases(see issue #319) that the coroutine 
+                          referenced by nothing and would collected by gc, then uv's callback touch an 
+                          invalid pointer. */
   int ref;             /* ref is null when refCount is 0 meaning we're weak */
   const char* type;
 } luv_handle_t;

diff --git a/tests/test-fiber-bug319.lua b/tests/test-fiber-bug319.lua
@@ -0,0 +1,84 @@
+--[[
+
+Copyright 2012 The Luvit Authors. All Rights Reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License")
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS-IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+
+--]]
+
+require("helper")
+local fiber = require("fiber")
+local timer = require("timer")
+local count = 0
+
+fiber.new(function(wrap, wait)
+  for i = 1, 10 do
+		timer.setTimeout(100, function()
+			count = count + 1
+		end)
+  end
+end)
+
+collectgarbage()
+
+local net = require('net')
+local PORT = process.env.PORT or 10082
+local messages = {'a','b','c'}
+
+fiber.new(function(wrap, wait)
+  local server
+  server = net.createServer(function(client)
+    client:on("data", function(data)
+      fiber.new(function(wrap, wait)
+        for _, message in ipairs(messages) do
+          wait(function(resume)
+            client:write(message, function(err)
+              timer.setTimeout(1, resume)
+            end)
+          end)
+        end
+        client:destroy()
+        server:close()
+      end)
+    end)
+  end)
+  server:listen(PORT, "127.0.0.1")
+  server:on("error", function(err)
+    assert(false)
+  end)
+  wait(function(resume) process.nextTick(resume) end)
+end)
+collectgarbage()
+
+local client = require("uv").Tcp:new()
+client:connect("127.0.0.1", PORT)
+client:on("connect", function()
+  client:write("hi")
+  local received = {} 
+  client:on("data", function(data)
+    received[#received+1] = data
+    if data == "3:c" then
+      client:close()
+      assert(received[1] == messages[1])
+      assert(received[2] == messages[2])
+      assert(received[3] == messages[3])
+    end
+  end)
+end)
+client:on("error", function()
+  assert(false)
+end)
+
+process:on('exit', function()
+  assert(count == 10)
+end)