Mudlet freeze calling display() on adjustable container

[Kebap]

Brief summary of issue

Mudlet freezes a few seconds after typing the wrong command

Steps to reproduce the issue

Discord user chad explains:

1. so if i make an adjustable container, called testContainer
   testContainer = Adjustable.Container:new({name = "ARS main window"})
2. and I do lua testContainer in my cmd prompt
3. it freezes mudlet and then gives me a memory error

Error output

[ERROR:] Objekt:<run lua code> Funktion:<Alias4>
        <not enough memory>

Extra information, such as Mudlet version, operating system and ideas for how to solve / implement:

that's not just on my shit laptop, or just 1 profile.
that's on my laptop and PC (16gb ram)
on new profiles, old profiles
Mudlet 4.10.1

[SlySven]

That behaviour sound consistent with an infinite recursion happening in the Lua code somehow...

[vadi2]

Labelling as high because it's poor behaviour, we expect Mudlet to do better here.

[SlySven]

Would it be fair to mark this as likely to be a lua only issue?

[Edru2]

That behaviour sound consistent with an infinite recursion happening in the Lua code somehow...

Yes that is the reason. As Geyser always keeps a reference of the parent in container (myGeyserElement.container).
This is a known issue with display and is also the reason that the creator of Geyser made the Geyser.display function (11 years ago 👀 ).

Mudlet/src/mudlet-lua/lua/geyser/GeyserUtil.lua

Line 35 in d84f0b5

function Geyser.display (table)

So instead of using display a workaround would be to use Geyser.display.

[vadi2]

Can we identify a Geyser object if one is given? Could make display() call that function in that case automatically.

[Kebap]

Maybe we can help display to never run into these infinite loops again?
For example, keep a list of all tables displayed already, and not display a full second version of one again.

[Edru2]

After diggin a bit I found out that prettywrite has a failsafe against infinite loops in it but it fails for Geyser in some instances (I'm not sure why)
for example:

test = {}
test[1] = test
display(test)

Won't cause an infinite loop.

I tried to comment out

Mudlet/src/mudlet-lua/lua/DebugTools.lua

Line 185 in 4042ac7

tables[t] = nil

and that seems to work but I'm not sure if that could cause other issues and why it was there in the first place.

[vadi2]

This is the reason: lunarmodules/Penlight#38

[vadi2]

Unrelated, it looks like there's an improvement for consistent key order we could take advantage of: lunarmodules/Penlight#293

[Edru2]

After digging further I noticed that the issue mainly occurs if nested Labels are used (Adjustable Container right click menu uses a nested Label)
I personally still think that commenting

Mudlet/src/mudlet-lua/lua/DebugTools.lua

Line 185 in 4042ac7

tables[t] = nil

out is a viable solution to the issue.

I tested https://github.com/kikito/inspect.lua which does more or less the same as prettywrite and they solve this issue the same way (as of it would be if the tables[t] part was commented out) but with the difference that every duplicated table gets an id instead of a generic <cycle>

I also found a similar issue using the second nestable labels example from the Geyser Wiki (the one with 'mainlabel') https://wiki.mudlet.org/index.php?title=Manual:Geyser#Demo -> hover over the 'Hover there part' and then use lua display(Geyser) still causes an stackoverflow error (not occurring if using inspect)

Maybe switching from prettywrite to inspect is something to think about.

[vadi2]

We don't have a particular reason to stick to one - if the other is better, let's go for it. How do the two compare, could you post before/afters?

[demonnic]

regarding consistent key order, could just change the use of pairs to spairs and it should handle all that. That's essentially all that code snippet linked does, we've just abstracted it for use anywhere as spairs.

[Edru2]

We don't have a particular reason to stick to one - if the other is better, let's go for it. How do the two compare, could you post before/afters?

It is pretty much doing the same but I don't know if it is 'better' at least it doesn't crash in Geyser (which makes it better in my opinion) ;)
I'll open a PR