Merge pull request #479 from psychon/leaks

Add integration tests checking for leaks and fix some results. Closes https://github.com/awesomeWM/awesome/pull/479.
2015-09-28 23:35:54 +02:00 · 2015-09-28 23:35:54 +02:00 · dd657c0531
parent d544421c67 3e9fdea650
commit dd657c0531
9 changed files with 282 additions and 147 deletions
--- a/lib/awful/tooltip.lua
+++ b/lib/awful/tooltip.lua
@ -59,17 +59,6 @@ local ipairs = ipairs
 -- @tfield boolean visible True if tooltip is visible.
 local tooltip = { mt = {}  }

--- Tooltip private data.
-- @local
-- @table tooltip.data
-- @tfield string fg tooltip foreground color.
-- @tfield string font Tooltip font.
-- @tfield function hide The hide() function.
-- @tfield function show The show() function.
-- @tfield gears.timer timer The text update timer.
-- @tfield function timer_function The text update timer function.
-local data = setmetatable({}, { __mode = 'k' })
-
 -- Place the tooltip on the screen.
 -- @tparam tooltip self A tooltip object.
 local function place(self)
@ -98,10 +87,10 @@ end
 local function show(self)
    -- do nothing if the tooltip is already shown
    if self.visible then return end
-    if data[self].timer then
-        if not data[self].timer.started then
-            data[self].timer_function()
-            data[self].timer:start()
+    if self.timer then
+        if not self.timer.started then
+            self.timer_function()
+            self.timer:start()
        end
    end
    set_geometry(self)
@ -116,9 +105,9 @@ end
 local function hide(self)
    -- do nothing if the tooltip is already hidden
    if not self.visible then return end
-    if data[self].timer then
-        if data[self].timer.started then
-            data[self].timer:stop()
+    if self.timer then
+        if self.timer.started then
+            self.timer:stop()
        end
    end
    self.visible = false
@ -154,8 +143,8 @@ end
 -- @tparam tooltip self A tooltip object.
 -- @tparam number timeout The timeout value.
 tooltip.set_timeout = function(self, timeout)
-    if data[self].timer then
-        data[self].timer.timeout = timeout
+    if self.timer then
+        self.timer.timeout = timeout
    end
 end

@ -165,8 +154,8 @@ end
 -- @tparam gears.object object An object with `mouse::enter` and
 --   `mouse::leave` signals.
 tooltip.add_to_object = function(self, object)
-    object:connect_signal("mouse::enter", data[self].show)
-    object:connect_signal("mouse::leave", data[self].hide)
+    object:connect_signal("mouse::enter", self.show)
+    object:connect_signal("mouse::leave", self.hide)
 end

 --- Remove tooltip from an object.
@ -175,8 +164,8 @@ end
 -- @tparam gears.object object An object with `mouse::enter` and
 --   `mouse::leave` signals.
 tooltip.remove_from_object = function(self, object)
-    object:disconnect_signal("mouse::enter", data[self].show)
-    object:disconnect_signal("mouse::leave", data[self].hide)
+    object:disconnect_signal("mouse::enter", self.show)
+    object:disconnect_signal("mouse::leave", self.hide)
 end


@ -213,24 +202,24 @@ tooltip.new = function(args)
            delay_timeout:stop()
        end)

-        data[self] = {
-            show = function()
-                if not delay_timeout.started then
-                    delay_timeout:start()
-                end
-            end,
-            hide = function()
-                if delay_timeout.started then
-                    delay_timeout:stop()
-                end
-                hide(self)
-            end,
-        }
+        function self.show()
+            if not delay_timeout.started then
+                delay_timeout:start()
+            end
+        end
+        function self.hide()
+            if delay_timeout.started then
+                delay_timeout:stop()
+            end
+            hide(self)
+        end
    else
-        data[self] = {
-            show = function() show(self) end,
-            hide = function() hide(self) end,
-        }
+        function self.show()
+            show(self)
+        end
+        function self.hide()
+            hide(self)
+        end
    end

    -- export functions
@ -242,11 +231,11 @@ tooltip.new = function(args)

    -- setup the timer action only if needed
    if args.timer_function then
-        data[self].timer = timer { timeout = args.timeout and args.timeout or 1 }
-        data[self].timer_function = function()
+        self.timer = timer { timeout = args.timeout and args.timeout or 1 }
+        self.timer_function = function()
                self:set_markup(args.timer_function())
            end
