Awesome: error in screen.__index : string expected, got userdata

[bew]

Hi,

Just trying git/master for my setup (archlinux packages were way too old with all the joy coming with v3.6)..

I have a crash in a protected_call with awful.autofocus :

2016-10-19 22:09:31 E: Error during a protected call: /home/lesell_b/.awesome-master/lib/awful/autofocus.lua:43: bad argument #2 to '__index' (string expected, got userdata)
stack traceback:
    /home/lesell_b/.awesome-master/lib/gears/protected_call.lua:16: in function </home/lesell_b/.awesome-master/lib/gears/protected_call.lua:15>
    [C]: in metamethod '__index'
    /home/lesell_b/.awesome-master/lib/awful/autofocus.lua:43: in function </home/lesell_b/.awesome-master/lib/awful/autofocus.lua:40>
    [C]: in function 'xpcall'
    /home/lesell_b/.awesome-master/lib/gears/protected_call.lua:36: in function </home/lesell_b/.awesome-master/lib/gears/protected_call.lua:35>
    (...tail calls...)
    /home/lesell_b/.awesome-master/lib/gears/timer.lua:167: in function </home/lesell_b/.awesome-master/lib/gears/timer.lua:165>

The corresponding code is :

local function check_focus_tag(t)
    local s = t.screen
    if (not s) or (not s.valid) then return end
    s = screen[s] -------------------------------------##### THIS LINE
    check_focus({ screen = s })
    if client.focus and screen[client.focus.screen] ~= s then
        local c = aclient.focus.history.get(s, 0, aclient.focus.filter)
        if c then
            c:emit_signal("request::activate", "autofocus.check_focus_tag",
                          {raise=false})
        end
    end
end

