Use atomic writes for the config files

Avoids the need for file locking and also ensures no partial data is
written in case of crash/powerloss. Fixes #1964123 [Settings reset in main calibre program when pc turns off due to power failure](https://bugs.launchpad.net/calibre/+bug/1964123)
This commit is contained in:
Kovid Goyal 2022-03-09 07:41:05 +05:30
parent 15ded1182f
commit 007099dd60
No known key found for this signature in database
GPG Key ID: 06BC317B515ACE7C
2 changed files with 74 additions and 60 deletions

View File

@ -15,12 +15,10 @@ from calibre.constants import (
)
from calibre.utils.config_base import (
Config, ConfigInterface, ConfigProxy, Option, OptionSet, OptionValues,
StringConfig, json_dumps, json_loads, make_config_dir, plugin_dir, prefs,
tweaks, from_json, to_json
StringConfig, commit_data, from_json, json_dumps, json_loads, make_config_dir,
plugin_dir, prefs, read_data, to_json, tweaks
)
from calibre.utils.lock import ExclusiveFile
from polyglot.builtins import string_or_bytes, native_string_type
from polyglot.builtins import native_string_type, string_or_bytes
# optparse uses gettext.gettext instead of _ from builtins, so we
# monkey patch it.
@ -29,7 +27,7 @@ optparse._ = _
if False:
# Make pyflakes happy
Config, ConfigProxy, Option, OptionValues, StringConfig, OptionSet,
ConfigInterface, tweaks, plugin_dir, prefs, from_json, to_json
ConfigInterface, tweaks, plugin_dir, prefs, from_json, to_json, make_config_dir
def check_config_write_access():
@ -53,6 +51,7 @@ class CustomHelpFormatter(optparse.IndentedHelpFormatter):
def format_option(self, option):
import textwrap
from calibre.utils.terminal import colored
result = []
@ -91,6 +90,7 @@ class OptionParser(optparse.OptionParser):
conflict_handler='resolve',
**kwds):
import textwrap
from calibre.utils.terminal import colored
usage = textwrap.dedent(usage)
@ -219,8 +219,8 @@ class DynamicConfig(dict):
self.refresh()
def read_old_serialized_representation(self):
from calibre.utils.shared_file import share_open
from calibre.utils.serialize import pickle_loads
from calibre.utils.shared_file import share_open
path = self.file_path.rpartition('.')[0]
try:
with share_open(path, 'rb') as f:
@ -238,9 +238,12 @@ class DynamicConfig(dict):
migrate = False
if clear_current:
self.clear()
if os.path.exists(self.file_path):
with ExclusiveFile(self.file_path) as f:
raw = f.read()
try:
raw = read_data(self.file_path)
except FileNotFoundError:
d = self.read_old_serialized_representation()
migrate = bool(d)
else:
if raw:
try:
d = json_loads(raw)
@ -250,14 +253,9 @@ class DynamicConfig(dict):
else:
d = self.read_old_serialized_representation()
migrate = bool(d)
else:
d = self.read_old_serialized_representation()
migrate = bool(d)
if migrate and d:
raw = json_dumps(d, ignore_unserializable=True)
with ExclusiveFile(self.file_path) as f:
f.seek(0), f.truncate()
f.write(raw)
commit_data(self.file_path, raw)
self.update(d)
@ -283,13 +281,8 @@ class DynamicConfig(dict):
def commit(self):
if not getattr(self, 'name', None):
return
if not os.path.exists(self.file_path):
make_config_dir()
raw = json_dumps(self)
with ExclusiveFile(self.file_path) as f:
f.seek(0)
f.truncate()
f.write(raw)
commit_data(self.file_path, raw)
dynamic = DynamicConfig()
@ -345,17 +338,19 @@ class XMLConfig(dict):
def refresh(self, clear_current=True):
d = {}
if os.path.exists(self.file_path):
with ExclusiveFile(self.file_path) as f:
raw = f.read()
try:
d = self.raw_to_object(raw) if raw.strip() else {}
except SystemError:
pass
except:
import traceback
traceback.print_exc()
d = {}
try:
raw = read_data(self.file_path)
except FileNotFoundError:
pass
else:
try:
d = self.raw_to_object(raw) if raw.strip() else {}
except SystemError:
pass
except:
import traceback
traceback.print_exc()
d = {}
if clear_current:
self.clear()
self.update(d)
@ -393,15 +388,11 @@ class XMLConfig(dict):
def commit(self):
if self.no_commit:
return
if hasattr(self, 'file_path') and self.file_path:
if getattr(self, 'file_path', None):
dpath = os.path.dirname(self.file_path)
if not os.path.exists(dpath):
os.makedirs(dpath, mode=CONFIG_DIR_MODE)
with ExclusiveFile(self.file_path) as f:
raw = self.to_raw()
f.seek(0)
f.truncate()
f.write(raw)
commit_data(self.file_path, self.to_raw())
def __enter__(self):
self.no_commit = True