-        data[self].timer:connect_signal("timeout", data[self].timer_function)
+        self.timer:connect_signal("timeout", self.timer_function)
    end

    -- Set default properties
@ -274,7 +263,7 @@ tooltip.new = function(args)

    -- Close the tooltip when clicking it.  This gets done on release, to not
    -- emit the release event on an underlying object, e.g. the titlebar icon.
-    self.wibox:buttons(abutton({}, 1, nil, function() data[self].hide() end))
+    self.wibox:buttons(abutton({}, 1, nil, function() self.hide() end))

    -- Re-place when the geometry of the wibox changes.
    self.wibox:connect_signal("property::width",  function() place(self) end)
--- a/lib/awful/widget/layoutbox.lua
+++ b/lib/awful/widget/layoutbox.lua
@ -18,29 +18,46 @@ local imagebox = require("wibox.widget.imagebox")

 local layoutbox = { mt = {} }

-local function update(w, screen, tooltip)
+local boxes = nil
+
+local function update(w, screen)
    local layout = layout.getname(layout.get(screen))
-    tooltip:set_text(layout or "[no name]")
+    w._layoutbox_tooltip:set_text(layout or "[no name]")
    w:set_image(layout and beautiful["layout_" .. layout])
 end

+local function update_from_tag(t)
+    local screen = tag.getscreen(t)
+    local w = boxes[screen]
+    if w then
+        update(w, screen)
+    end
+end
+
 --- Create a layoutbox widget. It draws a picture with the current layout
 -- symbol of the current tag.
 -- @param screen The screen number that the layout will be represented for.
 -- @return An imagebox widget configured as a layoutbox.
 function layoutbox.new(screen)
    local screen = screen or 1
-    local w = imagebox()
-    local tooltip = tooltip({ objects = {w}, delay_show = 1 })

-    update(w, screen, tooltip)
-
-    local function update_on_tag_selection(t)
-        return update(w, tag.getscreen(t), tooltip)
+    -- Do we already have the update callbacks registered?
+    if boxes == nil then
+        boxes = setmetatable({}, { __mode = "v" })
+        tag.attached_connect_signal(nil, "property::selected", update_from_tag)
+        tag.attached_connect_signal(nil, "property::layout", update_from_tag)
+        layoutbox.boxes = boxes
    end

-    tag.attached_connect_signal(screen, "property::selected", update_on_tag_selection)
-    tag.attached_connect_signal(screen, "property::layout", update_on_tag_selection)
+    -- Do we already have a layoutbox for this screen?
+    local w = boxes[screen]
+    if not w then
+        w = imagebox()
+        w._layoutbox_tooltip = tooltip({ objects = {w}, delay_show = 1 })
+
+        update(w, screen)
+        boxes[screen] = w
+    end

    return w
 end
--- a/lib/awful/widget/taglist.lua
+++ b/lib/awful/widget/taglist.lua
@ -27,6 +27,8 @@ local timer = require("gears.timer")
 local taglist = { mt = {} }
 taglist.filter = {}

+local instances = nil
+
 function taglist.taglist_label(t, args)
    if not args then args = {} end
    local theme = beautiful.get()
