luv_fs: fix gc of string during a fs.write

In virgo we hit a bug that only revealed itself when we were writing out to a file over NFS. This is likely because the NFS write out took a relatively long time and revealed a race. Example corrupted text file: # ch0002 tests/tmp/0002.chk OK »A¿7—A ch0003 tests/tmp/0003.chk OK 1337118370 Notice that the fifth line is garbage. That is because the string was getting GC'd before it was written to disk. So, we need to ref the string so this doesn't happen just as we ref the callback function.
luvit · May 15, 2012 · eba20e7 · eba20e7
1 parent 5befa91
commit eba20e7
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 11 deletions.
diff --git a/src/luv_fs.c b/src/luv_fs.c
@@ -176,8 +176,12 @@ void luv_after_fs(uv_fs_t* req) {
   luv_fs_ref_t* ref = req->data;
   lua_State *L = ref->L;
   int argc;
-  lua_rawgeti(L, LUA_REGISTRYINDEX, ref->r);
-  luaL_unref(L, LUA_REGISTRYINDEX, ref->r);
+
+  lua_rawgeti(L, LUA_REGISTRYINDEX, ref->rstr);
+  luaL_unref(L, LUA_REGISTRYINDEX, ref->rstr);
+
+  lua_rawgeti(L, LUA_REGISTRYINDEX, ref->rcb);
+  luaL_unref(L, LUA_REGISTRYINDEX, ref->rcb);
 
   argc = luv_process_fs_result(L, req);
 
@@ -187,16 +191,33 @@ void luv_after_fs(uv_fs_t* req) {
   free(ref); /* We're done with the ref object, free it */
 }
 
-/* Utility for storing the callback in the fs_req token */
-uv_fs_t* luv_fs_store_callback(lua_State* L, int index) {
-
+static luv_fs_ref_t* luv_fs_ref_alloc(lua_State* L) {
   luv_fs_ref_t* ref = malloc(sizeof(luv_fs_ref_t));
   ref->L = L;
-  if (lua_isfunction(L, index)) {
-    lua_pushvalue(L, index); /* Store the callback */
-    ref->r = luaL_ref(L, LUA_REGISTRYINDEX);
-  }
   ref->fs_req.data = ref;
+  ref->rcb = LUA_NOREF;
+  ref->rstr = LUA_NOREF;
+  return ref;
+}
+
+static void luv_fs_ref_callback(luv_fs_ref_t* ref, int index) {
+  if (lua_isfunction(ref->L, index)) {
+    lua_pushvalue(ref->L, index); /* Store the callback */
+    ref->rcb = luaL_ref(ref->L, LUA_REGISTRYINDEX);
+  }
+}
+
+static void luv_fs_ref_string(luv_fs_ref_t* ref, int index) {
+  if (lua_isstring(ref->L, index)) {
+    lua_pushvalue(ref->L, index);
+    ref->rstr = luaL_ref(ref->L, LUA_REGISTRYINDEX);
+  }
+}
+
+/* Utility for storing the callback in the fs_req token */
+uv_fs_t* luv_fs_store_callback(lua_State* L, int index) {
+  luv_fs_ref_t* ref = luv_fs_ref_alloc(L);
+  luv_fs_ref_callback(ref, index);
   return &ref->fs_req;
 }
 
@@ -258,7 +279,10 @@ int luv_fs_write(lua_State* L) {
   off_t offset = luaL_checkint(L, 2);
   size_t length;
   void* chunk = (void*)luaL_checklstring(L, 3, &length);
-  uv_fs_t* req = luv_fs_store_callback(L, 4);
+  luv_fs_ref_t* ref = luv_fs_ref_alloc(L);
+  luv_fs_ref_string(ref, 3);
+  luv_fs_ref_callback(ref, 4);
+  uv_fs_t* req = &ref->fs_req;
   FS_CALL(write, 4, NULL, file, chunk, length, offset);
 }
 

diff --git a/src/luv_fs.h b/src/luv_fs.h
@@ -62,7 +62,8 @@ int luv_fs_fchown(lua_State* L);
 
 typedef struct {
   lua_State* L;
-  int r;
+  int rcb; /* callback ref */
+  int rstr; /* string ref */
   uv_fs_t fs_req;
   void* buf;
 } luv_fs_ref_t;