From 04d9a557aabbb49cd70bcc7de1579d7e9b56c145 Mon Sep 17 00:00:00 2001 From: poire-z Date: Tue, 10 Dec 2019 23:00:08 +0100 Subject: [PATCH] Use fsync() for more robust setting files saving Bump base for util.fsyncOpenedFile() and util.fsyncDirectory(). Use these to force flush to storage device when saving luasetting, docsetting, and history.lua. Also dont rotate them to .old until they are at least 60 seconds old. Also make auto_save_paging_count count right. Also bump crengine: open (as text) small or empty files --- base | 2 +- frontend/apps/reader/modules/readerview.lua | 3 +- frontend/docsettings.lua | 23 ++++++++++++--- frontend/luadata.lua | 32 ++++++++++++--------- frontend/luasettings.lua | 18 +++++++++++- frontend/readhistory.lua | 6 ++-- spec/unit/docsettings_spec.lua | 12 ++++++++ 7 files changed, 73 insertions(+), 23 deletions(-) diff --git a/base b/base index 10cfb8379..f94e953b3 160000 --- a/base +++ b/base @@ -1 +1 @@ -Subproject commit 10cfb8379d6badfaa4a9599d6a659cc1983a0535 +Subproject commit f94e953b32858a02aa2d0a6b780ee28b88717ced diff --git a/frontend/apps/reader/modules/readerview.lua b/frontend/apps/reader/modules/readerview.lua index d3ad1e194..32b38aca1 100644 --- a/frontend/apps/reader/modules/readerview.lua +++ b/frontend/apps/reader/modules/readerview.lua @@ -928,11 +928,10 @@ function ReaderView:onReaderReady() end else self.autoSaveSettings = function() + self.auto_save_paging_count = self.auto_save_paging_count + 1 if self.auto_save_paging_count == DAUTO_SAVE_PAGING_COUNT then self.ui:saveSettings() self.auto_save_paging_count = 0 - else - self.auto_save_paging_count = self.auto_save_paging_count + 1 end end end diff --git a/frontend/docsettings.lua b/frontend/docsettings.lua index 43eacabe1..136427ac9 100644 --- a/frontend/docsettings.lua +++ b/frontend/docsettings.lua @@ -6,9 +6,9 @@ in the so-called sidecar directory local DataStorage = require("datastorage") local dump = require("dump") +local ffiutil = require("ffi/util") local lfs = require("libs/libkoreader-lfs") local logger = require("logger") -local purgeDir = require("ffi/util").purgeDir local DocSettings = {} @@ -187,9 +187,19 @@ function DocSettings:flush() local s_out = dump(self.data) os.setlocale('C', 'numeric') for _, f in pairs(serials) do + local directory_updated = false if lfs.attributes(f, "mode") == "file" then - logger.dbg("Rename ", f, " to ", f .. ".old") - os.rename(f, f .. ".old") + -- As an additional safety measure (to the ffiutil.fsync* calls + -- used below), we only backup the file to .old when it has + -- not been modified in the last 60 seconds. This should ensure + -- in the case the fsync calls are not supported that the OS + -- may have itself sync'ed that file content in the meantime. + local mtime = lfs.attributes(f, "modification") + if mtime < os.time() - 60 then + logger.dbg("Rename ", f, " to ", f .. ".old") + os.rename(f, f .. ".old") + directory_updated = true -- fsync directory content too below + end end logger.dbg("Write to ", f) local f_out = io.open(f, "w") @@ -197,6 +207,7 @@ function DocSettings:flush() f_out:write("-- we can read Lua syntax here!\nreturn ") f_out:write(s_out) f_out:write("\n") + ffiutil.fsyncOpenedFile(f_out) -- force flush to the storage device f_out:close() if self.candidates ~= nil @@ -212,6 +223,10 @@ function DocSettings:flush() end end + if directory_updated then + -- Ensure the file renaming is flushed to storage device + ffiutil.fsyncDirectory(f) + end break end end @@ -227,7 +242,7 @@ function DocSettings:purge() os.remove(self.history_file) end if lfs.attributes(self.sidecar, "mode") == "directory" then - purgeDir(self.sidecar) + ffiutil.purgeDir(self.sidecar) end self.data = {} end diff --git a/frontend/luadata.lua b/frontend/luadata.lua index 89dc72846..d58db173f 100644 --- a/frontend/luadata.lua +++ b/frontend/luadata.lua @@ -51,21 +51,27 @@ function LuaData:open(file_path, o) -- luacheck: ignore 312 end end - local ok = pcall(dofile, new.file) - - if ok then - logger.dbg("data is read from ", new.file) - else - logger.dbg(new.file, " is invalid, remove.") - os.remove(new.file) + local ok = false + if lfs.attributes(new.file, "mode") == "file" then + ok = pcall(dofile, new.file) + if ok then + logger.dbg("data is read from ", new.file) + else + logger.dbg(new.file, " is invalid, remove.") + os.remove(new.file) + end + end + if not ok then for i=1, self.max_backups, 1 do local backup_file = new.file..".old."..i - if pcall(dofile, backup_file) then - logger.dbg("data is read from ", backup_file) - break - else - logger.dbg(backup_file, " is invalid, remove.") - os.remove(backup_file) + if lfs.attributes(backup_file, "mode") == "file" then + if pcall(dofile, backup_file) then + logger.dbg("data is read from ", backup_file) + break + else + logger.dbg(backup_file, " is invalid, remove.") + os.remove(backup_file) + end end end end diff --git a/frontend/luasettings.lua b/frontend/luasettings.lua index afa355cb9..de5862f06 100644 --- a/frontend/luasettings.lua +++ b/frontend/luasettings.lua @@ -3,6 +3,7 @@ This module handles generic settings as well as KOReader's global settings syste ]] local dump = require("dump") +local ffiutil = require("ffi/util") local lfs = require("libs/libkoreader-lfs") local logger = require("logger") @@ -180,8 +181,18 @@ end --- Writes settings to disk. function LuaSettings:flush() if not self.file then return end + local directory_updated = false if lfs.attributes(self.file, "mode") == "file" then - os.rename(self.file, self.file .. ".old") + -- As an additional safety measure (to the ffiutil.fsync* calls + -- used below), we only backup the file to .old when it has + -- not been modified in the last 60 seconds. This should ensure + -- in the case the fsync calls are not supported that the OS + -- may have itself sync'ed that file content in the meantime. + local mtime = lfs.attributes(self.file, "modification") + if mtime < os.time() - 60 then + os.rename(self.file, self.file .. ".old") + directory_updated = true -- fsync directory content too below + end end local f_out = io.open(self.file, "w") if f_out ~= nil then @@ -189,8 +200,13 @@ function LuaSettings:flush() f_out:write("-- we can read Lua syntax here!\nreturn ") f_out:write(dump(self.data)) f_out:write("\n") + ffiutil.fsyncOpenedFile(f_out) -- force flush to the storage device f_out:close() end + if directory_updated then + -- Ensure the file renaming is flushed to storage device + ffiutil.fsyncDirectory(self.file) + end return self end diff --git a/frontend/readhistory.lua b/frontend/readhistory.lua index 33b4611ce..856d84836 100644 --- a/frontend/readhistory.lua +++ b/frontend/readhistory.lua @@ -1,10 +1,11 @@ local DataStorage = require("datastorage") local DocSettings = require("docsettings") local dump = require("dump") +local ffiutil = require("ffi/util") local getFriendlySize = require("util").getFriendlySize -local joinPath = require("ffi/util").joinPath +local joinPath = ffiutil.joinPath local lfs = require("libs/libkoreader-lfs") -local realpath = require("ffi/util").realpath +local realpath = ffiutil.realpath local history_file = joinPath(DataStorage:getDataDir(), "history.lua") @@ -91,6 +92,7 @@ function ReadHistory:_flush() end local f = io.open(history_file, "w") f:write("return " .. dump(content) .. "\n") + ffiutil.fsyncOpenedFile(f) -- force flush to the storage device f:close() end diff --git a/spec/unit/docsettings_spec.lua b/spec/unit/docsettings_spec.lua index 732be2229..58df901e5 100644 --- a/spec/unit/docsettings_spec.lua +++ b/spec/unit/docsettings_spec.lua @@ -100,6 +100,12 @@ describe("docsettings module", function() d:flush() -- metadata.pdf.lua should be generated. assert.Equals("file", lfs.attributes(d.sidecar_file, "mode")) + d:flush() + -- metadata.pdf.lua.old should not yet be generated. + assert.are.not_equal("file", lfs.attributes(d.sidecar_file .. ".old", "mode")) + -- make metadata.pdf.lua older to bypass 60s age needed for .old rotation + local minutes_ago = os.time() - 120 + lfs.touch(d.sidecar_file, minutes_ago) d:close() -- metadata.pdf.lua and metadata.pdf.lua.old should be generated. assert.Equals("file", lfs.attributes(d.sidecar_file, "mode")) @@ -152,6 +158,9 @@ describe("docsettings module", function() d:flush() -- metadata.pdf.lua should be generated. assert.Equals("file", lfs.attributes(d.sidecar_file, "mode")) + -- make metadata.pdf.lua older to bypass 60s age needed for .old rotation + local minutes_ago = os.time() - 120 + lfs.touch(d.sidecar_file, minutes_ago) d:close() -- metadata.pdf.lua and metadata.pdf.lua.old should be generated. assert.Equals("file", lfs.attributes(d.sidecar_file, "mode")) @@ -182,6 +191,9 @@ describe("docsettings module", function() d:flush() -- metadata.pdf.lua should be generated. assert.Equals("file", lfs.attributes(d.sidecar_file, "mode")) + -- make metadata.pdf.lua older to bypass 60s age needed for .old rotation + local minutes_ago = os.time() - 120 + lfs.touch(d.sidecar_file, minutes_ago) d:close() -- metadata.pdf.lua and metadata.pdf.lua.old should be generated. assert.Equals("file", lfs.attributes(d.sidecar_file, "mode"))