naughty: Never hold a strong reference to the notification in the box.

luajit was failing to GC the notification about 5% of the time. This
commit stores all widget notifications in a weak table and don't let
any lambda access the parent object notification object.

Each of those changes reduces the failure rate. There might still be
a couple in there, but the test passed 200x on my laptop with 100%
success rate.
This commit is contained in:
Emmanuel Lepage Vallee 2021-10-31 19:30:17 -07:00
parent 6e8b0d5a85
commit 9dcfde32e0
7 changed files with 105 additions and 62 deletions

View File

@ -46,22 +46,24 @@ end
-- @propemits true false -- @propemits true false
function background:set_notification(notif) function background:set_notification(notif)
if self._private.notification == notif then return end local old = self._private.notification[1]
if self._private.notification then if old == notif then return end
self._private.notification:disconnect_signal("property::bg",
if old then
old:disconnect_signal("property::bg",
self._private.background_changed_callback) self._private.background_changed_callback)
self._private.notification:disconnect_signal("property::border_width", old:disconnect_signal("property::border_width",
self._private.background_changed_callback) self._private.background_changed_callback)
self._private.notification:disconnect_signal("property::border_color", old:disconnect_signal("property::border_color",
self._private.background_changed_callback) self._private.background_changed_callback)
self._private.notification:disconnect_signal("property::shape", old:disconnect_signal("property::shape",
self._private.background_changed_callback) self._private.background_changed_callback)
end end
update_background(notif, self) update_background(notif, self)
self._private.notification = notif self._private.notification = setmetatable({notif}, {__mode="v"})
notif:connect_signal("property::bg" , self._private.background_changed_callback) notif:connect_signal("property::bg" , self._private.background_changed_callback)
notif:connect_signal("property::border_width", self._private.background_changed_callback) notif:connect_signal("property::border_width", self._private.background_changed_callback)
@ -82,12 +84,13 @@ local function new(args)
args = args or {} args = args or {}
local bg = wbg() local bg = wbg()
bg._private.notification = {}
bg:set_border_strategy("inner") bg:set_border_strategy("inner")
gtable.crush(bg, background, true) gtable.crush(bg, background, true)
function bg._private.background_changed_callback() function bg._private.background_changed_callback()
update_background(bg._private.notification, bg) update_background(bg._private.notification[1], bg)
end end
if args.notification then if args.notification then

View File

@ -47,14 +47,16 @@ local function init_screen(s)
end end
local function disconnect(self) local function disconnect(self)
if self._private.notification then local n = self._private.notification[1]
self._private.notification:disconnect_signal("destroyed",
if n then
n:disconnect_signal("destroyed",
self._private.destroy_callback) self._private.destroy_callback)
self._private.notification:disconnect_signal("property::margin", n:disconnect_signal("property::margin",
self._private.update) self._private.update)
self._private.notification:disconnect_signal("property::suspended", n:disconnect_signal("property::suspended",
self._private.hide) self._private.hide)
end end
end end
@ -123,14 +125,13 @@ local function finish(self)
end end
end end
local preset local preset = (self._private.notification[1] or {}).preset
if self.private and self.private.args and self.private.args.notification then
preset = self.private.args.notification.preset
end
update_position(self.position, preset) update_position(self.position, preset)
disconnect(self) disconnect(self)
self._private.notification = {}
end end
-- It isn't a good idea to use the `attach` `awful.placement` property. If the -- It isn't a good idea to use the `attach` `awful.placement` property. If the
@ -236,21 +237,19 @@ local function generate_widget(args, n)
end end
-- Call `:set_notification` on all children -- Call `:set_notification` on all children
awcommon._set_common_property(w, "notification", n or args.notification) awcommon._set_common_property(w, "notification", n)
return w return w
end end
local function init(self, notification) local function init(self, notification)
local args = self._private.args
local preset = notification.preset or {} local preset = notification.preset or {}
local position = args.position or notification.position or local position = self._private.position or notification.position or
preset.position or beautiful.notification_position or "top_right" preset.position or beautiful.notification_position or "top_right"
if not self.widget then if not self.widget then
self.widget = generate_widget(self._private.args, notification) self.widget = generate_widget(self._private, notification)
end end
local bg = self._private.widget:get_children_by_id( "background_role" )[1] local bg = self._private.widget:get_children_by_id( "background_role" )[1]
@ -295,20 +294,26 @@ local function init(self, notification)
end end
function box:set_notification(notif) function box:set_notification(notif)
if self._private.notification == notif then return end if self._private.notification[1] == notif then return end
disconnect(self) disconnect(self)
init(self, notif) init(self, notif)
self._private.notification = notif self._private.notification = setmetatable({notif}, {__mode="v"})
self:emit_signal("property::notification", notif) self:emit_signal("property::notification", notif)
end end
function box:get_notification()
return self._private.notification[1]
end
function box:get_position() function box:get_position()
if self._private.notification then local n = self._private.notification[1]
return self._private.notification:get_position()
if n then
return n:get_position()
end end
return "top_right" return "top_right"
@ -357,7 +362,9 @@ local function new(args)
if not new_args.widget then return nil end if not new_args.widget then return nil end
local ret = popup(new_args) local ret = popup(new_args)
ret._private.args = new_args ret._private.notification = {}
ret._private.widget_template = args.widget_template
ret._private.position = args.position
gtable.crush(ret, box, true) gtable.crush(ret, box, true)
@ -371,8 +378,10 @@ local function new(args)
--TODO remove --TODO remove
local function hide() local function hide()
if ret._private.notification then local n = ret._private.notification[1]
ret._private.notification:destroy()
if n then
n:destroy()
end end
end end

