lunarmodules / busted

Elegant Lua unit testing.
https://lunarmodules.github.io/busted/
MIT License
1.43k stars 186 forks source link

context: "error in __gc metamethod" with lgi #443

Open blueyed opened 9 years ago

blueyed commented 9 years ago

In the awesome window manager's test suite, I've seen weird and non-reproducible crashes with Lua 5.2 and 5.1.

With Lua 5.3 it's reproducible and a bit clearer, which might be also because of other package versions (of busted and lgi), but I've investigated using "master" for both of them):

> Lua 5.3: error in __gc metamethod
> (/usr/local/share/lua/5.3/busted/context.lua:5: bad argument #1 to 'for
> iterator' (expected lgi.callable, got userdata))

Also when using just print("done") there's this error:

/usr/local/bin/lua: error in __gc metamethod
(/usr/local/share/lua/5.3/busted/context.lua:8: bad argument #1 to 'print'
(lgi.record expected, got userdata))

```diff
diff --git i/busted/context.lua w/busted/context.lua
index 2a8bc6b..63a23ff 100644
--- i/busted/context.lua
+++ w/busted/context.lua
@@ -3,7 +3,9 @@ local tablex = require 'pl.tablex'
 local function save()
   local g = {}
   for k,_ in next, _G, nil do
+    print(k)
     g[k] = rawget(_G, k)
+    print("done")
   end
   return {
     gmt = debug.getmetatable(_G),

(With Lua 5.2 it was something like:

/usr/bin/lua5.2: error in __gc metamethod
(/usr/local/share/lua/5.2/busted/status.lua:2: bad argument #1 to '?'
(lgi.record expected, got userdata))

or

/usr/bin/lua5.2: error in __gc metamethod
(/usr/local/share/lua/5.2/busted/status.lua:16: bad argument #1 to '?'
(lgi.record expected, got userdata))

)

The errors are coming from lgi's __gc methods, e.g. https://github.com/pavouk/lgi/blob/1c5d85058f48572723442d940a4e7cfba0074b7f/lgi/callable.c#L611-630.

The following patch/hack fixes it (suggested by @psychon):

 ```diff
 busted/context.lua | 3 +++
 1 file changed, 3 insertions(+)

diff --git i/busted/context.lua w/busted/context.lua
index 2a8bc6b..647751a 100644
--- i/busted/context.lua
+++ w/busted/context.lua
@@ -1,5 +1,8 @@
 local tablex = require 'pl.tablex'

+local lgi = require("lgi")
+package.preload.lgi = function() return lgi end
+
 local function save()
   local g = {}
   for k,_ in next, _G, nil do


It seems like busted's `save` and `restore` methods do not preserve the metatables with the info that e.g. lgi.callable expects?!
(https://github.com/pavouk/lgi/blob/1c5d85058f48572723442d940a4e7cfba0074b7f/lgi/callable.c#L610-L655)
psychon commented 9 years ago

A "simple" reproducer:

mkdir -p spec ; (cd spec ; for x in $(seq 1 10) ; do echo 'describe("test", function() it("require", function() require("lgi").cairo.Matrix.create_identity() end) end)' > test${x}_spec.lua ; done ) ; busted

Output for me:

●●●●●/usr/bin/lua: error in __gc metamethod (bad argument #1 to '?' (lgi.record expected, got userdata))

If you additionally stick in a collectgarbage("collect") into the test method, you get:

●●●✱
3 successes / 0 failures / 1 error / 0 pending : 0.042503 seconds

Error → spec/test7_spec.lua @ 1
test require
spec/test7_spec.lua:1: attempt to call a nil value (field 'create_identity')
/usr/bin/lua: (error object is a nil value)
stack traceback:
    [C]: in function 'error'
    /home/psychon/.luarocks/share/lua/5.3/busted/compatibility.lua:36: in function 'busted.compatibility.exit'
    /home/psychon/.luarocks/share/lua/5.3/busted/runner.lua:185: in function 'busted.runner'
    /home/psychon/.luarocks/lib/luarocks/rocks/busted/scm-0/bin/busted:3: in main chunk
    [C]: in ?

And only the last test fails, the first three succeeds (what happened to the other 6 tests?).

psychon commented 9 years ago

One more thing:

Sticking a print("lgi") into lgi.lua shows that lgi is loaded again for each test. Then I added this hack to the beginning of bin/busted:

local req = require
function require(a, ...)
    local result = req(a, ...)
    if a == "lgi.core" then print(a, result) end
    return result
end

Output is (shortened):

lgi.core    table: 0x1be1ab0
[...]
lgi.core    table: 0x1be1ab0
●lgi.core table: 0x1cbf670
[...]
lgi.core    table: 0x1cbf670
●lgi.core table: 0x1d95630
[...]
lgi.core    table: 0x1d95630
●lgi.core table: 0x1e6b0c0
lgi.core    table: 0x1e6b0c0
[...]

So the C part of lgi is really loaded multiple times (different tables are returned by require). Note that due to linking to GLib, this is not safe and supposedly breaks things: https://github.com/pavouk/lgi/blob/1c5d85058f48572723442d940a4e7cfba0074b7f/lgi/core.c#L663-L670

DorianGray commented 9 years ago

Ah, so what you want to do is pass in --no-auto-insulate and run the tests. You can put that flag in your .busted file to make it always that way.

DorianGray commented 9 years ago

Alternatively, I believe there's a way to include a file pre-test that sets up globals for c related dependencies. A search of our issues would probably turn up something to that effect.

blueyed commented 9 years ago

Thanks for the support. It was a very tricky issue.

It's fixed for awesome now in https://github.com/awesomeWM/awesome/commit/b75f374d9c301324b5894647deaa021e3eaaa782.

Please feel free to close the issue. But maybe this case could be handled better in busted, e.g. by pointing at the helper/preload feature/setting?

DorianGray commented 9 years ago

Yeah, we will update the docs before we release 2.0 proper. I'll keep this open to remind us.