Fix memory leak in the default config on screen removal

The default config had tables like mywibox and mywibox[s] was the wibox
that is visible on screen s. When a screen is removed, nothing cleans up
these tables and so the screen and the wibox could not be garbage
collected. The same applies to the layoutbox, taglist etc.

This commit removes the global mywibox table and instead saves it as a
property on the screen. This way, the screen is not explicitly
referenced and when it is removed, the screen, its wibox and all of its
widgets become unreachable and can be garbage collected.

This commit also updates the docs and the tests that referenced things
(mostly the wibox) via mywibox[s] to now use s.mywibox.

Fixes: https://github.com/awesomeWM/awesome/issues/1125
Signed-off-by: Uli Schlachter <psychon@znc.in>
This commit is contained in:
Uli Schlachter 2016-09-30 22:46:51 +02:00
parent b6a3ae43fb
commit da6012da3e
9 changed files with 38 additions and 52 deletions

View File

@ -124,12 +124,8 @@ mykeyboardlayout = awful.widget.keyboardlayout()
mytextclock = wibox.widget.textclock()
-- Create a wibox for each screen and add it
mywibox = {}
mypromptbox = {}
mylayoutbox = {}
mytaglist = {}
-- @TAGLIST_BUTTON@
mytaglist.buttons = awful.util.table.join(
local taglist_buttons = awful.util.table.join(
awful.button({ }, 1, function(t) t:view_only() end),
awful.button({ modkey }, 1, function(t)
if client.focus then
@ -146,9 +142,8 @@ mytaglist.buttons = awful.util.table.join(
awful.button({ }, 5, function(t) awful.tag.viewprev(t.screen) end)
)
mytasklist = {}
-- @TASKLIST_BUTTON@
mytasklist.buttons = awful.util.table.join(
local tasklist_buttons = awful.util.table.join(
awful.button({ }, 1, function (c)
if c == client.focus then
c.minimized = true
@ -198,42 +193,42 @@ awful.screen.connect_for_each_screen(function(s)
awful.tag({ "1", "2", "3", "4", "5", "6", "7", "8", "9" }, s, awful.layout.layouts[1])
-- Create a promptbox for each screen
mypromptbox[s] = awful.widget.prompt()
s.mypromptbox = awful.widget.prompt()
-- Create an imagebox widget which will contains an icon indicating which layout we're using.
-- We need one layoutbox per screen.
mylayoutbox[s] = awful.widget.layoutbox(s)
mylayoutbox[s]:buttons(awful.util.table.join(
s.mylayoutbox = awful.widget.layoutbox(s)
s.mylayoutbox:buttons(awful.util.table.join(
awful.button({ }, 1, function () awful.layout.inc( 1) end),
awful.button({ }, 3, function () awful.layout.inc(-1) end),
awful.button({ }, 4, function () awful.layout.inc( 1) end),
awful.button({ }, 5, function () awful.layout.inc(-1) end)))
-- Create a taglist widget
mytaglist[s] = awful.widget.taglist(s, awful.widget.taglist.filter.all, mytaglist.buttons)
s.mytaglist = awful.widget.taglist(s, awful.widget.taglist.filter.all, taglist_buttons)
-- Create a tasklist widget
mytasklist[s] = awful.widget.tasklist(s, awful.widget.tasklist.filter.currenttags, mytasklist.buttons)
s.mytasklist = awful.widget.tasklist(s, awful.widget.tasklist.filter.currenttags, tasklist_buttons)
-- @DOC_WIBAR@
-- Create the wibox
mywibox[s] = awful.wibar({ position = "top", screen = s })
s.mywibox = awful.wibar({ position = "top", screen = s })
-- @DOC_SETUP_WIDGETS@
-- Add widgets to the wibox
mywibox[s]:setup {
s.mywibox:setup {
layout = wibox.layout.align.horizontal,
{ -- Left widgets
layout = wibox.layout.fixed.horizontal,
mylauncher,
mytaglist[s],
mypromptbox[s],
s.mytaglist,
s.mypromptbox,
},
mytasklist[s], -- Middle widget
s.mytasklist, -- Middle widget
{ -- Right widgets
layout = wibox.layout.fixed.horizontal,
mykeyboardlayout,
wibox.widget.systray(),
mytextclock,
mylayoutbox[s],
s.mylayoutbox,
},
}
end)
@ -332,13 +327,13 @@ globalkeys = awful.util.table.join(
{description = "restore minimized", group = "client"}),
-- Prompt
awful.key({ modkey }, "r", function () mypromptbox[awful.screen.focused()]:run() end,
awful.key({ modkey }, "r", function () awful.screen.focused().mypromptbox:run() end,
{description = "run prompt", group = "launcher"}),
awful.key({ modkey }, "x",
function ()
awful.prompt.run({ prompt = "Run Lua code: " },
mypromptbox[awful.screen.focused()].widget,
awful.screen.focused().mypromptbox.widget,
awful.util.eval, nil,
awful.util.get_cache_dir() .. "/history_eval")
end,

View File

@ -39,7 +39,7 @@ and usually provide some options.
Code:
mywibox[s] : setup {
s.mywibox : setup {
s == 1 and my_first_widget, -- Only display on screen 1
my_second_widget,
{ -- Add a background color/pattern for my_third_widget
@ -66,7 +66,7 @@ declarative layout, or `nil`.
Code:
mywibox[s] : setup {
s.mywibox : setup {
{
-- Force the textbox to always be 300 pixel long
{
@ -113,7 +113,7 @@ and `right` can be defined
Code:
mywibox[s] : setup {
s.mywibox : setup {
my_textbox1, -- Left
nil, -- Nothing in the middle
my_textbox2, -- Right
@ -129,7 +129,7 @@ is a simple circle widget:
Code:
mywibox[s] : setup {
s.mywibox : setup {
fit = function(self, context, width, height)
return height, height -- A square taking the full height
end,
@ -165,7 +165,7 @@ Code:
tb:set_markup("Hello world! ")
-- Repeat "tb" 3 times
mywibox[s] : setup {
s.mywibox : setup {
tb,
tb,
tb,
@ -198,7 +198,7 @@ underscore characters and non-numeric first character)
Code:
mywibox[s] : setup {
s.mywibox : setup {
{
id = "second",
widget = wibox.widget.textbox
@ -211,8 +211,8 @@ Code:
layout = wibox.layout.fixed.horizontal,
}
mywibox[s].first.second:set_markup("changed!")
mywibox[s]:get_children_by_id("third")[1]:set_markup("Also changed!")
s.mywibox.first.second:set_markup("changed!")
s.mywibox:get_children_by_id("third")[1]:set_markup("Also changed!")
@ -234,7 +234,7 @@ Code:
vicious.register(w, f(args))
end
mywibox[s] : setup {
s.mywibox : setup {
{
vicious = {vicious.widgets.cpu, "CPU: $1%", 3},
widget = wibox.widget.textbox
@ -273,7 +273,7 @@ Code:
local l = wibox.widget.align()
-- 3 circle
mywibox[s] : setup {
s.mywibox : setup {
circle,
circle,
circle,
@ -287,5 +287,5 @@ Code:
table.insert(three_circle, circle)
end
mywibox[s] : setup (three_circle)
s.mywibox : setup (three_circle)

View File

@ -160,4 +160,4 @@ some examples. Here is the most simple example you can get:
This can then be used as `bgimage` for a `wibox`, `awful.wibar` or
`wibox.container.background`:
mywibox[screen.primary].bgimage = img
screen.primary.mywibox.bgimage = img

View File

@ -145,7 +145,7 @@ You can use a `clientkeys` binding.:
Add the following key binding to your `globalkeys`:
awful.key({ modkey }, "b", function ()
mywibox[mouse.screen].visible = not mywibox[mouse.screen].visible
mouse.screen.mywibox.visible = not mouse.screen.mywibox.visible
end),
### How to toggle clients floating state?

View File

@ -236,7 +236,7 @@ end
-- hooks = {
-- -- Apply startup notification properties with Shift-Return.
-- {{"Shift" }, "Return", function(command)
-- mypromptbox[awful.screen.focused()]:spawn_and_handle_error(
-- awful.screen.focused().mypromptbox:spawn_and_handle_error(
-- command, {floating=true})
-- end},
-- -- Override default behavior of "Return": launch commands prefixed

View File

@ -20,7 +20,7 @@
-- 221 end
-- 222 end
-- 223 end),
-- 224 mylayoutbox[s],
-- 224 s.mylayoutbox,
--
-- ![Example screenshot](../images/awful_widget_watch.png)
--

View File

@ -12,13 +12,12 @@ local function emit_refresh()
end
-- Make the layoutbox in the default config GC'able
-- luacheck: globals mywibox mylayoutbox
for s in screen do
mywibox[s]:set_widget(wibox.widget.textbox())
mywibox[s].visible = false
s.mywibox:set_widget(wibox.widget.textbox())
s.mywibox.visible = false
s.mywibox = nil
s.mylayoutbox = nil
end
mywibox = nil
mylayoutbox = nil
emit_refresh()
-- Test if some objects can be garbage collected

View File

@ -47,7 +47,7 @@ local steps = {
assert(geom.width + 2*bw == 600, geom.width + 2*bw)
assert(geom.height + 2*bw == 610, geom.height + 2*bw)
local wb = mywibox[fake_screen]
local wb = fake_screen.mywibox
assert(wb.screen == fake_screen, tostring(wb.screen) .. " ~= " .. tostring(fake_screen))
assert(wb.x == 100, wb.x)
assert(wb.y == 110, wb.y)
@ -58,19 +58,12 @@ local steps = {
-- Step 3: Say goodbye to the screen
function()
local wb = fake_screen.mywibox
fake_screen:fake_remove()
-- Now that the screen is invalid, the wibox shouldn't refer to it any
-- more
assert(mywibox[fake_screen].screen ~= fake_screen)
-- TODO: This is a hack to make the test work, how to do this so that it
-- also works "in the wild"?
mypromptbox[fake_screen] = nil
mylayoutbox[fake_screen] = nil
mytaglist[fake_screen] = nil
mytasklist[fake_screen] = nil
mywibox[fake_screen] = nil
assert(wb.screen ~= fake_screen)
-- Wrap in a weak table to allow garbage collection
fake_screen = setmetatable({ fake_screen }, { __mode = "v" })

View File

@ -2,12 +2,11 @@ local placement = require("awful.placement")
local wibox = require("wibox")
local wibar = require("awful.wibar")
-- luacheck: globals mywibox
local steps = {}
local parent, small
local twibar, bwibar, lwibar, rwibar = mywibox[screen.primary]
local twibar, bwibar, lwibar, rwibar = screen.primary.mywibox
-- Test the struts without using wibars
table.insert(steps, function()