From a18348542cd04d4c4bcd3e3e19096f77673cefb5 Mon Sep 17 00:00:00 2001
From: Emmanuel Lepage Vallee <elv1313@gmail.com>
Date: Sat, 27 Apr 2019 15:45:56 -0400
Subject: [PATCH] shims: Enforce working on valid screen objects.

Previously it was possible to manipulate deleted screens and that
made debugging harder down the line. By catching this early, it
wont be as nightmarish.
---
 tests/examples/shims/mouse.lua  | 14 ++++++++++---
 tests/examples/shims/screen.lua | 37 ++++++++++++++++++++++++++++++---
 2 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/tests/examples/shims/mouse.lua b/tests/examples/shims/mouse.lua
index 20c35f0d..13867f4d 100644
--- a/tests/examples/shims/mouse.lua
+++ b/tests/examples/shims/mouse.lua
@@ -53,9 +53,14 @@ function mouse.push_history()
     mouse.history = {}
 end
 
+local forced_screen = nil
+
 return setmetatable(mouse, {
     __index = function(self, key)
         if key == "screen" then
+            if forced_screen and screen._deleted[forced_screen] then
+                forced_screen = nil
+            end
             return screen[1]
         end
         local h = rawget(mouse,"_i_handler")
@@ -63,10 +68,13 @@ return setmetatable(mouse, {
             return h(self, key)
         end
     end,
-    __newindex = function(...)
+    __newindex = function(_, k, v)
         local h = rawget(mouse,"_ni_handler")
-        if h then
-            h(...)
+        if k == "screen" then
+            -- This will assert if the screen is invalid
+            forced_screen = v and screen[v] or nil
+        elseif h then
+            h(_, k, v)
         end
     end,
 })
diff --git a/tests/examples/shims/screen.lua b/tests/examples/shims/screen.lua
index a84c0796..8a428937 100644
--- a/tests/examples/shims/screen.lua
+++ b/tests/examples/shims/screen.lua
@@ -1,7 +1,7 @@
 local gears_obj = require("gears.object")
 
 local screen, meta = awesome._shim_fake_class()
-screen._count = 0
+screen._count, screen._deleted = 0, {}
 
 local function create_screen(args)
     local s = gears_obj()
@@ -41,6 +41,8 @@ local function create_screen(args)
     end
 
     return setmetatable(s,{ __index = function(_, key)
+            assert(s.valid)
+
             if key == "geometry" then
                 return {
                     x      = geo.x or 0,
@@ -59,7 +61,7 @@ local function create_screen(args)
                 return meta.__index(_, key)
             end
         end,
-        __newindex = function(...) return meta.__newindex(...) end
+        __newindex = function(...) assert(s.valid); return meta.__newindex(...) end
     })
 end
 
@@ -91,8 +93,36 @@ function screen._get_extents()
     return xmax.geometry.x+xmax.geometry.width, ymax.geometry.y+ymax.geometry.height
 end
 
+-- The way the `screen` module is used to store both number and object is
+-- problematic since it can cause invalid screens to be "resurrected".
+local function catch_invalid(_, s)
+    -- The CAPI implementation allows `nil`
+    if s == nil or type(s) == "string" then return end
+
+    assert(screen ~= s, "Some code tried (and failed) to shadow the global `screen`")
+
+    -- Valid numbers wont get this far.
+    assert(type(s) == "table", "Expected a table, but got a "..type(s))
+
+    if type(s) == "number" then return end
+
+    assert(s.geometry, "The object is not a screen")
+    assert(s.outputs, "The object is not a screen")
+
+    assert((not screen._deleted[s]) or (not s.valid), "The shims are broken")
+
+    assert(not screen._deleted[s], "The screen "..tostring(s).."has been deleted")
+
+    -- Other errors. If someone place an object in the `screen` module, it wont
+    -- get this far. So the remaining cases are probably bugs.
+    assert(false)
+end
+
 function screen._clear()
+    assert(#screen == screen._count)
     for i=1, #screen do
+        screen._deleted[screen[i]] = true
+        screen[i].valid = false
         screen[screen[i]] = nil
         screen[i] = nil
     end
@@ -144,7 +174,8 @@ function screen.count()
 end
 
 return setmetatable(screen, {
-    __call = iter_scr
+    __call  = iter_scr,
+    __index = catch_invalid
 })
 
 -- vim: filetype=lua:expandtab:shiftwidth=4:tabstop=8:softtabstop=4:textwidth=80