diff --git a/frontend/apps/filemanager/filemanager.lua b/frontend/apps/filemanager/filemanager.lua index 2ff24882a..23a3c7607 100644 --- a/frontend/apps/filemanager/filemanager.lua +++ b/frontend/apps/filemanager/filemanager.lua @@ -544,7 +544,7 @@ function FileManager:init() if FileManager.instance == nil then logger.dbg("Spinning up new FileManager instance", tostring(self)) else - -- Should never happen, given what we did above... + -- Should never happen, given what we did in showFiles... logger.err("FileManager instance mismatch! Opened", tostring(self), "while we still have an existing instance:", tostring(FileManager.instance), debug.traceback()) end FileManager.instance = self @@ -1159,6 +1159,7 @@ function FileManager:getStartWithMenuTable() } end +--- @note: This is the *only* safe way to instantiate a new FileManager instance! function FileManager:showFiles(path, focused_file) -- Warn about and close any pre-existing FM instances first... if FileManager.instance then diff --git a/frontend/apps/reader/readerui.lua b/frontend/apps/reader/readerui.lua index eddd2e14d..d561da097 100644 --- a/frontend/apps/reader/readerui.lua +++ b/frontend/apps/reader/readerui.lua @@ -443,7 +443,7 @@ function ReaderUI:init() if ReaderUI.instance == nil then logger.dbg("Spinning up new ReaderUI instance", tostring(self)) else - -- Should never happen, given what we did above... + -- Should never happen, given what we did in (do)showReader... logger.err("ReaderUI instance mismatch! Opened", tostring(self), "while we still have an existing instance:", tostring(ReaderUI.instance), debug.traceback()) end ReaderUI.instance = self @@ -504,6 +504,8 @@ function ReaderUI:onSetupShowReader() end --- @note: Will sanely close existing FileManager/ReaderUI instance for you! +--- This is the *only* safe way to instantiate a new ReaderUI instance! +--- (i.e., don't look at the testsuite, which resorts to all kinds of nasty hacks). function ReaderUI:showReader(file, provider) logger.dbg("show reader ui") diff --git a/spec/unit/filemanager_spec.lua b/spec/unit/filemanager_spec.lua index c995ca7cc..730afa7d6 100644 --- a/spec/unit/filemanager_spec.lua +++ b/spec/unit/filemanager_spec.lua @@ -18,7 +18,7 @@ describe("FileManager module", function() root_path = "../../test", } UIManager:show(filemanager) - UIManager:scheduleIn(1, function() UIManager:close(filemanager) end) + UIManager:scheduleIn(1, function() filemanager:onClose() end) UIManager:run() end) it("should show error on non-existent file", function() @@ -34,6 +34,7 @@ describe("FileManager module", function() assert.is_nil(lfs.attributes(tmp_fn)) filemanager:deleteFile(tmp_fn) UIManager.show = old_show + filemanager:onClose() end) it("should not delete settings for non-document file", function() local filemanager = FileManager:new{ @@ -62,6 +63,7 @@ describe("FileManager module", function() end filemanager:deleteFile(tmp_fn) UIManager.show = old_show + filemanager:onClose() -- make sure history file exists assert.is_nil(lfs.attributes(tmp_fn)) @@ -97,6 +99,7 @@ describe("FileManager module", function() end filemanager:deleteFile(tmp_fn) UIManager.show = old_show + filemanager:onClose() assert.is_nil(lfs.attributes(tmp_fn)) assert.is_nil(lfs.attributes(tmp_sidecar)) diff --git a/spec/unit/readerbookmark_spec.lua b/spec/unit/readerbookmark_spec.lua index 535e340ee..0dc22fc4e 100644 --- a/spec/unit/readerbookmark_spec.lua +++ b/spec/unit/readerbookmark_spec.lua @@ -24,14 +24,17 @@ describe("ReaderBookmark module", function() UIManager:nextTick(function() UIManager:close(readerui.highlight.highlight_dialog) UIManager:close(readerui) + -- We haven't torn it down yet + ReaderUI.instance = readerui end) UIManager:run() end local function toggler_dogear(readerui) readerui.bookmark:onToggleBookmark() - --- @todo Replace scheduleIn with nextTick - UIManager:scheduleIn(1, function() + UIManager:nextTick(function() UIManager:close(readerui) + -- We haven't torn it down yet + ReaderUI.instance = readerui end) UIManager:run() end @@ -39,6 +42,8 @@ describe("ReaderBookmark module", function() UIManager:nextTick(function() UIManager:close(readerui.bookmark.bookmark_menu) UIManager:close(readerui) + -- We haven't torn it down yet + ReaderUI.instance = readerui end) UIManager:run() end diff --git a/spec/unit/readerdictionary_spec.lua b/spec/unit/readerdictionary_spec.lua index 5902d90fa..ac8bc645c 100644 --- a/spec/unit/readerdictionary_spec.lua +++ b/spec/unit/readerdictionary_spec.lua @@ -32,6 +32,8 @@ describe("Readerdictionary module", function() UIManager:scheduleIn(1, function() UIManager:close(dictionary.dict_window) UIManager:close(readerui) + -- We haven't torn it down yet + ReaderUI.instance = readerui end) UIManager:run() Screen:shot(name) diff --git a/spec/unit/readerhighlight_spec.lua b/spec/unit/readerhighlight_spec.lua index c621e15cb..a7423ac6b 100644 --- a/spec/unit/readerhighlight_spec.lua +++ b/spec/unit/readerhighlight_spec.lua @@ -19,6 +19,8 @@ describe("Readerhighlight module", function() UIManager:scheduleIn(1, function() UIManager:close(readerui.dictionary.dict_window) UIManager:close(readerui) + -- We haven't torn it down yet + ReaderUI.instance = readerui UIManager:quit() end) UIManager:run() @@ -43,6 +45,8 @@ describe("Readerhighlight module", function() UIManager:scheduleIn(1, function() UIManager:close(readerui.highlight.highlight_dialog) UIManager:close(readerui) + -- We haven't torn it down yet + ReaderUI.instance = readerui UIManager:quit() end) UIManager:run() @@ -59,6 +63,8 @@ describe("Readerhighlight module", function() UIManager:nextTick(function() UIManager:close(readerui.highlight.edit_highlight_dialog) UIManager:close(readerui) + -- We haven't torn it down yet + ReaderUI.instance = readerui UIManager:quit() end) UIManager:run() @@ -183,6 +189,8 @@ describe("Readerhighlight module", function() readerui.highlight:clear() readerui.document.configurable.text_wrap = 0 UIManager:close(readerui) -- close to flush settings + -- We haven't torn it down yet + ReaderUI.instance = readerui end) it("should highlight single word", function() highlight_single_word(readerui, Geom:new{ x = 260, y = 70 }) @@ -273,6 +281,8 @@ describe("Readerhighlight module", function() readerui.highlight:clear() readerui.document.configurable.text_wrap = 0 UIManager:close(readerui) -- close to flush settings + -- We haven't torn it down yet + ReaderUI.instance = readerui end) it("should highlight single word", function() highlight_single_word(readerui, Geom:new{ x = 260, y = 70 }) diff --git a/spec/unit/readerpaging_spec.lua b/spec/unit/readerpaging_spec.lua index 6c35694fd..f2a0cfa53 100644 --- a/spec/unit/readerpaging_spec.lua +++ b/spec/unit/readerpaging_spec.lua @@ -28,7 +28,11 @@ describe("Readerpaging module", function() it("should emit EndOfBook event at the end", function() UIManager:quit() UIManager:show(readerui) - UIManager:nextTick(function() UIManager:close(readerui) end) + UIManager:nextTick(function() + UIManager:close(readerui) + -- We haven't torn it down yet + ReaderUI.instance = readerui + end) UIManager:run() readerui:handleEvent(Event:new("SetScrollMode", false)) readerui.zooming:setZoomMode("pageheight") @@ -65,7 +69,11 @@ describe("Readerpaging module", function() it("should emit EndOfBook event at the end", function() UIManager:quit() UIManager:show(readerui) - UIManager:nextTick(function() UIManager:close(readerui) end) + UIManager:nextTick(function() + UIManager:close(readerui) + -- We haven't torn it down yet + ReaderUI.instance = readerui + end) UIManager:run() paging.page_positions = {} readerui:handleEvent(Event:new("SetScrollMode", true)) @@ -84,6 +92,7 @@ describe("Readerpaging module", function() it("should scroll backward on the first page without crash", function() local sample_djvu = "spec/front/unit/data/djvu3spec.djvu" + -- Unsafe second // ReaderUI instance! local tmp_readerui = ReaderUI:new{ dimen = Screen:getSize(), document = DocumentRegistry:openDocument(sample_djvu), @@ -91,10 +100,13 @@ describe("Readerpaging module", function() tmp_readerui.paging:onScrollPanRel(-100) tmp_readerui:closeDocument() tmp_readerui:onClose() + -- Restore the ref to the original ReaderUI instance + ReaderUI.instance = readerui end) it("should scroll forward on the last page without crash", function() local sample_djvu = "spec/front/unit/data/djvu3spec.djvu" + -- Unsafe second // ReaderUI instance! local tmp_readerui = ReaderUI:new{ dimen = Screen:getSize(), document = DocumentRegistry:openDocument(sample_djvu), @@ -106,6 +118,8 @@ describe("Readerpaging module", function() paging:onScrollPanRel(120) tmp_readerui:closeDocument() tmp_readerui:onClose() + -- Restore the ref to the original ReaderUI instance + ReaderUI.instance = readerui end) end) end) diff --git a/spec/unit/readerrolling_spec.lua b/spec/unit/readerrolling_spec.lua index daddf056c..964351fbe 100644 --- a/spec/unit/readerrolling_spec.lua +++ b/spec/unit/readerrolling_spec.lua @@ -80,6 +80,7 @@ describe("Readerrolling module", function() it("should emit EndOfBook event at the end sample txt", function() local sample_txt = "spec/front/unit/data/sample.txt" + -- Unsafe second // ReaderUI instance! local txt_readerui = ReaderUI:new{ dimen = Screen:getSize(), document = DocumentRegistry:openDocument(sample_txt), @@ -108,6 +109,8 @@ describe("Readerrolling module", function() readerui.onEndOfBook = nil txt_readerui:closeDocument() txt_readerui:onClose() + -- Restore the ref to the original ReaderUI instance + ReaderUI.instance = readerui end) end) @@ -207,14 +210,14 @@ describe("Readerrolling module", function() end local test_book = "spec/front/unit/data/sample.txt" require("docsettings"):open(test_book):purge() + readerui:closeDocument() + readerui:onClose() local tmp_readerui = ReaderUI:new{ document = DocumentRegistry:openDocument(test_book), } ReaderView.onPageUpdate = saved_handler tmp_readerui:closeDocument() tmp_readerui:onClose() - readerui:closeDocument() - readerui:onClose() end) end) end) diff --git a/spec/unit/readerui_spec.lua b/spec/unit/readerui_spec.lua index 289128e98..1def3804c 100644 --- a/spec/unit/readerui_spec.lua +++ b/spec/unit/readerui_spec.lua @@ -30,7 +30,11 @@ describe("Readerui module", function() it("should show reader", function() UIManager:quit() UIManager:show(readerui) - UIManager:scheduleIn(1, function() UIManager:close(readerui) end) + UIManager:scheduleIn(1, function() + UIManager:close(readerui) + -- We haven't torn it down yet + ReaderUI.instance = readerui + end) UIManager:run() end) it("should close document", function() @@ -38,10 +42,13 @@ describe("Readerui module", function() assert(readerui.document == nil) readerui:onClose() end) - it("should not reset running_instance by mistake", function() - ReaderUI:doShowReader(sample_epub) + it("should not reset ReaderUI.instance by mistake", function() + ReaderUI:doShowReader(sample_epub) -- spins up a new, sane instance local new_readerui = ReaderUI:_getRunningInstance() assert.is.truthy(new_readerui.document) + -- This *will* trip: + -- * A pair of ReaderUI instance mimsatch warnings (on open/close) because it bypasses the safety of doShowReader! + -- * A refcount warning from DocumentRegistry, because bypassinf the safeties means that two different instances opened the same Document. ReaderUI:new{ dimen = Screen:getSize(), document = DocumentRegistry:openDocument(sample_epub) diff --git a/spec/unit/screenshoter_spec.lua b/spec/unit/screenshoter_spec.lua index dbe53623f..000ddb56c 100644 --- a/spec/unit/screenshoter_spec.lua +++ b/spec/unit/screenshoter_spec.lua @@ -28,7 +28,11 @@ describe("ReaderScreenshot module", function() readerui:handleEvent(Event:new("SetRotationMode", Screen.ORIENTATION_PORTRAIT)) UIManager:quit() UIManager:show(readerui) - UIManager:scheduleIn(1, function() UIManager:close(readerui) end) + UIManager:scheduleIn(1, function() + UIManager:close(readerui) + -- We haven't torn it down yet + ReaderUI.instance = readerui + end) UIManager:run() readerui.screenshot:onScreenshot(name) assert.truthy(lfs.attributes(name, "mode")) @@ -40,7 +44,11 @@ describe("ReaderScreenshot module", function() readerui:handleEvent(Event:new("SetRotationMode", Screen.ORIENTATION_LANDSCAPE)) UIManager:quit() UIManager:show(readerui) - UIManager:scheduleIn(2, function() UIManager:close(readerui) end) + UIManager:scheduleIn(2, function() + UIManager:close(readerui) + -- We haven't torn it down yet + ReaderUI.instance = readerui + end) UIManager:run() readerui.screenshot:onScreenshot(name) assert.truthy(lfs.attributes(name, "mode"))