Details! Damage Meter

Details! Damage Meter

243M Downloads

C_Stack_Overflow in dropdown elements, because they keep calling themselves inside their own metafunctions

Undisclosed-Information opened this issue · 4 comments

commented

Bug

Please consider renaming two metafunctions in panel.lua#L370 which are calling themselves over and over and over again! because they are named the same as their internal counterpart:

function PanelMetaFunctions:Show()
     self.frame:Show() --let me call myself again!
end

function PanelMetaFunctions:Hide()
     self.frame:Hide() --let me call myself again!
end

This specific issue is showing its bad habits inside window_options2_sections.lua#L3863 when it is attempting to switch profiles. This bug is probably also leaking across other elements that are made through the detailsFramework.NewPanel function that is assigning said metafunctions in panel.lua#L671.

How to reproduce

just keep switching back and forth between profiles (or keep selecting the same profile) in Options Panel > Profiles > Select Profile.
At some point LUA will complain with a nasty C stack overflow.

Solution

Rename said metafunctions so that elements cant call themselves mutliple times.

function PanelMetaFunctions:Show2()
     self.frame:Show() --unable to call Show2!
end

function PanelMetaFunctions:Hide2()
     self.frame:Hide() --unable to call Hide2!
end
commented

thanks for reporting!

commented

I do not see a point at which this code would create an overflow. The metatable is on the table containing the frame and not the frame itself. Calling hide on the frame will not trigger that function to run again.

commented

I was close... but not close enough.

so the actual issue is caused from an [OnHide] handler that is "basically" expanded infinitely through a HookScript function inside the plugins.lua file. It all starts here:

  1. the creation of the panel (with all its base panel-handlers and panel-metafunctions): window_options2.lua#L39
function Details.options.InitializeOptionsWindow(instance)
	local DetailsOptionsWindow = DF:NewPanel(UIParent, _, "DetailsOptionsWindow", _, 897, 592)
        .....
  1. then we move to the plugin embed of our panel frame: window_options2.lua#L46
_G.DetailsPluginContainerWindow.EmbedPlugin(f, f, true)
  1. which updates the [OnHide] handler in this function: plugins.lua#L717
f.RefreshFrame(frame)
  1. well, its not really updating it, its extending it by hooking the same function over and over: plugins.lua#L657. When we open and close dropdown elements, this function is called multiple times for the same frame, making us reach our stack limit, because we cant keep "hooking" stuff forever and ever ❤️
function f.RefreshFrame (frame)
     frame:EnableMouse(false)
     frame:SetSize(f.FrameWidth, f.FrameHeight)
     frame:SetScript("OnMouseDown", nil)
     frame:SetScript("OnMouseUp", nil)
     --frame:SetScript("OnHide", on_hide)
     frame:HookScript("OnHide", on_hide) --The root cause of the C_Stack_Overflow
     frame:ClearAllPoints()
     PixelUtil.SetPoint(frame, "topleft", f, "topleft", 0, 0)
     frame:Show()
end

Solution

Geting rid of hooking scripts in a function that is called an excessive amount of time is a step in the right direction, but perhaps a rewrite of some parts of the logic in plugins.lua might not be a bad idea.

For example in the use-case I described with dropdown elements, the ClosePlugin function is double / triple dipping on frames that were already processed. More [Hide] calls, resulting in more [OnHide] calls, making new [Hide] calls... you get the point. Thats what I noticed when I first reported the issue.

commented

Oh yes absolutely. I can see some of this stuff. I already was investigating the issue but got sidetracked by another semi issue :P. Thanks for the writeup.