View File

@ -5,11 +5,11 @@ __copyright__ = '2011, Kovid Goyal <kovid@kovidgoyal.net>'
__docformat__ = 'restructuredtext en'
import os, re, traceback, numbers
from contextlib import suppress
from functools import partial
from collections import defaultdict
from copy import deepcopy
from calibre.utils.lock import ExclusiveFile
from calibre.constants import config_dir, CONFIG_DIR_MODE, preferred_encoding, filesystem_encoding, iswindows
from polyglot.builtins import iteritems
@ -344,6 +344,34 @@ class ConfigInterface:
self.option_set.smart_update(opts1, opts2)
def read_data(file_path, count=10, sleep_time=0.2):
import time
count = 10
for i in range(count):
try:
with open(file_path, 'rb') as f:
return f.read()
except FileNotFoundError:
raise
except OSError:
if i > count - 2:
raise
# Try the operation repeatedly in case something like a virus
# scanner has opened one of the files (I love windows)
time.sleep(sleep_time)
def commit_data(file_path, data):
import tempfile
from calibre.utils.filenames import atomic_rename
bdir = os.path.dirname(file_path)
os.makedirs(bdir, exist_ok=True, mode=CONFIG_DIR_MODE)
with tempfile.NamedTemporaryFile(dir=bdir, delete=False) as f:
f.write(data)
atomic_rename(f.name, file_path)
class Config(ConfigInterface):
'''
A file based configuration.
@ -361,13 +389,13 @@ class Config(ConfigInterface):
src = ''
migrate = False
path = self.config_file_path
if os.path.exists(path):
with ExclusiveFile(path) as f:
try:
src = f.read().decode('utf-8')
except ValueError:
print("Failed to parse", path)
traceback.print_exc()
with suppress(FileNotFoundError):
src_bytes = read_data(path)
try:
src = src_bytes.decode('utf-8')
except ValueError:
print("Failed to parse", path)
traceback.print_exc()
if not src:
path = path.rpartition('.')[0]
from calibre.utils.shared_file import share_open
@ -381,9 +409,7 @@ class Config(ConfigInterface):
ans = self.option_set.parse_string(src)
if migrate:
new_src = self.option_set.serialize(ans, ignore_unserializable=True)
with ExclusiveFile(self.config_file_path) as f:
f.seek(0), f.truncate()
f.write(new_src)
commit_data(self.config_file_path, new_src)
return ans
def set(self, name, val):
@ -391,16 +417,13 @@ class Config(ConfigInterface):
raise ValueError('The option %s is not defined.'%name)
if not os.path.exists(config_dir):
make_config_dir()
with ExclusiveFile(self.config_file_path) as f:
src = f.read()
opts = self.option_set.parse_string(src)
setattr(opts, name, val)
src = self.option_set.serialize(opts)
f.seek(0)
f.truncate()
if isinstance(src, str):
src = src.encode('utf-8')
f.write(src)
src = read_data(self.config_file_path)
opts = self.option_set.parse_string(src)
setattr(opts, name, val)
src = self.option_set.serialize(opts)
if isinstance(src, str):
src = src.encode('utf-8')
commit_data(self.config_file_path, src)
class StringConfig(ConfigInterface):