From 089ed0e8dd63fca4b91a6b2d88801c8d45d6b85d Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Sun, 14 Jun 2015 14:21:23 +0200 Subject: [PATCH 1/2] gears.object: Add :weak_connect_signal() Connecting to a signal weakly has the same effect as connecting to it strongly, but it allows the garbage collector to disconnect the signal in case nothing else references this function. Signed-off-by: Uli Schlachter --- lib/gears/object.lua | 27 +++++++++++-- spec/gears/object_spec.lua | 80 +++++++++++++++++++++++++++++++++++--- 2 files changed, 97 insertions(+), 10 deletions(-) diff --git a/lib/gears/object.lua b/lib/gears/object.lua index 2fc899f30..7e3d62b2b 100644 --- a/lib/gears/object.lua +++ b/lib/gears/object.lua @@ -38,7 +38,10 @@ function object:add_signal(name) check(self) assert(type(name) == "string", "name must be a string, got: " .. type(name)) if not self._signals[name] then - self._signals[name] = {} + self._signals[name] = { + strong = {}, + weak = setmetatable({}, { __mode = "k" }) + } end end @@ -48,7 +51,19 @@ end function object:connect_signal(name, func) assert(type(func) == "function", "callback must be a function, got: " .. type(func)) local sig = find_signal(self, name, "connect to") - sig[func] = func + assert(sig.weak[func] == nil, "Trying to connect a strong callback which is already connected weakly") + sig.strong[func] = true +end + +--- Connect to a signal weakly. This allows the callback function to be garbage +-- collected and automatically disconnects the signal when that happens. +-- @param name The name of the signal +-- @param func The callback to call when the signal is emitted +function object:weak_connect_signal(name, func) + assert(type(func) == "function", "callback must be a function, got: " .. type(func)) + local sig = find_signal(self, name, "connect to") + assert(sig.strong[func] == nil, "Trying to connect a weak callback which is already connected strongly") + sig.weak[func] = true end --- Disonnect to a signal @@ -56,7 +71,8 @@ end -- @param func The callback that should be disconnected function object:disconnect_signal(name, func) local sig = find_signal(self, name, "disconnect from") - sig[func] = nil + sig.weak[func] = nil + sig.strong[func] = nil end --- Emit a signal @@ -67,7 +83,10 @@ end -- that are given to emit_signal() function object:emit_signal(name, ...) local sig = find_signal(self, name, "emit") - for func in pairs(sig) do + for func in pairs(sig.strong) do + func(self, ...) + end + for func in pairs(sig.weak) do func(self, ...) end end diff --git a/spec/gears/object_spec.lua b/spec/gears/object_spec.lua index 45fc6df7e..692e9fd19 100644 --- a/spec/gears/object_spec.lua +++ b/spec/gears/object_spec.lua @@ -12,13 +12,19 @@ describe("gears.object", function() obj:add_signal("signal") end) - it("connect non-existent signal", function() + it("strong connect non-existent signal", function() assert.has.errors(function() obj:connect_signal("foo", function() end) end) end) - it("disconnect non-existent signal", function() + it("weak connect non-existent signal", function() + assert.has.errors(function() + obj:weak_connect_signal("foo", function() end) + end) + end) + + it("strong disconnect non-existent signal", function() assert.has.errors(function() obj:disconnect_signal("foo", function() end) end) @@ -30,16 +36,27 @@ describe("gears.object", function() end) end) - it("connecting and emitting signal", function() + it("strong connecting and emitting signal", function() local called = false - obj:connect_signal("signal", function() + local function cb() called = true - end) + end + obj:connect_signal("signal", cb) obj:emit_signal("signal") assert.is_true(called) end) - it("connecting, disconnecting and emitting signal", function() + it("weak connecting and emitting signal", function() + local called = false + local function cb() + called = true + end + obj:weak_connect_signal("signal", cb) + obj:emit_signal("signal") + assert.is_true(called) + end) + + it("strong connecting, disconnecting and emitting signal", function() local called = false local function cb() called = true @@ -50,13 +67,64 @@ describe("gears.object", function() assert.is_false(called) end) + it("weak connecting, disconnecting and emitting signal", function() + local called = false + local function cb() + called = true + end + obj:weak_connect_signal("signal", cb) + obj:disconnect_signal("signal", cb) + obj:emit_signal("signal") + assert.is_false(called) + end) + it("arguments to signal", function() obj:connect_signal("signal", function(arg1, arg2) assert.is.equal(obj, arg1) assert.is.same(42, arg2) end) + obj:weak_connect_signal("signal", function(arg1, arg2) + assert.is.equal(obj, arg1) + assert.is.same(42, arg2) + end) obj:emit_signal("signal", 42) end) + + it("strong non-auto disconnect", function() + local called = false + obj:connect_signal("signal", function() + called = true + end) + collectgarbage("collect") + obj:emit_signal("signal") + assert.is_true(called) + end) + + it("weak auto disconnect", function() + local called = false + obj:weak_connect_signal("signal", function() + called = true + end) + collectgarbage("collect") + obj:emit_signal("signal") + assert.is_false(called) + end) + + it("strong connect after weak connect", function() + local function cb() end + obj:weak_connect_signal("signal", cb) + assert.has.errors(function() + obj:connect_signal("signal", cb) + end) + end) + + it("weak connect after strong connect", function() + local function cb() end + obj:connect_signal("signal", cb) + assert.has.errors(function() + obj:weak_connect_signal("signal", cb) + end) + end) end) -- vim: filetype=lua:expandtab:shiftwidth=4:tabstop=8:softtabstop=4:textwidth=80 From d990e7918fd6dcf6fea98a1ac528c8fd95a03955 Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Sun, 14 Jun 2015 14:27:43 +0200 Subject: [PATCH 2/2] Use :weak_connect_signal() for connecting to widget::updated This way "that other widget" doesn't prevent the current widget from being garbage collected. Please note that this in all of these cases the widget under consideration does have a strong reference to the callback function. This means that the callback cannot be garbage collected until "this widget" itself is collected. Thanks to this, this change is safe. Signed-off-by: Uli Schlachter --- lib/wibox/drawable.lua | 2 +- lib/wibox/layout/align.lua | 2 +- lib/wibox/layout/constraint.lua | 2 +- lib/wibox/layout/fixed.lua | 2 +- lib/wibox/layout/flex.lua | 2 +- lib/wibox/layout/margin.lua | 2 +- lib/wibox/layout/mirror.lua | 2 +- lib/wibox/layout/rotate.lua | 2 +- lib/wibox/widget/background.lua | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/wibox/drawable.lua b/lib/wibox/drawable.lua index 76ee284cc..b5ac9ba13 100644 --- a/lib/wibox/drawable.lua +++ b/lib/wibox/drawable.lua @@ -120,7 +120,7 @@ function drawable:set_widget(widget) self.widget = widget if widget then - widget:connect_signal("widget::updated", self.draw) + widget:weak_connect_signal("widget::updated", self.draw) end -- Make sure the widget gets drawn diff --git a/lib/wibox/layout/align.lua b/lib/wibox/layout/align.lua index 2c03715f7..de4682e9e 100644 --- a/lib/wibox/layout/align.lua +++ b/lib/wibox/layout/align.lua @@ -140,7 +140,7 @@ local function widget_changed(layout, old_w, new_w) end if new_w then widget_base.check_widget(new_w) - new_w:connect_signal("widget::updated", layout._emit_updated) + new_w:weak_connect_signal("widget::updated", layout._emit_updated) end layout._emit_updated() end diff --git a/lib/wibox/layout/constraint.lua b/lib/wibox/layout/constraint.lua index 0aeda1ca0..34b552bd0 100644 --- a/lib/wibox/layout/constraint.lua +++ b/lib/wibox/layout/constraint.lua @@ -48,7 +48,7 @@ function constraint:set_widget(widget) end if widget then widget_base.check_widget(widget) - widget:connect_signal("widget::updated", self._emit_updated) + widget:weak_connect_signal("widget::updated", self._emit_updated) end self.widget = widget self:emit_signal("widget::updated") diff --git a/lib/wibox/layout/fixed.lua b/lib/wibox/layout/fixed.lua index 4b8a03e75..5329e1fd6 100644 --- a/lib/wibox/layout/fixed.lua +++ b/lib/wibox/layout/fixed.lua @@ -54,7 +54,7 @@ end function fixed:add(widget) widget_base.check_widget(widget) table.insert(self.widgets, widget) - widget:connect_signal("widget::updated", self._emit_updated) + widget:weak_connect_signal("widget::updated", self._emit_updated) self._emit_updated() end diff --git a/lib/wibox/layout/flex.lua b/lib/wibox/layout/flex.lua index 528d7061f..55107c9d0 100644 --- a/lib/wibox/layout/flex.lua +++ b/lib/wibox/layout/flex.lua @@ -62,7 +62,7 @@ end function flex:add(widget) widget_base.check_widget(widget) table.insert(self.widgets, widget) - widget:connect_signal("widget::updated", self._emit_updated) + widget:weak_connect_signal("widget::updated", self._emit_updated) self._emit_updated() end diff --git a/lib/wibox/layout/margin.lua b/lib/wibox/layout/margin.lua index 87a8ff85b..2a1a0df36 100644 --- a/lib/wibox/layout/margin.lua +++ b/lib/wibox/layout/margin.lua @@ -58,7 +58,7 @@ function margin:set_widget(widget) end if widget then widget_base.check_widget(widget) - widget:connect_signal("widget::updated", self._emit_updated) + widget:weak_connect_signal("widget::updated", self._emit_updated) end self.widget = widget self._emit_updated() diff --git a/lib/wibox/layout/mirror.lua b/lib/wibox/layout/mirror.lua index 41b17a358..06211d80a 100644 --- a/lib/wibox/layout/mirror.lua +++ b/lib/wibox/layout/mirror.lua @@ -60,7 +60,7 @@ function mirror:set_widget(widget) end if widget then widget_base.check_widget(widget) - widget:connect_signal("widget::updated", self._emit_updated) + widget:weak_connect_signal("widget::updated", self._emit_updated) end self.widget = widget self._emit_updated() diff --git a/lib/wibox/layout/rotate.lua b/lib/wibox/layout/rotate.lua index 3a25ef436..e8beef799 100644 --- a/lib/wibox/layout/rotate.lua +++ b/lib/wibox/layout/rotate.lua @@ -61,7 +61,7 @@ function rotate:set_widget(widget) end if widget then widget_base.check_widget(widget) - widget:connect_signal("widget::updated", self._emit_updated) + widget:weak_connect_signal("widget::updated", self._emit_updated) end self.widget = widget self._emit_updated() diff --git a/lib/wibox/widget/background.lua b/lib/wibox/widget/background.lua index 0d85b3dc8..8aa701cdc 100644 --- a/lib/wibox/widget/background.lua +++ b/lib/wibox/widget/background.lua @@ -62,7 +62,7 @@ function background:set_widget(widget) end if widget then base.check_widget(widget) - widget:connect_signal("widget::updated", self._emit_updated) + widget:weak_connect_signal("widget::updated", self._emit_updated) end self.widget = widget self._emit_updated()