@ -159,39 +161,53 @@ function taglist.new(screen, filter, buttons, style, update_function, base_widge
    local data = setmetatable({}, { __mode = 'k' })

    local queued_update = {}
-    local u = function (s)
-        if s ~= screen then return end
-
+    function w._do_taglist_update()
        -- Add a delayed callback for the first update.
-        if not queued_update[s] then
+        if not queued_update[screen] then
            timer.delayed_call(function()
-                taglist_update(s, w, buttons, filter, data, style, uf)
-                queued_update[s] = false
+                taglist_update(screen, w, buttons, filter, data, style, uf)
+                queued_update[screen] = false
            end)
-            queued_update[s] = true
+            queued_update[screen] = true
        end
    end
-    local uc = function (c) return u(c.screen) end
-    local ut = function (t) return u(tag.getscreen(t)) end
-    capi.client.connect_signal("focus", uc)
-    capi.client.connect_signal("unfocus", uc)
-    tag.attached_connect_signal(screen, "property::selected", ut)
-    tag.attached_connect_signal(screen, "property::icon", ut)
-    tag.attached_connect_signal(screen, "property::hide", ut)
-    tag.attached_connect_signal(screen, "property::name", ut)
-    tag.attached_connect_signal(screen, "property::activated", ut)
-    tag.attached_connect_signal(screen, "property::screen", ut)
-    tag.attached_connect_signal(screen, "property::index", ut)
-    tag.attached_connect_signal(screen, "property::urgent", ut)
-    capi.client.connect_signal("property::screen", function(c, old_screen)
-        if screen == c.screen or screen == old_screen then
-            u(screen)
+    if instances == nil then
+        instances = {}
+        local function u(s)
+            local i = instances[s]
+            if i then
+                for _, tlist in pairs(i) do
+                    tlist._do_taglist_update()
+                end
+            end
        end
-    end)
-    capi.client.connect_signal("tagged", uc)
-    capi.client.connect_signal("untagged", uc)
-    capi.client.connect_signal("unmanage", uc)
-    u(screen)
+        local uc = function (c) return u(c.screen) end
+        local ut = function (t) return u(tag.getscreen(t)) end
+        capi.client.connect_signal("focus", uc)
+        capi.client.connect_signal("unfocus", uc)
+        tag.attached_connect_signal(screen, "property::selected", ut)
+        tag.attached_connect_signal(screen, "property::icon", ut)
+        tag.attached_connect_signal(screen, "property::hide", ut)
+        tag.attached_connect_signal(screen, "property::name", ut)
+        tag.attached_connect_signal(screen, "property::activated", ut)
+        tag.attached_connect_signal(screen, "property::screen", ut)
+        tag.attached_connect_signal(screen, "property::index", ut)
+        tag.attached_connect_signal(screen, "property::urgent", ut)
+        capi.client.connect_signal("property::screen", function(c, old_screen)
+            u(c.screen)
+            u(old_screen)
+        end)
+        capi.client.connect_signal("tagged", uc)
+        capi.client.connect_signal("untagged", uc)
+        capi.client.connect_signal("unmanage", uc)
+    end
+    w._do_taglist_update()
+    local list = instances[s]
+    if not list then
+        list = setmetatable({}, { __mode = "v" })
+        instances[screen] = list
+    end
+    table.insert(list, w)
    return w
 end

--- a/lib/awful/widget/tasklist.lua
+++ b/lib/awful/widget/tasklist.lua
@ -23,6 +23,8 @@ local timer = require("gears.timer")

 local tasklist = { mt = {} }

+local instances
+
 -- Public structures
 tasklist.filter = {}

@ -169,7 +171,7 @@ function tasklist.new(screen, filter, buttons, style, update_function, base_widg
    local data = setmetatable({}, { __mode = 'k' })

    local queued_update = false
-    local u = function ()
+    function w._do_tasklist_update()
        -- Add a delayed callback for the first update.
        if not queued_update then
            timer.delayed_call(function()
@ -179,34 +181,56 @@ function tasklist.new(screen, filter, buttons, style, update_function, base_widg
            queued_update = true
        end
    end
-    tag.attached_connect_signal(screen, "property::selected", u)
-    tag.attached_connect_signal(screen, "property::activated", u)
-    capi.client.connect_signal("property::urgent", u)
-    capi.client.connect_signal("property::sticky", u)
-    capi.client.connect_signal("property::ontop", u)
-    capi.client.connect_signal("property::above", u)
-    capi.client.connect_signal("property::below", u)
-    capi.client.connect_signal("property::floating", u)
-    capi.client.connect_signal("property::maximized_horizontal", u)
-    capi.client.connect_signal("property::maximized_vertical", u)
-    capi.client.connect_signal("property::minimized", u)
-    capi.client.connect_signal("property::name", u)
-    capi.client.connect_signal("property::icon_name", u)
-    capi.client.connect_signal("property::icon", u)
-    capi.client.connect_signal("property::skip_taskbar", u)
-    capi.client.connect_signal("property::screen", function(c, old_screen)
-        if screen == c.screen or screen == old_screen then
-            u()
+    if instances == nil then
+        instances = {}
+        local function us(s)
+            local i = instances[s]
+            if i then
+                for _, tlist in pairs(i) do
+                    tlist._do_tasklist_update()
+                end
+            end
        end
-    end)
-    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("list", u)
-    capi.client.connect_signal("focus", u)
-    capi.client.connect_signal("unfocus", u)
-    u()
+        local function u()
+            for s in ipairs(instances) do
+                us(s)
+            end
+        end
+
+        tag.attached_connect_signal(nil, "property::selected", u)
+        tag.attached_connect_signal(nil, "property::activated", u)
+        capi.client.connect_signal("property::urgent", u)
+        capi.client.connect_signal("property::sticky", u)
+        capi.client.connect_signal("property::ontop", u)
+        capi.client.connect_signal("property::above", u)
+        capi.client.connect_signal("property::below", u)
+        capi.client.connect_signal("property::floating", u)
+        capi.client.connect_signal("property::maximized_horizontal", u)
+        capi.client.connect_signal("property::maximized_vertical", u)
+        capi.client.connect_signal("property::minimized", u)
+        capi.client.connect_signal("property::name", u)
+        capi.client.connect_signal("property::icon_name", u)
+        capi.client.connect_signal("property::icon", u)
+        capi.client.connect_signal("property::skip_taskbar", u)
+        capi.client.connect_signal("property::screen", function(c, old_screen)
+            us(c.screen)
+            us(old_screen)
+        end)
+        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("list", u)
+        capi.client.connect_signal("focus", u)
+        capi.client.connect_signal("unfocus", u)
+    end
+    w._do_tasklist_update()
+    local list = instances[s]
+    if not list then
+        list = setmetatable({}, { __mode = "v" })
+        instances[screen] = list
+    end
+    table.insert(list, w)
    return w
 end

--- a/lib/awful/widget/textclock.lua
+++ b/lib/awful/widget/textclock.lua
@ -31,13 +31,14 @@ function textclock.new(format, timeout)
    local timeout = timeout or 60

    local w = textbox()
-    local t = timer { timeout = timeout }
-    t:connect_signal("timeout", function()
+    local t
+    function w._textclock_update_cb()
        w:set_markup(DateTime.new_now_local():format(format))
        t.timeout = calc_timeout(timeout)
        t:again()
-    end)
-    t:start()
+        return true -- Continue the timer
+    end
+    t = timer.weak_start_new(timeout, w._textclock_update_cb)
    t:emit_signal("timeout")
    return w
 end
--- a/lib/wibox/widget/base.lua
+++ b/lib/wibox/widget/base.lua
@ -18,7 +18,7 @@ local base = {}
 -- {{{ Caches

 -- Indexes are widgets, allow them to be garbage-collected
-local widget_dependencies = setmetatable({}, { __mode = "k" })
+local widget_dependencies = setmetatable({}, { __mode = "kv" })

 -- Get the cache of the given kind for this widget. This returns a gears.cache
 -- that calls the callback of kind `kind` on the widget.
--- a/tests/_wibox_helper.lua
+++ b/tests/_wibox_helper.lua
@ -0,0 +1,32 @@
+local awful = require("awful")
+local cairo = require("lgi").cairo
+local wibox = require("wibox")
+
+return { create_wibox = function()
+    local img = cairo.ImageSurface(cairo.Format.ARGB32, 20, 20)
+
+    -- Widgets that are aligned to the left
+    local left_layout = wibox.layout.fixed.horizontal()
+    left_layout:add(awful.widget.launcher({ image = img, command = "bash" }))
+    left_layout:add(awful.widget.taglist(1, awful.widget.taglist.filter.all))
+    left_layout:add(awful.widget.prompt())
+
+    -- Widgets that are aligned to the right
+    local right_layout = wibox.layout.fixed.horizontal()
+    local textclock = awful.widget.textclock()
+    right_layout:add(textclock)
+    right_layout:add(awful.widget.layoutbox(1))
+
+    -- Now bring it all together (with the tasklist in the middle)
+    local layout = wibox.layout.align.horizontal()
+    layout:set_left(left_layout)
+    layout:set_middle(awful.widget.tasklist(1, awful.widget.tasklist.filter.currenttags))
+    layout:set_right(right_layout)
+
+    -- Create wibox
+    local wb = wibox({ width = 1024, height = 20, screen = 1 })
+    --wb.visible = true
+    wb:set_widget(layout)
+
+    return wb, textclock, img, left_layout, right_layout, layout
+end }
--- a/tests/test-benchmark.lua
+++ b/tests/test-benchmark.lua
@ -2,10 +2,8 @@
 -- that we notice if they break.

 local awful = require("awful")
-local wibox = require("wibox")
-local lgi = require("lgi")
-local GLib = lgi.GLib
-local cairo = lgi.cairo
+local GLib = require("lgi").GLib
+local create_wibox = require("_wibox_helper").create_wibox

 local not_under_travis = not os.getenv("CI")

@ -41,33 +39,9 @@ local function do_pending_repaint()
    awesome.emit_signal("refresh")
 end

-local function create_wibox()
-    local img = cairo.ImageSurface(cairo.Format.ARGB32, 20, 20)
-
-    -- Widgets that are aligned to the left
-    local left_layout = wibox.layout.fixed.horizontal()
-    left_layout:add(awful.widget.launcher({ image = img, command = "bash" }))
-    left_layout:add(awful.widget.taglist(1, awful.widget.taglist.filter.all))
-    left_layout:add(awful.widget.prompt())
-
-    -- Widgets that are aligned to the right
-    local right_layout = wibox.layout.fixed.horizontal()
-    local textclock = awful.widget.textclock()
-    right_layout:add(textclock)
-    right_layout:add(awful.widget.layoutbox(1))
-
-    -- Now bring it all together (with the tasklist in the middle)
-    local layout = wibox.layout.align.horizontal()
-    layout:set_left(left_layout)
-    layout:set_middle(awful.widget.tasklist(1, awful.widget.tasklist.filter.currenttags))
-    layout:set_right(right_layout)
-
-    -- Create wibox
-    local wb = wibox({ width = 1024, height = 20, screen = 1 })
-    wb.visible = true
-    wb:set_widget(layout)
-
-    return wb, textclock
+local function create_and_draw_wibox()
+    create_wibox()
+    do_pending_repaint()
 end

 local wb, textclock = create_wibox()
@ -92,7 +66,7 @@ local function e2e_tag_switch()
    do_pending_repaint()
 end

-benchmark(create_wibox, "create wibox")
+benchmark(create_and_draw_wibox, "create&draw wibox")
 benchmark(update_textclock, "update textclock")
 benchmark(relayout_textclock, "relayout textclock")
 benchmark(redraw_textclock, "redraw textclock")
--- a/tests/test-leaks.lua
+++ b/tests/test-leaks.lua
@ -0,0 +1,82 @@
+-- Some memory leak checks as integration tests.
+
+local awful = require("awful")
+local cairo = require("lgi").cairo
+local create_wibox = require("_wibox_helper").create_wibox
+local gears = require("gears")
+local wibox = require("wibox")
+
+local errors = {}
+
+local prepare_for_collect = nil
+local function emit_refresh()
+    awesome.emit_signal("refresh")
+end
+
+-- Make the layoutbox in the default config GC'able
+mywibox[1].visible = false
+mywibox = nil
+mylayoutbox = nil
+emit_refresh()
+
+-- Test if some objects can be garbage collected
+local function collectable(a, b, c, d, e, f, g, h, last)
+    assert(last == nil, "got more arguments than supported")
+    local objs = setmetatable({ a, b, c, d, e, f, g, h }, { __mode = "v" })
+    a, b, c, d, e, f, g, h = nil, nil, nil, nil, nil, nil, nil, nil
+    if prepare_for_collect then
+        prepare_for_collect()
+        prepare_for_collect = nil
+    end
+    collectgarbage("collect")
+    collectgarbage("collect")
+    -- Check if the table is now empty
+    for k, v in pairs(objs) do
+        print("Some object was not garbage collected!")
+        error(v)
+    end
+end
+
+-- Use the layoutbox for testing delayed tooltips
+local function tooltip_delayed()
+    local l = awful.widget.layoutbox(1)
+    local t = l._layoutbox_tooltip
+    assert(t)
+    return l, t
+end
+
+local function tooltip_now()
+    local w = wibox.widget.textbox("some textbox")
+    local t = awful.tooltip({ objects = {w} })
+    return w, t
+end
+
+-- First test some basic widgets
+collectable(wibox.widget.base.make_widget())
+collectable(wibox.widget.textbox("foo"))
+collectable(wibox.layout.fixed.horizontal())
+collectable(wibox.layout.align.horizontal())
+
+-- Then some random widgets from awful
+collectable(awful.widget.launcher({ image = cairo.ImageSurface(cairo.Format.ARGB32, 20, 20), command = "bash" }))
+collectable(awful.widget.prompt())
+collectable(awful.widget.textclock())
+collectable(awful.widget.layoutbox(1))
+
+-- Some widgets do things via timer.delayed_call
+prepare_for_collect = emit_refresh
+collectable(tooltip_delayed())
+
+prepare_for_collect = emit_refresh
+collectable(tooltip_now())
+
+prepare_for_collect = emit_refresh
+collectable(awful.widget.taglist(1, awful.widget.taglist.filter.all))
+
+prepare_for_collect = emit_refresh
+collectable(awful.widget.tasklist(1, awful.widget.tasklist.filter.currenttags))
+
+prepare_for_collect = emit_refresh
+collectable(create_wibox())
+
+require("_runner").run_steps({ function() return true end })