From 9cda84ef2ff5ad095fee0d7dd987596edc9a9c69 Mon Sep 17 00:00:00 2001 From: NiLuJe Date: Sat, 19 Dec 2020 05:32:23 +0100 Subject: [PATCH] Prevent Busted from blowing up on cdata finalizers by properly closing opened documents... --- frontend/device/kindle/powerd.lua | 21 +++++------ frontend/ui/renderimage.lua | 2 +- spec/unit/cache_spec.lua | 3 ++ spec/unit/evernote_plugin_main_spec.lua | 5 ++- spec/unit/readerbookmark_spec.lua | 8 +++++ spec/unit/readerdictionary_spec.lua | 4 +++ spec/unit/readerfooter_spec.lua | 48 +++++++++++++++++++++++++ spec/unit/readerhighlight_spec.lua | 12 +++++++ spec/unit/readerlink_spec.lua | 14 ++++++++ spec/unit/readerpaging_spec.lua | 12 +++++++ spec/unit/readerrolling_spec.lua | 8 ++++- spec/unit/readersearch_spec.lua | 16 ++++++--- spec/unit/readertoc_spec.lua | 10 ++++++ spec/unit/readerui_spec.lua | 2 ++ spec/unit/readerview_spec.lua | 8 +++++ spec/unit/screenshoter_spec.lua | 2 ++ 16 files changed, 155 insertions(+), 20 deletions(-) diff --git a/frontend/device/kindle/powerd.lua b/frontend/device/kindle/powerd.lua index cff22e6e3..d82e2d6ef 100644 --- a/frontend/device/kindle/powerd.lua +++ b/frontend/device/kindle/powerd.lua @@ -125,19 +125,6 @@ function KindlePowerD:isChargingHW() return is_charging == 1 end -local KindlePowerD_mt = {} -function KindlePowerD_mt:__gc() - if self.lipc_handle then - self.lipc_handle:close() - self.lipc_handle = nil - end -end - --- Funky ass newproxy() workaround to make __gc work on a plain table in Lua 5.1/LuaJIT --- c.f., https://github.com/katlogic/__gc -local setmetatable = require("ffi/__gc") -setmetatable(KindlePowerD, KindlePowerD_mt) - function KindlePowerD:_readFLIntensity() return self:read_int_file(self.fl_intensity_file) end @@ -167,4 +154,12 @@ function KindlePowerD:toggleSuspend() end end +--- @fixme: This won't ever fire, as KindlePowerD is already a metatable on a plain table. +function KindlePowerD:__gc() + if self.lipc_handle then + self.lipc_handle:close() + self.lipc_handle = nil + end +end + return KindlePowerD diff --git a/frontend/ui/renderimage.lua b/frontend/ui/renderimage.lua index be966cae4..5061d1f26 100644 --- a/frontend/ui/renderimage.lua +++ b/frontend/ui/renderimage.lua @@ -141,7 +141,7 @@ function RenderImage:renderGifImageDataWithGifLib(data, size, want_frames, width end end local setmetatable = require("ffi/__gc") - frames = setmetatable(frames, frames_mt) + setmetatable(frames, frames_mt) return frames else local page = gif:openPage(1) diff --git a/spec/unit/cache_spec.lua b/spec/unit/cache_spec.lua index a502e1e2b..3e8dc7d08 100644 --- a/spec/unit/cache_spec.lua +++ b/spec/unit/cache_spec.lua @@ -10,6 +10,9 @@ describe("Cache module", function() local sample_pdf = "spec/front/unit/data/sample.pdf" doc = DocumentRegistry:openDocument(sample_pdf) end) + teardown(function() + doc:close() + end) it("should clear cache", function() Cache:clear() diff --git a/spec/unit/evernote_plugin_main_spec.lua b/spec/unit/evernote_plugin_main_spec.lua index 4bdbe80ca..eef4aa40e 100644 --- a/spec/unit/evernote_plugin_main_spec.lua +++ b/spec/unit/evernote_plugin_main_spec.lua @@ -68,6 +68,9 @@ describe("Evernote plugin module", function() } end) + teardown(function() + readerui:onClose() + end) it("should write clippings to txt file", function () local file_mock = mock( { @@ -101,4 +104,4 @@ describe("Evernote plugin module", function() end) -end) \ No newline at end of file +end) diff --git a/spec/unit/readerbookmark_spec.lua b/spec/unit/readerbookmark_spec.lua index c03583f22..535e340ee 100644 --- a/spec/unit/readerbookmark_spec.lua +++ b/spec/unit/readerbookmark_spec.lua @@ -53,6 +53,10 @@ describe("ReaderBookmark module", function() } readerui.status.enabled = false end) + teardown(function() + readerui:closeDocument() + readerui:onClose() + end) before_each(function() UIManager:quit() UIManager:show(readerui) @@ -136,6 +140,10 @@ describe("ReaderBookmark module", function() } readerui.status.enabled = false end) + teardown(function() + readerui:closeDocument() + readerui:onClose() + end) before_each(function() UIManager:quit() UIManager:show(readerui) diff --git a/spec/unit/readerdictionary_spec.lua b/spec/unit/readerdictionary_spec.lua index c2f84a25f..5902d90fa 100644 --- a/spec/unit/readerdictionary_spec.lua +++ b/spec/unit/readerdictionary_spec.lua @@ -19,6 +19,10 @@ describe("Readerdictionary module", function() rolling = readerui.rolling dictionary = readerui.dictionary end) + teardown(function() + readerui:closeDocument() + readerui:onClose() + end) it("should show quick lookup window", function() local name = "screenshots/reader_dictionary.png" UIManager:quit() diff --git a/spec/unit/readerfooter_spec.lua b/spec/unit/readerfooter_spec.lua index c1ca6f318..ac512fed1 100644 --- a/spec/unit/readerfooter_spec.lua +++ b/spec/unit/readerfooter_spec.lua @@ -89,6 +89,8 @@ describe("Readerfooter module", function() } assert.is.same(true, readerui.view.footer_visible) G_reader_settings:delSetting("reader_footer_mode") + readerui:closeDocument() + readerui:onClose() end) it("should setup footer as visible not in all_at_once", function() @@ -116,6 +118,8 @@ describe("Readerfooter module", function() assert.is.same(true, readerui.view.footer_visible) assert.is.same(1, readerui.view.footer.mode, 1) G_reader_settings:delSetting("reader_footer_mode") + readerui:closeDocument() + readerui:onClose() end) it("should setup footer as invisible in full screen mode", function() @@ -133,6 +137,8 @@ describe("Readerfooter module", function() } assert.is.same(false, readerui.view.footer_visible) G_reader_settings:delSetting("reader_footer_mode") + readerui:closeDocument() + readerui:onClose() end) it("should setup footer as visible in mini progress bar mode", function() @@ -150,6 +156,8 @@ describe("Readerfooter module", function() } assert.is.same(false, readerui.view.footer_visible) G_reader_settings:delSetting("reader_footer_mode") + readerui:closeDocument() + readerui:onClose() end) it("should setup footer as invisible", function() @@ -167,6 +175,8 @@ describe("Readerfooter module", function() } assert.is.same(true, readerui.view.footer_visible) G_reader_settings:delSetting("reader_footer_mode") + readerui:closeDocument() + readerui:onClose() end) it("should setup footer for epub without error", function() @@ -186,6 +196,8 @@ describe("Readerfooter module", function() -- c.f., NOTE above, Statistics are disabled, hence the N/A results assert.are.same('1 / '..page_count..' | '..timeinfo..' | ⇒ 0 | 0% | ⤠ 0% | ⏳ N/A | ⤻ N/A', footer.footer_text.text) + readerui:closeDocument() + readerui:onClose() end) it("should setup footer for pdf without error", function() @@ -202,6 +214,8 @@ describe("Readerfooter module", function() local timeinfo = readerui.view.footer.textGeneratorMap.time(footer) assert.are.same('1 / 2 | '..timeinfo..' | ⇒ 1 | 0% | ⤠ 50% | ⏳ N/A | ⤻ N/A', readerui.view.footer.footer_text.text) + readerui:closeDocument() + readerui:onClose() end) it("should switch between different modes", function() @@ -257,6 +271,8 @@ describe("Readerfooter module", function() -- reenable chapter time to read, text should be chapter time to read tapFooterMenu(fake_menu, "Chapter time to read".." (⤻)") assert.are.same('⤻ N/A', footer.footer_text.text) + readerui:closeDocument() + readerui:onClose() end) it("should rotate through different modes", function() @@ -296,6 +312,8 @@ describe("Readerfooter module", function() -- Make it visible again to make the following tests behave... footer:onTapFooter() assert.is.same(1, footer.mode) + readerui:closeDocument() + readerui:onClose() end) it("should pick up screen resize in resetLayout", function() @@ -330,6 +348,8 @@ describe("Readerfooter module", function() expected = is_am() and 518 or 510 assert.is.same(expected, footer.progress_bar.width) Screen.getWidth = old_screen_getwidth + readerui:closeDocument() + readerui:onClose() end) it("should update width on PosUpdate event", function() @@ -353,6 +373,8 @@ describe("Readerfooter module", function() assert.are.same(expected, footer.progress_bar.width) expected = is_am() and 394 or 402 assert.are.same(expected, footer.text_width) + readerui:closeDocument() + readerui:onClose() end) it("should support chapter markers", function() @@ -378,6 +400,8 @@ describe("Readerfooter module", function() footer.settings.toc_markers = false footer:setTocMarkers() assert.are.same(nil, footer.progress_bar.ticks) + readerui:closeDocument() + readerui:onClose() end) it("should schedule/unschedule auto refresh time task", function() @@ -412,6 +436,8 @@ describe("Readerfooter module", function() end end assert.is.same(0, found) + readerui:closeDocument() + readerui:onClose() end) it("should not schedule auto refresh time task if footer is disabled", function() @@ -438,6 +464,8 @@ describe("Readerfooter module", function() end end assert.is.same(0, found) + readerui:closeDocument() + readerui:onClose() end) it("should toggle auto refresh time task by toggling the menu", function() @@ -487,6 +515,8 @@ describe("Readerfooter module", function() end end assert.is.same(1, found) + readerui:closeDocument() + readerui:onClose() end) it("should support toggle footer through menu if tap zone is disabled", function() @@ -532,6 +562,8 @@ describe("Readerfooter module", function() assert.is.same(2, footer.mode) DTAP_ZONE_MINIBAR = saved_tap_zone_minibar --luacheck: ignore + readerui:closeDocument() + readerui:onClose() end) it("should remove and add modes to footer text in all_at_once mode", function() @@ -563,6 +595,8 @@ describe("Readerfooter module", function() -- add mode to footer text tapFooterMenu(fake_menu, "Progress percentage".." (⤠)") assert.are.same('1 / 2 | ⤠ 50%', footer.footer_text.text) + readerui:closeDocument() + readerui:onClose() end) it("should initialize text mode in all_at_once mode", function() @@ -587,6 +621,8 @@ describe("Readerfooter module", function() assert.is.truthy(footer.settings.all_at_once) assert.is.truthy(0, footer.mode) assert.is.falsy(readerui.view.footer_visible) + readerui:closeDocument() + readerui:onClose() end) it("should support disabling all the modes", function() @@ -624,6 +660,8 @@ describe("Readerfooter module", function() assert.is.same(true, footer.has_no_mode) tapFooterMenu(fake_menu, "Progress percentage".." (⤠)") assert.is.same(false, footer.has_no_mode) + readerui:closeDocument() + readerui:onClose() end) it("should return correct footer height in time mode", function() @@ -643,6 +681,8 @@ describe("Readerfooter module", function() assert.falsy(footer.has_no_mode) assert.truthy(readerui.view.footer_visible) assert.is.same(15, footer:getHeight()) + readerui:closeDocument() + readerui:onClose() end) it("should return correct footer height when all modes are disabled", function() @@ -662,6 +702,8 @@ describe("Readerfooter module", function() assert.truthy(footer.has_no_mode) assert.truthy(readerui.view.footer_visible) assert.is.same(15, footer:getHeight()) + readerui:closeDocument() + readerui:onClose() end) it("should disable footer when all modes + progressbar are disabled", function() @@ -680,6 +722,8 @@ describe("Readerfooter module", function() assert.truthy(footer.has_no_mode) assert.falsy(readerui.view.footer_visible) + readerui:closeDocument() + readerui:onClose() end) it("should disable footer if settings.disabled is true", function() @@ -698,6 +742,8 @@ describe("Readerfooter module", function() assert.falsy(readerui.view.footer_visible) assert.truthy(footer.onCloseDocument == nil) assert.truthy(footer.mode == 0) + readerui:closeDocument() + readerui:onClose() end) it("should toggle between full and min progress bar for cre documents", function() @@ -723,5 +769,7 @@ describe("Readerfooter module", function() readerui.rolling:onSetStatusLine(0) assert.is.same(0, footer.mode) assert.falsy(readerui.view.footer_visible) + readerui:closeDocument() + readerui:onClose() end) end) diff --git a/spec/unit/readerhighlight_spec.lua b/spec/unit/readerhighlight_spec.lua index 4f4f2a7dd..c621e15cb 100644 --- a/spec/unit/readerhighlight_spec.lua +++ b/spec/unit/readerhighlight_spec.lua @@ -74,6 +74,10 @@ describe("Readerhighlight module", function() document = DocumentRegistry:openDocument(sample_epub), } end) + teardown(function() + readerui:closeDocument() + readerui:onClose() + end) before_each(function() UIManager:quit() readerui.rolling:onGotoPage(page) @@ -117,6 +121,10 @@ describe("Readerhighlight module", function() } readerui:handleEvent(Event:new("SetScrollMode", false)) end) + teardown(function() + readerui:closeDocument() + readerui:onClose() + end) describe("for scanned page with text layer", function() before_each(function() UIManager:quit() @@ -201,6 +209,10 @@ describe("Readerhighlight module", function() } readerui:handleEvent(Event:new("SetScrollMode", true)) end) + teardown(function() + readerui:closeDocument() + readerui:onClose() + end) describe("for scanned page with text layer", function() before_each(function() UIManager:quit() diff --git a/spec/unit/readerlink_spec.lua b/spec/unit/readerlink_spec.lua index 28b36f6c3..98786961d 100644 --- a/spec/unit/readerlink_spec.lua +++ b/spec/unit/readerlink_spec.lua @@ -22,6 +22,8 @@ describe("ReaderLink module", function() readerui.rolling:onGotoPage(5) readerui.link:onTap(nil, {pos = {x = 320, y = 190}}) assert.is.same(37, readerui.rolling.current_page) + readerui:closeDocument() + readerui:onClose() end) it("should jump to links in pdf page mode", function() @@ -36,6 +38,8 @@ describe("ReaderLink module", function() readerui.link:onTap(nil, {pos = {x = 363, y = 565}}) UIManager:run() assert.is.same(22, readerui.paging.current_page) + readerui:closeDocument() + readerui:onClose() end) it("should jump to links in pdf scroll mode", function() @@ -54,6 +58,8 @@ describe("ReaderLink module", function() -- page positions may have unexpected impact on page number assert.truthy(readerui.paging.current_page == 21 or readerui.paging.current_page == 20) + readerui:closeDocument() + readerui:onClose() end) it("should be able to go back after link jump in epub #nocov", function() @@ -66,6 +72,8 @@ describe("ReaderLink module", function() assert.is.same(37, readerui.rolling.current_page) readerui.link:onGoBackLink() assert.is.same(5, readerui.rolling.current_page) + readerui:closeDocument() + readerui:onClose() end) it("should be able to go back after link jump in pdf page mode", function() @@ -82,6 +90,8 @@ describe("ReaderLink module", function() assert.is.same(22, readerui.paging.current_page) readerui.link:onGoBackLink() assert.is.same(1, readerui.paging.current_page) + readerui:closeDocument() + readerui:onClose() end) it("should be able to go back after link jump in pdf scroll mode", function() @@ -100,6 +110,8 @@ describe("ReaderLink module", function() or readerui.paging.current_page == 20) readerui.link:onGoBackLink() assert.is.same(1, readerui.paging.current_page) + readerui:closeDocument() + readerui:onClose() end) it("should be able to go back to the same position after link jump in pdf scroll mode", function() @@ -175,5 +187,7 @@ describe("ReaderLink module", function() readerui.link:onGoBackLink() assert.is.same(3, readerui.paging.current_page) assert.are.same(expected_page_states, readerui.view.page_states) + readerui:closeDocument() + readerui:onClose() end) end) diff --git a/spec/unit/readerpaging_spec.lua b/spec/unit/readerpaging_spec.lua index c49185f9b..6c35694fd 100644 --- a/spec/unit/readerpaging_spec.lua +++ b/spec/unit/readerpaging_spec.lua @@ -20,6 +20,10 @@ describe("Readerpaging module", function() } paging = readerui.paging end) + teardown(function() + readerui:closeDocument() + readerui:onClose() + end) it("should emit EndOfBook event at the end", function() UIManager:quit() @@ -53,6 +57,10 @@ describe("Readerpaging module", function() } paging = readerui.paging end) + teardown(function() + readerui:closeDocument() + readerui:onClose() + end) it("should emit EndOfBook event at the end", function() UIManager:quit() @@ -81,6 +89,8 @@ describe("Readerpaging module", function() document = DocumentRegistry:openDocument(sample_djvu), } tmp_readerui.paging:onScrollPanRel(-100) + tmp_readerui:closeDocument() + tmp_readerui:onClose() end) it("should scroll forward on the last page without crash", function() @@ -94,6 +104,8 @@ describe("Readerpaging module", function() paging:onScrollPanRel(120) paging:onScrollPanRel(-1) paging:onScrollPanRel(120) + tmp_readerui:closeDocument() + tmp_readerui:onClose() end) end) end) diff --git a/spec/unit/readerrolling_spec.lua b/spec/unit/readerrolling_spec.lua index 47ba387c0..daddf056c 100644 --- a/spec/unit/readerrolling_spec.lua +++ b/spec/unit/readerrolling_spec.lua @@ -106,6 +106,8 @@ describe("Readerrolling module", function() txt_rolling:onGotoViewRel(1) assert.is.truthy(called) readerui.onEndOfBook = nil + txt_readerui:closeDocument() + txt_readerui:onClose() end) end) @@ -205,10 +207,14 @@ describe("Readerrolling module", function() end local test_book = "spec/front/unit/data/sample.txt" require("docsettings"):open(test_book):purge() - ReaderUI:new{ + 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/readersearch_spec.lua b/spec/unit/readersearch_spec.lua index aa31fcd58..dedbfbb9a 100644 --- a/spec/unit/readersearch_spec.lua +++ b/spec/unit/readersearch_spec.lua @@ -12,9 +12,9 @@ describe("Readersearch module", function() end) describe("search API for EPUB documents", function() - local doc, search, rolling + local readerui, doc, search, rolling setup(function() - local readerui = ReaderUI:new{ + readerui = ReaderUI:new{ dimen = Screen:getSize(), document = DocumentRegistry:openDocument(sample_epub), } @@ -22,6 +22,10 @@ describe("Readersearch module", function() search = readerui.search rolling = readerui.rolling end) + teardown(function() + readerui:closeDocument() + readerui:onClose() + end) it("should search backward", function() rolling:onGotoPage(10) assert.truthy(search:searchFromCurrent("Verona", 1)) @@ -117,9 +121,9 @@ describe("Readersearch module", function() end) describe("search API for PDF documents", function() - local doc, search, paging + local readerui, doc, search, paging setup(function() - local readerui = ReaderUI:new{ + readerui = ReaderUI:new{ dimen = Screen:getSize(), document = DocumentRegistry:openDocument(sample_pdf), } @@ -127,6 +131,10 @@ describe("Readersearch module", function() search = readerui.search paging = readerui.paging end) + teardown(function() + readerui:closeDocument() + readerui:onClose() + end) it("should match single word with case insensitive option in one page", function() assert.are.equal(9, #doc.koptinterface:findAllMatches(doc, "what", true, 20)) assert.are.equal(51, #doc.koptinterface:findAllMatches(doc, "the", true, 20)) diff --git a/spec/unit/readertoc_spec.lua b/spec/unit/readertoc_spec.lua index e7b0b7614..58f348852 100644 --- a/spec/unit/readertoc_spec.lua +++ b/spec/unit/readertoc_spec.lua @@ -10,6 +10,12 @@ describe("Readertoc module", function() DEBUG = require("dbg") local sample_epub = "spec/front/unit/data/juliet.epub" + -- Clear settings from previous tests + local DocSettings = require("docsettings") + local doc_settings = DocSettings:open(sample_epub) + doc_settings:close() + doc_settings:purge() + readerui = ReaderUI:new{ dimen = Screen:getSize(), document = DocumentRegistry:openDocument(sample_epub), @@ -97,6 +103,10 @@ describe("Readertoc module", function() assert.are.same(12, #toc.collapsed_toc) toc:collapseToc(18) assert.are.same(7, #toc.collapsed_toc) + + --- @note: Delay the teardown 'til the last test, because of course the tests rely on incremental state changes across tests... + readerui:closeDocument() + readerui:onClose() end) end) end) diff --git a/spec/unit/readerui_spec.lua b/spec/unit/readerui_spec.lua index f57a9e77a..289128e98 100644 --- a/spec/unit/readerui_spec.lua +++ b/spec/unit/readerui_spec.lua @@ -36,6 +36,7 @@ describe("Readerui module", function() it("should close document", function() readerui:closeDocument() assert(readerui.document == nil) + readerui:onClose() end) it("should not reset running_instance by mistake", function() ReaderUI:doShowReader(sample_epub) @@ -46,6 +47,7 @@ describe("Readerui module", function() document = DocumentRegistry:openDocument(sample_epub) }:onClose() assert.is.truthy(new_readerui.document) + new_readerui:closeDocument() new_readerui:onClose() end) end) diff --git a/spec/unit/readerview_spec.lua b/spec/unit/readerview_spec.lua index 9495d118e..bc06153a8 100644 --- a/spec/unit/readerview_spec.lua +++ b/spec/unit/readerview_spec.lua @@ -47,6 +47,10 @@ describe("Readerview module", function() error("UIManager's task queue should be emtpy.") end end + + if readerui.document then + readerui:closeDocument() + end end) it("should return and restore view context in page mode", function() @@ -99,6 +103,8 @@ describe("Readerview module", function() assert.is.same(view.visible_area.x, 0) assert.is.same(view.visible_area.y, 10) G_reader_settings:delSetting("reader_footer_mode") + readerui:closeDocument() + readerui:onClose() end) it("should return and restore view context in scroll mode", function() @@ -152,5 +158,7 @@ describe("Readerview module", function() assert.is.same(view.page_states[1].visible_area.x, 0) assert.is.same(view.page_states[1].visible_area.y, 10) G_reader_settings:delSetting("reader_footer_mode") + readerui:closeDocument() + readerui:onClose() end) end) diff --git a/spec/unit/screenshoter_spec.lua b/spec/unit/screenshoter_spec.lua index ce2d94b95..dbe53623f 100644 --- a/spec/unit/screenshoter_spec.lua +++ b/spec/unit/screenshoter_spec.lua @@ -19,6 +19,8 @@ describe("ReaderScreenshot module", function() teardown(function() readerui:handleEvent(Event:new("SetRotationMode", Screen.ORIENTATION_PORTRAIT)) + readerui:closeDocument() + readerui:onClose() end) it("should get screenshot in portrait", function()