Improve behaviour of GC'd objects

Before this commit: When we are GC'ing an object, we clear its metatable, since
otherwise crashes could occur in various places. This means that if someone
tries to use such an object, they get an unhelpful error message like "attempt
to index userdata object" and they don't understand what the problem is. Also,
this means that foo.valid does not actually work after GC.

This commit changes this behaviour. Instead of setting an empty metatable, we
now create a metatable with an __index and __newindex method. These metamethods
produce better error messages that they sat the underlying object was already
garbage collected. Better yet, the __index metamethod makes foo.valid be false
instead of causing an error, so that the existing machinery for detecting
invalid objects continues to work.

This commit also adds a functional test that verifies this behaviour.

Signed-off-by: Uli Schlachter <psychon@znc.in>

common/luaclass.c

@@ -163,6 +163,32 @@ luaA_class_add_property(lua_class_t *lua_class,
                                     });
 }
 
+/** Newindex meta function for objects after they were GC'd.
+ * \param L The Lua VM state.
+ * \return The number of elements pushed on stack.
+ */
+static int
+luaA_class_newindex_invalid(lua_State *L)
+{
+    return luaL_error(L, "attempt to index an object that was already garbage collected");
+}
+
+/** Index meta function for objects after they were GC'd.
+ * \param L The Lua VM state.
+ * \return The number of elements pushed on stack.
+ */
+static int
+luaA_class_index_invalid(lua_State *L)
+{
+    const char *attr = luaL_checkstring(L, 2);
+    if (A_STREQ(attr, "valid"))
+    {
+        lua_pushboolean(L, false);
+        return 1;
+    }
+    return luaA_class_newindex_invalid(L);
+}
+
 /** Garbage collect a Lua object.
  * \param L The Lua VM state.
  * \return The number of elements pushed on stack.
@@ -181,8 +207,13 @@ luaA_class_gc(lua_State *L)
             class->collector(item);
     /* Unset its metatable so that e.g. luaA_toudata() will no longer accept
      * this object. This is needed since other __gc methods can still use this.
+     * We also make sure that `item.valid == false`.
      */
     lua_newtable(L);
+    lua_pushcfunction(L, luaA_class_index_invalid);
+    lua_setfield(L, -2, "__index");
+    lua_pushcfunction(L, luaA_class_newindex_invalid);
+    lua_setfield(L, -2, "__newindex");
     lua_setmetatable(L, 1);
     return 0;
 }

tests/test-use-after-gc.lua

@@ -0,0 +1,31 @@
+-- Test behaviour of C objects after they were finalized
+
+local runner = require("_runner")
+
+local done
+do
+    local obj
+    local func = function()
+        assert(obj.valid == false)
+        assert(not pcall(function()
+            print(obj.visible)
+        end))
+        assert(not pcall(function()
+            obj.visible = true
+        end))
+        done = true
+    end
+    if _VERSION >= "Lua 5.2" then
+        setmetatable({}, { __gc = func })
+    else
+        local newproxy = newproxy -- luacheck: globals newproxy
+        getmetatable(newproxy(true)).__gc = func
+    end
+    obj = drawin({})
+end
+collectgarbage("collect")
+assert(done)
+
+runner.run_steps({ function() return true end })
+
+-- vim: filetype=lua:expandtab:shiftwidth=4:tabstop=8:softtabstop=4:textwidth=80