diff --git a/frontend/document/credocument.lua b/frontend/document/credocument.lua index 290ec7caa..a8b52029c 100644 --- a/frontend/document/credocument.lua +++ b/frontend/document/credocument.lua @@ -305,12 +305,20 @@ function CreDocument:_readMetadata() end function CreDocument:close() - if self.buffer then - self.buffer:free() - self.buffer = nil - end + -- Let Document do the refcount check, and tell us if we actually need to tear down the instance. + if Document.close(self) then + -- Yup, final Document instance, we can safely destroy internal data. + -- (Document already took care of our self._document userdata). + if self.buffer then + self.buffer:free() + self.buffer = nil + end - Document.close(self) + -- Only exists if the call cache is enabled + if self._callCacheDestroy then + self._callCacheDestroy() + end + end end function CreDocument:updateColorRendering() @@ -1742,12 +1750,6 @@ function CreDocument:setupCallCache() if do_stats then self.close = function(_self) dumpStats() - _self._callCacheDestroy() - CreDocument.close(_self) - end - else - self.close = function(_self) - _self._callCacheDestroy() CreDocument.close(_self) end end diff --git a/frontend/document/document.lua b/frontend/document/document.lua index 5f30b60b1..d4b6c45e4 100644 --- a/frontend/document/document.lua +++ b/frontend/document/document.lua @@ -92,21 +92,29 @@ end -- this might be overridden by a document implementation -- (in which case, do make sure it calls this one, too, to avoid refcounting mismatches in DocumentRegistry!) +-- Returns true if the Document instance needs to be destroyed (no more live refs), +-- false if not (i.e., we've just decreased the refcount, so, leave internal engine data alone). +-- nil if all hell broke loose. function Document:close() local DocumentRegistry = require("document/documentregistry") if self.is_open then - if DocumentRegistry:closeDocument(self.file) == 0 then + local refcount = DocumentRegistry:closeDocument(self.file) + if refcount == 0 then self.is_open = false self._document:close() self._document = nil -- NOTE: DocumentRegistry:openDocument will force a GC sweep the next time we open a Document. -- MµPDF will also do a bit of spring cleaning of its internal cache when opening a *different* document. + return true else - logger.warn("Tried to close a document with *multiple* remaining hot references") + -- This can happen in perfectly sane contexts (i.e., Reader -> History -> View fullsize cover on the *same* book). + logger.dbg("Document: Decreased refcount to", refcount, "for", self.file) + return false end else - logger.warn("Tried to close an already closed document") + logger.warn("Tried to close an already closed document:", self.file) + return nil end end diff --git a/frontend/document/documentregistry.lua b/frontend/document/documentregistry.lua index ef7b1d451..36e4720fa 100644 --- a/frontend/document/documentregistry.lua +++ b/frontend/document/documentregistry.lua @@ -214,6 +214,7 @@ function DocumentRegistry:openDocument(file, provider) end else self.registry[file].refs = self.registry[file].refs + 1 + logger.dbg("DocumentRegistry: Increased refcount to", self.registry[file].refs, "for", file) end if self.registry[file] then return self.registry[file].doc @@ -232,7 +233,16 @@ function DocumentRegistry:closeDocument(file) return self.registry[file].refs end else - error("Try to close unregistered file.") + error("Tried to close an unregistered file.") + end +end + +--- Queries the current refcount for a given file +function DocumentRegistry:getReferenceCount(file) + if self.registry[file] then + return self.registry[file].refs + else + return nil end end diff --git a/frontend/document/pdfdocument.lua b/frontend/document/pdfdocument.lua index 3cd0d4874..4130b321b 100644 --- a/frontend/document/pdfdocument.lua +++ b/frontend/document/pdfdocument.lua @@ -261,9 +261,16 @@ function PdfDocument:writeDocument() end function PdfDocument:close() - if self.is_edited then - self:writeDocument() + -- NOTE: We can't just rely on Document:close's return code for that, as we need self._document + -- in :writeDocument, and it would have been destroyed. + local DocumentRegistry = require("document/documentregistry") + if DocumentRegistry:getReferenceCount(self.file) == 1 then + -- We're the final reference to this Document instance. + if self.is_edited then + self:writeDocument() + end end + Document.close(self) end diff --git a/frontend/document/picdocument.lua b/frontend/document/picdocument.lua index 7a58aa05d..1422a7481 100644 --- a/frontend/document/picdocument.lua +++ b/frontend/document/picdocument.lua @@ -22,6 +22,7 @@ function PicDocument:init() error("Failed to open image:" .. self._document) end + self.is_open = true self.info.has_pages = true self.info.configurable = false diff --git a/plugins/statistics.koplugin/main.lua b/plugins/statistics.koplugin/main.lua index d2f9d208a..7690dad5c 100644 --- a/plugins/statistics.koplugin/main.lua +++ b/plugins/statistics.koplugin/main.lua @@ -122,7 +122,7 @@ local monthTranslation = { } function ReaderStatistics:isDocless() - return self.ui == nil or self.ui.document == nil + return self.ui == nil or self.ui.document == nil or self.ui.document.is_pic == true end -- NOTE: This is used in a migration script by ui/data/onetime_migration, @@ -139,9 +139,10 @@ ReaderStatistics.default_settings = { } function ReaderStatistics:init() - if not self:isDocless() and self.ui.document.is_pic then + if self:isDocless() then return end + self.start_current_period = os.time() self:resetVolatileStats()