It's weird, I can't get it to crash when unmanaging client, but this was happenning before too, it is the same error (you'll see).

As it works well in the default rc.lua, I tried to understand where to problem could come, and finally I don't understand how the default rc.lua could work, I'll explain below.

If I understand well, the screen object (from C api) can be indexed by screen numbers to get the actual corresponding screen object, ex :

local s = 1
local myscreen = screen[s]
-- Here, myscreen is an object for screen n°1 manipulation

First thing I don't understand, is that after a quick look in objects/screen.c in the __index metamethod, the key for __index should be a LUA_TSTRING type, and in the above example, it's a LUA_TNUMBER.. So this shouln't even work at all. But maybe there is some magic in the lua C api for this, I don't know it well.

Assuming this is working, I'm going back to the first code snippet. The variable s (before the crashing line) should be a number, corresponding to the screen number, as it is used as screen[s].
But s = t.screen which is already a screen object, then how could the screen[s] code work ?

As a patch in awful/autofocus.lua :

23c23
<         local c = aclient.focus.history.get(screen[obj.screen], 0, aclient.focus.filter)

---
>         local c = aclient.focus.history.get(obj.screen, 0, aclient.focus.filter)
43d46
<     s = screen[s]
45c46
<     if client.focus and screen[client.focus.screen] ~= s then

---
>     if client.focus and client.focus.screen ~= s then

For me this is what crashes.. Am I missing something ? thanks

---

Edit: The error happened again, but now in gears/wallpaper.lua#L24 where the code does :

local function get_screen(s)
    return s and screen[s]        ----#### THIS LINE
end

[psychon]

This comment explains some things unrelated to your issue. I'll post a separate comment about this issue later.

and finally I don't understand how the default rc.lua could work, I'll explain below.

If I understand well, the screen object (from C api) can be indexed by screen numbers to get the actual corresponding screen object, ex :

Well, no. It can be a number (screen index), It can be a string (Output name of a RandR output, e.g. "HDMI1" or "primary" to get the primary screen) and it can be a screen object (in which case just this screen object is returned again).

First thing I don't understand, is that after a quick look in objects/screen.c in the __index metamethod, the key for __index should be a LUA_TSTRING type, and in the above example, it's a LUA_TNUMBER.. So this shouln't even work at all. But maybe there is some magic in the lua C api for this, I don't know it well.

The __index metamethod for screens is luaA_screen_module_index (naming scheme: luaA_screen_index would be the metamethod for individual screens while the one with module_ in its name is for the global screen table). This function first checks if it was called with a string (lua_type(L, 2) == LUA_TSTRING) in which case it looks up a screen by RandR output name (or handles "primary"). Then it calls luaA_checkscreen() to get the "normal" screen handling. This function then handles numbers (screen index) and screen objects.

So screen[s] should always be safe whenever s is something "screen-like" (index, name or screen object).

The reason that lots of code does (seemingly unnecessarily) screen[s] is backwards compatibility: Previously we were only using screen indicies (numbers), now we try to use screen objects exclusively. Doing unnecessary screen[s]-calls should keep the old code working.

[psychon]

I have a crash in a protected_call with awful.autofocus :
2016-10-19 22:09:31 E: Error during a protected call: /home/lesell_b/.awesome-master/lib/awful/autofocus.lua:43: bad argument #2 to '__index' (string expected, got userdata)
stack traceback:
/home/lesell_b/.awesome-master/lib/gears/protected_call.lua:16: in function
[C]: in metamethod '__index'
/home/lesell_b/.awesome-master/lib/awful/autofocus.lua:43: in function
[C]: in function 'xpcall'
/home/lesell_b/.awesome-master/lib/gears/protected_call.lua:36: in function
(...tail calls...)
/home/lesell_b/.awesome-master/lib/gears/timer.lua:167: in function

So, we are in a protected call (gears.timer does protected calls, so the following tail call should be an unimportant function in gears.protected_call and then the actual protected call handling).
This calls awful.autofocus which then tries to handle a tag-based autofocus (check_focus_tag). The tag has a valid screen (check in line 42: if (not s) or (not s.valid) then return end), but then doing screen[s] errors out (which is, by the way, unnecessary: the s.valid in the line above will error out if s is a string or integer).
(The following call to gears.protected_call is unrelated: This is just the error-printing function and it would be better if this didn't appear in the traceback. I'll fix this.)

The error message is "string expected, got userdata". This is weird. luaA_screen_module_index shouldn't cause such a message. If first checks if the argument is of type string or number and then calls luaA_checkudata which expects a screen object. If the passed-in argument is not a screen object, the error message should say "screen expected, got userdata" (implemented in luaA_checkudata and luaA_typeerror). So... your error message makes no sense to me.

My best assumption would be: Something overwrote the global screens-table, but this should be unlikely. On the other hand, you think that only screens are valid indicies here, so: Do you have any magic in your config that replaces the screens-table?

Edit: The error happened again, but now in gears/wallpaper.lua#L24 where the code does :

Do you have a traceback for this? Why is a wallpaper being set?

Is the error perhaps related to adding or removing screens (or enabling/disabling them via xrandr)?

[bew]

Thanks for your first message, it's clear now !

My best assumption would be: Something overwrote the global screen-table, but this should be unlightly.

This is what I thought also, but every time there is a screen = something, it's always a local variable or a table key = value, so it shouldn't do any problem..

On the other hand, you think that only screens are valid indicies here, so: Do you have any magic in your config that replaces the screen-table?

Absolutly not.. (edit: well there is but it's not myconfig, see the next comment)

Edit: The error happened again, but now in gears/wallpaper.lua#L24 where the code does :

Do you have a traceback for this? Why is a wallpaper being set?

Sorry I didn't mentionned, I have a keybind that trigger a random wallpaper being set.

What's really weird, is that when I reload my config, everything works fine, and I can trigger the wallpaper change any number of time.. but after a while (I can't say how many), when I retry, I get that error...

I don't know if this could be the GC that takes too much, and I bet the screen-table cannot be GCed...

[bew]

After some more in-depth research I've found a flaw :

I'm using scratch, from https://awesome.naquadah.org/wiki/Scratchpad_manager to have a drop-down terminal, I think I've found where the problem is :

In drop.lua :

function toggle(prog, opt)
    vert   = opt.vert   or "top"
    horiz  = opt.horiz  or "center"
    width  = opt.width  or 1
    height = opt.height or 0.25
    sticky = opt.sticky or false

    ----------------------- HERE ----------------------
    screen = opt.screen or capi.mouse.screen
    -- it overwrite the global screen object

    -- rest of the code
end

I knew I had to rewrite that module, but never thought it would crash everything like this...
I'll make a fix, and see if it was it, but there is a serious problem with that code, for sure !

[psychon]

Oh, that would fit. Indexing a screen would produce the error message that you see ("string expected").