From 90044e00da0b9b2f4bb53cf8811f32cfa3a3a106 Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Wed, 5 Oct 2016 20:35:04 +0200 Subject: [PATCH 1/6] wibox: Remove weak table hack No idea what self referencing loops this refers to. Lua 5.1's and LuaJIT's garbage collector both should handle cycles just fine. Things only start getting complicated when you start using weak tables. Unless someone comes up with an example where this patch causes a leak, let's remove the weak table magic. Signed-off-by: Uli Schlachter --- lib/wibox/init.lua | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/wibox/init.lua b/lib/wibox/init.lua index a078534e9..f186a61bb 100644 --- a/lib/wibox/init.lua +++ b/lib/wibox/init.lua @@ -158,11 +158,8 @@ local function new(args) local ret = object() local w = capi.drawin(args) - -- lua 5.1 and luajit have issues with self referencing loops - local avoid_leak = setmetatable({ret},{__mode="v"}) - function w.get_wibox() - return avoid_leak[1] + return ret end ret.drawin = w From 22d1375e5f4cdec3e25bc1a993c3b30ed2d23154 Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Wed, 5 Oct 2016 20:41:18 +0200 Subject: [PATCH 2/6] awful.client: Remove persistent_properties_loaded Instead of having an extra weak table to save a boolean per client, this now sets a property directly on the client. Signed-off-by: Uli Schlachter --- lib/awful/client.lua | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/awful/client.lua b/lib/awful/client.lua index 334df5169..81c762cb7 100644 --- a/lib/awful/client.lua +++ b/lib/awful/client.lua @@ -48,7 +48,6 @@ client.data = {} client.data.marked = {} client.data.properties = setmetatable({}, { __mode = 'k' }) client.data.persistent_properties_registered = {} -- keys are names of persistent properties, value always true -client.data.persistent_properties_loaded = setmetatable({}, { __mode = 'k' }) -- keys are clients, value always true -- Functions client.urgent = require("awful.client.urgent") @@ -986,8 +985,8 @@ end -- @return The property. -- @deprecated awful.client.property.get function client.property.get(c, prop) - if not client.data.persistent_properties_loaded[c] then - client.data.persistent_properties_loaded[c] = true + if not c.data._persistent_properties_loaded then + c.data._persistent_properties_loaded = true for p in pairs(client.data.persistent_properties_registered) do local value = c:get_xproperty("awful.client.property." .. p) if value ~= nil then From 3d048dca04c115546fbde2cf74246bac74e93583 Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Wed, 5 Oct 2016 20:55:28 +0200 Subject: [PATCH 3/6] awful.client: Save client properties under c.data Instead of using a weak table with some magic to save properties of a client, the code now uses the c.data table provided by the C code instead. Signed-off-by: Uli Schlachter --- lib/awful/client.lua | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/lib/awful/client.lua b/lib/awful/client.lua index 81c762cb7..6e6a28b2a 100644 --- a/lib/awful/client.lua +++ b/lib/awful/client.lua @@ -46,7 +46,6 @@ local client = {object={}} -- Private data client.data = {} client.data.marked = {} -client.data.properties = setmetatable({}, { __mode = 'k' }) client.data.persistent_properties_registered = {} -- keys are names of persistent properties, value always true -- Functions @@ -994,8 +993,8 @@ function client.property.get(c, prop) end end end - if client.data.properties[c] then - return client.data.properties[c][prop] + if c.data.awful_client_properties then + return c.data.awful_client_properties[prop] end end @@ -1009,14 +1008,14 @@ end -- @param value The value. -- @deprecated awful.client.property.set function client.property.set(c, prop, value) - if not client.data.properties[c] then - client.data.properties[c] = {} + if not c.data.awful_client_properties then + c.data.awful_client_properties = {} end - if client.data.properties[c][prop] ~= value then + if c.data.awful_client_properties[prop] ~= value then if client.data.persistent_properties_registered[prop] then c:set_xproperty("awful.client.property." .. prop, value) end - client.data.properties[c][prop] = value + c.data.awful_client_properties[prop] = value c:emit_signal("property::" .. prop) end end @@ -1033,9 +1032,9 @@ function client.property.persist(prop, kind) client.data.persistent_properties_registered[prop] = true -- Make already-set properties persistent - for c in pairs(client.data.properties) do - if client.data.properties[c] and client.data.properties[c][prop] ~= nil then - c:set_xproperty(xprop, client.data.properties[c][prop]) + for c in pairs(capi.client.get()) do + if c.data.awful_client_properties and c.data.awful_client_properties[prop] ~= nil then + c:set_xproperty(xprop, c.data.awful_client_properties[prop]) end end end From 4ef63d94162e5b5fedf8781a407f7c6e4d921703 Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Wed, 5 Oct 2016 21:00:47 +0200 Subject: [PATCH 4/6] awful.screen: Save last mouse position as screen property Instead of using a weak table to save the last mouse position, this is now saved directly as a property under the screen. Signed-off-by: Uli Schlachter --- lib/awful/screen.lua | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/awful/screen.lua b/lib/awful/screen.lua index 9d5930291..64fd6d548 100644 --- a/lib/awful/screen.lua +++ b/lib/awful/screen.lua @@ -31,8 +31,6 @@ local screen = {object={}} local data = {} data.padding = {} -screen.mouse_per_screen = setmetatable({}, {__mode="k"}) - --- Take an input geometry and substract/add a delta. -- @tparam table geo A geometry (width, height, x, y) table. -- @tparam table delta A delta table (top, bottom, x, y). @@ -99,7 +97,7 @@ function screen.focus(_screen) local s = get_screen(capi.mouse.screen) local pos - if not screen.mouse_per_screen[_screen] then + if not _screen.mouse_per_screen then -- This is the first time we enter this screen, -- keep relative mouse position on the new screen. pos = capi.mouse.coords() @@ -110,11 +108,11 @@ function screen.focus(_screen) pos.y = _screen.geometry.y + rely * _screen.geometry.height else -- restore mouse position - pos = screen.mouse_per_screen[_screen] + pos = _screen.mouse_per_screen end -- save pointer position of current screen - screen.mouse_per_screen[s] = capi.mouse.coords() + s.mouse_per_screen = capi.mouse.coords() -- move cursor without triggering signals mouse::enter and mouse::leave capi.mouse.coords(pos, true) From bf97cb6bfedaf64ae99bf8c7a824d6c2983433b2 Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Wed, 5 Oct 2016 21:03:34 +0200 Subject: [PATCH 5/6] awful.tag: Save dynamic_cache as a tag property Instead of using magic with a weak table, the code now saves this cache as a property under the tag object. Signed-off-by: Uli Schlachter --- lib/awful/tag.lua | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/awful/tag.lua b/lib/awful/tag.lua index 20b9c8e5e..466172023 100644 --- a/lib/awful/tag.lua +++ b/lib/awful/tag.lua @@ -35,7 +35,6 @@ local tag = {object = {}, mt = {} } -- Private data local data = {} data.history = {} -data.dynamic_cache = setmetatable({}, { __mode = 'k' }) data.tags = setmetatable({}, { __mode = 'k' }) -- History functions @@ -710,11 +709,11 @@ function tag.object.set_layout(t, layout) and getmetatable(layout) and getmetatable(layout).__call ) then - if not data.dynamic_cache[t] then - data.dynamic_cache[t] = {} + if not t.dynamic_layout_cache then + t.dynamic_layout_cache = {} end - local instance = data.dynamic_cache[t][layout] or layout(t) + local instance = t.dynamic_layout_cache[layout] or layout(t) -- Always make sure the layout is notified it is enabled if tag.getproperty(t, "screen").selected_tag == t and instance.wake_up then @@ -723,7 +722,7 @@ function tag.object.set_layout(t, layout) -- Avoid creating the same layout twice, use layout:reset() to reset if instance.is_dynamic then - data.dynamic_cache[t][layout] = instance + t.dynamic_layout_cache[layout] = instance end layout = instance From ab789e57a9990fc23ebef04b2e079676d7f8f7c0 Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Wed, 5 Oct 2016 21:16:33 +0200 Subject: [PATCH 6/6] awful.tag: Save all "generic" tag properties as real props Instead of using magic with a weak table, the code now saves this data as a property under the tag object. This avoids all kinds of leaks, for example caused by t.foo = t. Signed-off-by: Uli Schlachter --- lib/awful/tag.lua | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/awful/tag.lua b/lib/awful/tag.lua index 466172023..79193fda3 100644 --- a/lib/awful/tag.lua +++ b/lib/awful/tag.lua @@ -35,7 +35,6 @@ local tag = {object = {}, mt = {} } -- Private data local data = {} data.history = {} -data.tags = setmetatable({}, { __mode = 'k' }) -- History functions tag.history = {} @@ -216,7 +215,7 @@ function tag.add(name, props) local newtag = capi.tag{ name = name } -- Start with a fresh property table to avoid collisions with unsupported data - data.tags[newtag] = {screen=properties.screen, index=properties.index} + newtag.data.awful_tag_properties = {screen=properties.screen, index=properties.index} newtag.activated = true @@ -322,7 +321,7 @@ function tag.object.delete(self, fallback_tag, force) end -- delete the tag - data.tags[self].screen = nil + self.data.awful_tag_properties.screen = nil self.activated = false -- Update all indexes @@ -1277,7 +1276,7 @@ end -- @tparam tag _tag The tag. -- @return The data table. function tag.getdata(_tag) - return data.tags[_tag] + return _tag.data.awful_tag_properties end --- Get a tag property. @@ -1289,8 +1288,9 @@ end -- @tparam string prop The property name. -- @return The property. function tag.getproperty(_tag, prop) - if data.tags[_tag] then - return data.tags[_tag][prop] + if not _tag then return end -- FIXME: Turn this into an error? + if _tag.data.awful_tag_properties then + return _tag.data.awful_tag_properties[prop] end end @@ -1305,12 +1305,12 @@ end -- @param prop The property name. -- @param value The value. function tag.setproperty(_tag, prop, value) - if not data.tags[_tag] then - data.tags[_tag] = {} + if not _tag.data.awful_tag_properties then + _tag.data.awful_tag_properties = {} end - if data.tags[_tag][prop] ~= value then - data.tags[_tag][prop] = value + if _tag.data.awful_tag_properties[prop] ~= value then + _tag.data.awful_tag_properties[prop] = value _tag:emit_signal("property::" .. prop) end end @@ -1462,8 +1462,8 @@ capi.screen.connect_signal("removed", function(s) for _, t in pairs(s.tags) do t.activated = false - if data.tags[t] then - data.tags[t].screen = nil + if t.data.awful_tag_properties then + t.data.awful_tag_properties.screen = nil end end end)