From 16b92a0614a3a5a669204a75f031d9bf1da34676 Mon Sep 17 00:00:00 2001
From: Emmanuel Lepage Vallee <elv1313@gmail.com>
Date: Sat, 12 Aug 2023 20:30:45 -0700
Subject: [PATCH] wibox: Fix __to_string and garbage collection.

There was no "real" unsolvable GC issues, but at least for Lua
5.2 and 5.3, it wasn't possible to GC a wibox anymore. This
commit unwind the circular references with 3 new weak refs. This
is enough to get a specific test to pass on Lua 5.3. It's voodoo,
but this was actually a pretty bad leak. There's other according
to some test I wrote, but that's one.

As for to_string, it appears to be accidental refactoring oversight.
---
 lib/wibox/drawable.lua |  4 ++++
 lib/wibox/init.lua     | 33 ++++++++++++++++++++++++---------
 2 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/lib/wibox/drawable.lua b/lib/wibox/drawable.lua
index f5e8a6c8e..ae0922647 100644
--- a/lib/wibox/drawable.lua
+++ b/lib/wibox/drawable.lua
@@ -55,6 +55,10 @@ local function get_widget_context(self)
         for k, v in pairs(self._widget_context_skeleton) do
             context[k] = v
         end
+
+        -- Set the metatable to give access to the weak wibox.
+        setmetatable(context, getmetatable(self._widget_context_skeleton))
+
         self._widget_context = context
 
         -- Give widgets a chance to react to the new context
diff --git a/lib/wibox/init.lua b/lib/wibox/init.lua
index 9715a93ee..ddbe83109 100644
--- a/lib/wibox/init.lua
+++ b/lib/wibox/init.lua
@@ -254,7 +254,10 @@ local function setup_signals(w)
     local function clone_signal(name)
         -- When "name" is emitted on wibox.drawin, also emit it on wibox
         obj:connect_signal(name, function(_, ...)
-            w:emit_signal(name, ...)
+            local wb = obj.get_wibox()
+            if wb then
+                wb:emit_signal(name, ...)
+            end
         end)
     end
 
@@ -296,16 +299,30 @@ local function new(args)
     local ret = object()
     local w = capi.drawin(args)
 
+    local weak_wibox = setmetatable({ret}, {__mode = "v"})
+
+    -- Strong ref
     function w.get_wibox()
         return ret
     end
 
     ret.drawin = w
-    ret._drawable = wibox.drawable(w.drawable, { wibox = ret },
+
+    -- Do not pass a strong reference to the wibox, it will confuse the GC.
+    local context_skeleton = setmetatable({}, {
+        __index = function(_, key)
+            if key == "wibox" then return weak_wibox[1] end
+
+            return nil
+        end
+    })
+
+    ret._drawable = wibox.drawable(w.drawable, context_skeleton,
         "wibox drawable (" .. object.modulename(3) .. ")")
 
+    -- Weak ref
     function ret._drawable.get_wibox()
-        return ret
+        return weak_wibox[1]
     end
 
     ret._drawable:_inform_visible(w.visible)
@@ -328,13 +345,7 @@ local function new(args)
     ret:set_fg(args.fg or beautiful.fg_normal)
 
     -- Add __tostring method to metatable.
-    local mt = {}
     local orig_string = tostring(ret)
-    mt.__tostring = function()
-        return string.format("wibox: %s (%s)",
-                             tostring(ret._drawable), orig_string)
-    end
-    ret = setmetatable(ret, mt)
 
     -- Make sure the wibox is drawn at least once
     ret.draw()
@@ -359,6 +370,10 @@ local function new(args)
             else
                 rawset(self, k, v)
             end
+        end,
+        __tostring = function()
+            return string.format("wibox: %s (%s)",
+                                tostring(ret._drawable), orig_string)
         end
     })