Cannot display "_G.package.loaded" value
madmaxoft opened this issue · 16 comments
When I run my script in the MCServer interpreter ( https://github.com/mc-server/MCServer ) and it hits a breakpoint, there's no way to view the value of package or package.loaded. Instead of the expected table, the watch window shows this text:
"...PathFragment\\\ZeroBraneStudio\\lualibs/mobdebug/mobdebug.lua:178: invalid type in array indexing.\n value is 'string'; 'number' expected.\n"
The same happens when trying to print the value in the Remote console window using =package.loaded command.
When running a script in the regular Lua interpreter, the value is displayed correctly, so this issue does depend on the interpreter.
Running ZBS commit e24ee26 on Windows 7 64-bit, although I remember seeing this issue earlier, perhaps a month or two back, so it's nothing new, I just didn't need it that much so I didn't report it.
Mattes, are you using tolua? The only reference that seems to be relevant that I could find is in tolua sources. The line in mobdebug where it fails is this: if type(mt) == 'table' and (mt.__serialize or mt.__tostring) then, so it seems to be failing on some values that have a metatable that looks like a table (reports type table), but doesn't like retrieving __serialize or __tostring keys from that table (as they are not numbers).
If that's the case, it doesn't seem to be ZBS or Mobdebug related as it's strange to have a table that doesn't allow checking for arbitrary keys (and I'm not sure how I'd fix this).
Yes, MCServer does use tolua for the bindings.
I'd expect the debugger to report an error only on such a sub-table, but still be able to display the parent table's contents. This way, I can't view package.loaded, package and not even _G, all of those report the error. Could the debugger's serialization code be wrapped in a pcall on each sub-table recursion, so that an error in the child table doesn't affect the parent table?
Not sure; this looks like tolua bug/deficiency. I don't mind adding specific code to handle important cases, but in this case pcall would penalize every code path that goes through it (although not many tables probably have metatables).
Did you figure out why you get this error in the first place? Is it incorrect handling of metatables? It looks like you should get the same error in your script (outside of mobdebug/serpent) if you do getmetatable(package.loaded).__tostring, which is strange.
Turns out the package.loaded was a false alarm, that table contained a reference to _G which had the main culprit in it.
I investigated a bit further. I copied the serialization code from mobdebug.lua to my script, so that I could call it, too, and then ran it against _G, while printing the values being serialized.
The problem is in the interaction with tolua. I have a C++ global array exported through it, and tolua generates metamethods for accessing elements in the array using a numerical index. MobDebug, however, tries to index using a string, __tostring. This is rejected with the error by the bindings code, and I'm pretty sure that's not a bug in the bindings code - why would you index a C++ array with anything else than a number.
Is the pcall penalization that much of a problem?
I have a C++ global array exported through it, and tolua generates metamethods for accessing elements in the array using a numerical index. MobDebug, however, tries to index using a string, __tostring.
That's the strange thing: mobdebug tries to access __tostring on the metatable, not on the original table! Why would tolua provide a metatable that can only be indexes with numerical indexes?
Is the pcall penalization that much of a problem?
It creates a new context, so the original can be properly restored on a failure, so it's definitely more expensive than a normal function call; by how much it's difficult to say. I'm not against wrapping it into a pcall, but I still want to better understand first what's happening...
The problem is that the metatable for that exported symbol is equal to the table itself (t == mt in the Serpent code). I'm not an expert in Lua so I'm not sure what that is used for exactly, but I think I've read something about this when emulating object-oriented programming with Lua, so perhaps tolua is following that.
Right, but I'm still confused: how is this tolua "trick" supposed to work with all other metatable-related functions in Lua? Sure some of them would need to use __tostring or something similar? Would using rawget help in this case?
To be honest, I have no idea how this works, this is too low-level for me. I suppose this number-restricted metatable is used only for the arrays, that would make some sense.
If you want me to test some code changes in Serpent, you'd need to be a bit more specific about what to modify in order to test.
You don't need to change Serpent; I'd be interested to see if the following fragment triggers the error:
-- assuming your Array variable is called array
print(rawget(getmetatable(array), "__tostring")) -- should work
print(pcall(function() return getmetatable(array).__tostring end)) -- should work
print(getmetatable(array).__tostring) -- should failExactly as you expected. The first line prints nil, the second line prints the error message (invalid type in array indexing...) and the third line fails with the same error message.
Good; thank you for the update. I can use rawget then in the following way:
diff --git a/lualibs/mobdebug/mobdebug.lua b/lualibs/mobdebug/mobdebug.lua
index 89d6be4..32a3b1d 100644
--- a/lualibs/mobdebug/mobdebug.lua
+++ b/lualibs/mobdebug/mobdebug.lua
@@ -175,9 +175,9 @@ local function s(t, opts)
if seen[t] then -- already seen this element
sref[#sref+1] = spath..space..'='..space..seen[t]
return tag..'nil'..comment('ref', level) end
- if type(mt) == 'table' and (mt.__serialize or mt.__tostring) then -- knows how to serialize itself
+ if type(mt) == 'table' and (rawget(mt, "__serialize") or rawget(mt, "__tostring")) then -- knows how to serialize itself
seen[t] = insref or spath
- if mt.__serialize then t = mt.__serialize(t) else t = tostring(t) end
+ if rawget(mt, "__serialize") then t = mt.__serialize(t) else t = tostring(t) end
ttype = type(t) end -- new value falls through to be serialized
if ttype == "table" then
if level >= maxl then return tag..'{}'..comment('max', level) endThe main flipside is that it only checks the metatable and it may have its own metatables, which are not checked. The alternative using pcall would be adding and pcall(function() mt.__tostring end) to the if statement in question (as you were suggesting).
Yes, this change has fixed the issue for me.
@madmaxoft, can you also check the following patch:
diff --git a/lualibs/mobdebug/mobdebug.lua b/lualibs/mobdebug/mobdebug.lua
index 89d6be4..a1c7dd5 100644
--- a/lualibs/mobdebug/mobdebug.lua
+++ b/lualibs/mobdebug/mobdebug.lua
@@ -175,7 +175,8 @@ local function s(t, opts)
if seen[t] then -- already seen this element
sref[#sref+1] = spath..space..'='..space..seen[t]
return tag..'nil'..comment('ref', level) end
- if type(mt) == 'table' and (mt.__serialize or mt.__tostring) then -- knows how to serialize itself
+ if type(mt) == 'table' and pcall(function() return mt.__tostring end)
+ and (mt.__serialize or mt.__tostring) then -- knows how to serialize itself
seen[t] = insref or spath
if mt.__serialize then t = mt.__serialize(t) else t = tostring(t) end
ttype = type(t) end -- new value falls through to be serializedYes, this works as well.
@madmaxoft, could you also try this more complete fix? Thanks!
diff --git a/lualibs/mobdebug/mobdebug.lua b/lualibs/mobdebug/mobdebug.lua
index 99e8e4a..d44c8f9 100644
--- a/lualibs/mobdebug/mobdebug.lua
+++ b/lualibs/mobdebug/mobdebug.lua
@@ -127,7 +127,7 @@ end
local function q(s) return s:gsub('([%(%)%.%%%+%-%*%?%[%^%$%]])','%%%1') end
local serpent = (function() ---- include Serpent module for serialization
-local n, v = "serpent", 0.28 -- (C) 2012-15 Paul Kulchenko; MIT License
+local n, v = "serpent", 0.281 -- (C) 2012-15 Paul Kulchenko; MIT License
local c, d = "Paul Kulchenko", "Lua serializer and pretty printer"
local snum = {[tostring(1/0)]='1/0 --[[math.huge]]',[tostring(-1/0)]='-1/0 --[[-math.huge]]',[tostring(0/0)]='0/0'}
local badtype = {thread = true, userdata = true, cdata = true}
@@ -151,7 +151,7 @@ local function s(t, opts)
local function safestr(s) return type(s) == "number" and tostring(huge and snum[tostring(s)] or s)
or type(s) ~= "string" and tostring(s) -- escape NEWLINE/010 and EOF/026
or ("%q"):format(s):gsub("\010","n"):gsub("\026","\\026") end
- local function comment(s,l) return comm and (l or 0) < comm and ' --[['..tostring(s)..']]' or '' end
+ local function comment(s,l) return comm and (l or 0) < comm and ' --[['..select(2, pcall(tostring, s))..']]' or '' end
local function globerr(s,l) return globals[s] and globals[s]..comment(s,l) or not fatal
and safestr(select(2, pcall(tostring, s))) or error("Can't serialize "..tostring(s)) end
local function safename(path, name) -- generates foo.bar, foo[3], or foo['b a r']
@@ -175,7 +175,9 @@ local function s(t, opts)
if seen[t] then -- already seen this element
sref[#sref+1] = spath..space..'='..space..seen[t]
return tag..'nil'..comment('ref', level) end
- if type(mt) == 'table' and (mt.__serialize or mt.__tostring) then -- knows how to serialize itself
+ -- protect from those cases where __tostring may fail
+ if type(mt) == 'table' and pcall(function() return mt.__tostring and mt.__tostring(t) end)
+ and (mt.__serialize or mt.__tostring) then -- knows how to serialize itself
seen[t] = insref or spath
if mt.__serialize then t = mt.__serialize(t) else t = tostring(t) end
ttype = type(t) end -- new value falls through to be serializedYes, that fixes the issue for me, too.