View File

@ -168,14 +168,16 @@ local function wb_label(action, self)
end end
local function update(self) local function update(self)
if not self._private.layout or not self._private.notification then return end local n = self._private.notification[1]
if not self._private.layout or not n then return end
awcommon.list_update( awcommon.list_update(
self._private.layout, self._private.layout,
self._private.default_buttons, self._private.default_buttons,
function(o) return wb_label(o, self) end, function(o) return wb_label(o, self) end,
self._private.data, self._private.data,
self._private.notification.actions, n.actions,
{ {
widget_template = self._private.widget_template widget_template = self._private.widget_template
} }
@ -229,7 +231,7 @@ local actionlist = {}
function actionlist:set_notification(notif) function actionlist:set_notification(notif)
self._private.notification = notif self._private.notification = setmetatable({notif}, {__mode="v"})
if not self._private.layout then if not self._private.layout then
self._private.layout = wibox.layout.fixed.horizontal() self._private.layout = wibox.layout.fixed.horizontal()
@ -329,8 +331,9 @@ local function new(_, args)
gtable.crush(wdg, actionlist, true) gtable.crush(wdg, actionlist, true)
wdg._private.data = {} wdg._private.data = {}
wdg._private.notification = {}
gtable.crush(wdg, args) gtable.crush(wdg, args, false)
wdg._private.style = wdg._private.style or {} wdg._private.style = wdg._private.style or {}
@ -338,7 +341,7 @@ local function new(_, args)
wdg._private.default_buttons = gtable.join( wdg._private.default_buttons = gtable.join(
abutton({ }, 1, function(a) abutton({ }, 1, function(a)
local notif = wdg._private.notification or args.notification local notif = wdg._private.notification[1]
a:invoke(notif) a:invoke(notif)
end) end)
) )

View File

@ -22,7 +22,7 @@ local function notif_size()
constraint:set_width(beautiful.notification_max_width or dpi(500)) constraint:set_width(beautiful.notification_max_width or dpi(500))
rawset(constraint, "set_notification", function(_, notif) rawset(constraint, "set_notification", function(_, notif)
constraint._private.notification = notif constraint._private.notification = setmetatable({notif}, {__mode = "v"})
local s = false local s = false
if notif.width and notif.width ~= beautiful.notification_max_width then if notif.width and notif.width ~= beautiful.notification_max_width then
@ -37,6 +37,10 @@ local function notif_size()
constraint.strategy = s and "exact" or "max" constraint.strategy = s and "exact" or "max"
end) end)
rawset(constraint, "get_notification", function()
return constraint._private.notification[1]
end)
return constraint return constraint
end end

View File

@ -83,10 +83,12 @@ end
-- @propemits true false -- @propemits true false
function icon:set_notification(notif) function icon:set_notification(notif)
if self._private.notification == notif then return end local old = (self._private.notification or {})[1]
if self._private.notification then if old == notif then return end
self._private.notification:disconnect_signal("destroyed",
if old then
old:disconnect_signal("destroyed",
self._private.icon_changed_callback) self._private.icon_changed_callback)
end end
@ -96,7 +98,7 @@ function icon:set_notification(notif)
self:set_image(icn) self:set_image(icn)
end end
self._private.notification = notif self._private.notification = setmetatable({notif}, {__mode="v"})
notif:connect_signal("property::icon", self._private.icon_changed_callback) notif:connect_signal("property::icon", self._private.icon_changed_callback)
self:emit_signal("property::notification", notif) self:emit_signal("property::notification", notif)
@ -154,10 +156,14 @@ local function new(args)
local tb = imagebox() local tb = imagebox()
gtable.crush(tb, icon, true) gtable.crush(tb, icon, true)
tb._private.notification = {}
function tb._private.icon_changed_callback() function tb._private.icon_changed_callback()
local n = tb._private.notification[1]
local icn = gsurface.load_silently(tb._private.notification.icon) if not n then return end
local icn = gsurface.load_silently(n.icon)
if icn then if icn then
tb:set_image(icn) tb:set_image(icn)

View File

@ -27,18 +27,20 @@ local message = {}
-- @propemits true false -- @propemits true false
function message:set_notification(notif) function message:set_notification(notif)
if self._private.notification == notif then return end local old = self._private.notification[1]
if self._private.notification then if old == notif then return end
self._private.notification:disconnect_signal("property::message",
if old then
old:disconnect_signal("property::message",
self._private.message_changed_callback) self._private.message_changed_callback)
self._private.notification:disconnect_signal("property::fg", old:disconnect_signal("property::fg",
self._private.message_changed_callback) self._private.message_changed_callback)
end end
markup(self, notif.message, notif.fg, notif.font) markup(self, notif.message, notif.fg, notif.font)
self._private.notification = notif self._private.notification = setmetatable({notif}, {__mode="v"})
notif:connect_signal("property::message", self._private.message_changed_callback) notif:connect_signal("property::message", self._private.message_changed_callback)
notif:connect_signal("property::fg" , self._private.message_changed_callback) notif:connect_signal("property::fg" , self._private.message_changed_callback)
@ -57,16 +59,23 @@ local function new(args)
local tb = textbox() local tb = textbox()
tb:set_wrap("word") tb:set_wrap("word")
tb:set_font(beautiful.notification_font) tb:set_font(beautiful.notification_font)
tb._private.notification = {}
gtable.crush(tb, message, true) gtable.crush(tb, message, true)
function tb._private.message_changed_callback() function tb._private.message_changed_callback()
local n = tb._private.notification[1]
if n then
markup( markup(
tb, tb,
tb._private.notification.message, n.message,
tb._private.notification.fg, n.fg,
tb._private.notification.font n.font
) )
else
markup(tb, nil, nil)
end
end end
if args.notification then if args.notification then

View File

@ -27,18 +27,20 @@ local title = {}
-- @propemits true false -- @propemits true false
function title:set_notification(notif) function title:set_notification(notif)
if self._private.notification == notif then return end local old = self._private.notification[1]
if self._private.notification then if old == notif then return end
self._private.notification:disconnect_signal("property::message",
if old then
old:disconnect_signal("property::message",
self._private.title_changed_callback) self._private.title_changed_callback)
self._private.notification:disconnect_signal("property::fg", old:disconnect_signal("property::fg",
self._private.title_changed_callback) self._private.title_changed_callback)
end end
markup(self, notif.title, notif.fg, notif.font) markup(self, notif.title, notif.fg, notif.font)
self._private.notification = notif self._private.notification = setmetatable({notif}, {__mode="v"})
self._private.title_changed_callback() self._private.title_changed_callback()
notif:connect_signal("property::title", self._private.title_changed_callback) notif:connect_signal("property::title", self._private.title_changed_callback)
@ -58,16 +60,23 @@ local function new(args)
local tb = textbox() local tb = textbox()
tb:set_wrap("word") tb:set_wrap("word")
tb:set_font(beautiful.notification_font) tb:set_font(beautiful.notification_font)
tb._private.notification = {}
gtable.crush(tb, title, true) gtable.crush(tb, title, true)
function tb._private.title_changed_callback() function tb._private.title_changed_callback()
local n = tb._private.notification[1]
if n then
markup( markup(
tb, tb,
tb._private.notification.title, n.title,
tb._private.notification.fg, n.fg,
tb._private.notification.font n.font
) )
else
markup("", nil, nil)
end
end end
if args.notification then if args.notification then