From 791794fa42d22b582191c7de4676bbfd45246198 Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Sun, 24 Jan 2016 13:45:06 +0100 Subject: [PATCH 1/4] awful.titlebar: Fix memory leak with clients Any awful.titlebar.widget.button widget (e.g. floatingbutton or closebutton) decides on the currently visible symbol based on several factors. One of them is "is the client currently focused?" and thus the button has to be updated when the client is focused/unfocused. The way the code did this was to use client.connect_signal("focus", f) and client.connect_signal("unfocus", f). However, these signals are never disconnected and kept alive forever. The callback function had a strong reference to the client (as an upvalue) and thus this also prevented the client from being garbage collected. Fix this by using c:connect_signal("focus/unfocis", f) instead. These kind of signals are only kept alive by the client object and don't prevent it from being garbage collected. This fixes the new test that the previous commit added. Signed-off-by: Uli Schlachter --- lib/awful/titlebar.lua | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/awful/titlebar.lua b/lib/awful/titlebar.lua index 41ec779b5..dfe322626 100644 --- a/lib/awful/titlebar.lua +++ b/lib/awful/titlebar.lua @@ -238,11 +238,8 @@ function titlebar.widget.button(c, name, selector, action) -- We do magic based on whether a client is focused above, so we need to -- connect to the corresponding signal here. - local function focus_func(o) - if o == c then update() end - end - capi.client.connect_signal("focus", focus_func) - capi.client.connect_signal("unfocus", focus_func) + c:connect_signal("focus", update) + c:connect_signal("unfocus", update) return ret end From 69d96ba34d69708001adb0bf0a542d5e04b1bc7a Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Sun, 24 Jan 2016 16:27:31 +0100 Subject: [PATCH 2/4] awful.titlebar: Unreference titlebars in unmanage Signed-off-by: Uli Schlachter --- lib/awful/titlebar.lua | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/awful/titlebar.lua b/lib/awful/titlebar.lua index dfe322626..d434a0c6d 100644 --- a/lib/awful/titlebar.lua +++ b/lib/awful/titlebar.lua @@ -296,6 +296,10 @@ function titlebar.widget.stickybutton(c) return widget end +client.connect_signal("unmanage", function(c) + all_titlebars[c] = nil +end) + return setmetatable(titlebar, { __call = function(_, ...) return new(...) end}) -- vim: filetype=lua:expandtab:shiftwidth=4:tabstop=8:softtabstop=4:textwidth=80 From 7d32f7b733a502a4d58844649fc1faba8981f0a0 Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Sun, 24 Jan 2016 16:28:07 +0100 Subject: [PATCH 3/4] tasklist: Unreference client in unmanage Signed-off-by: Uli Schlachter --- lib/awful/widget/tasklist.lua | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/awful/widget/tasklist.lua b/lib/awful/widget/tasklist.lua index a0605834f..ccae7fa16 100644 --- a/lib/awful/widget/tasklist.lua +++ b/lib/awful/widget/tasklist.lua @@ -181,6 +181,9 @@ function tasklist.new(screen, filter, buttons, style, update_function, base_widg queued_update = true end end + function w._unmanage(c) + data[c] = nil + end if instances == nil then instances = {} local function us(s) @@ -219,7 +222,14 @@ function tasklist.new(screen, filter, buttons, style, update_function, base_widg capi.client.connect_signal("property::hidden", u) capi.client.connect_signal("tagged", u) capi.client.connect_signal("untagged", u) - capi.client.connect_signal("unmanage", u) + capi.client.connect_signal("unmanage", function(c) + u(c) + for s, i in pairs(instances) do + for _, tlist in pairs(i) do + tlist._unmanage(c) + end + end + end) capi.client.connect_signal("list", u) capi.client.connect_signal("focus", u) capi.client.connect_signal("unfocus", u) From e2d75dbcfd6ba8ee821fef690d17ca3bce07fdeb Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Sun, 24 Jan 2016 13:38:48 +0100 Subject: [PATCH 4/4] Add a functional testing for leaks with clients This opens xterm, closes it and makes sure that the client object representing xterm is GC'able at the end. The test will fail currently. Signed-off-by: Uli Schlachter --- tests/test-leak-client.lua | 78 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 tests/test-leak-client.lua diff --git a/tests/test-leak-client.lua b/tests/test-leak-client.lua new file mode 100644 index 000000000..205e48248 --- /dev/null +++ b/tests/test-leak-client.lua @@ -0,0 +1,78 @@ +-- Some memory leak checks involving clients as integration tests. +local awful = require("awful") +local wibox = require("wibox") + +-- "Enable" titlebars (so that the titlebar can prevent garbage collection) +client.connect_signal("manage", function (c) + local buttons = awful.util.table.join( + awful.button({ }, 1, function() + client.focus = c + c:raise() + awful.mouse.client.move(c) + end), + awful.button({ }, 3, function() + client.focus = c + c:raise() + awful.mouse.client.resize(c) + end) + ) + + -- Widgets that are aligned to the left + local left_layout = wibox.layout.fixed.horizontal(awful.titlebar.widget.iconwidget(c)) + left_layout:buttons(buttons) + + -- The title goes in the middle + local title = awful.titlebar.widget.titlewidget(c) + title:set_align("center") + local middle_layout = wibox.layout.flex.horizontal(title) + middle_layout:buttons(buttons) + + awful.titlebar(c):set_widget(wibox.layout.align.horizontal( + left_layout, + middle_layout, + wibox.layout.fixed.horizontal( + awful.titlebar.widget.floatingbutton(c), + awful.titlebar.widget.maximizedbutton(c), + awful.titlebar.widget.stickybutton(c), + awful.titlebar.widget.ontopbutton(c), + awful.titlebar.widget.closebutton(c) + )), { position = "bottom"}) +end) + +-- We tell the garbage collector when to work, disable it +collectgarbage("stop") + +-- We try to get a client representing an already-closed client. +-- The first iteration starts xterm, the second keeps a weak reference to it and +-- closes it and the last one checks that the client object is GC'able. +local objs = nil +local steps = { + function(count) + if count == 1 then + awful.spawn("xterm") + elseif not objs then + local c = client.get()[1] + if c then + objs = setmetatable({ c }, { __mode = "v" }) + c:kill() + end + else + assert(#objs == 1) + + -- Test that we have a client and that it's invalid (tostring() + -- causes an "invalid object" error) + local success, msg = pcall(function() tostring(objs[1]) end) + assert(not success) + assert(msg:find("invalid object"), msg) + + -- Check that it is garbage-collectable + collectgarbage("collect") + assert(#objs == 0) + return true + end + end, +} + +require("_runner").run_steps(steps) + +-- vim: filetype=lua:expandtab:shiftwidth=4:tabstop=8:softtabstop=4:textwidth=80