Safer config saving#6987
Merged
Merged
Conversation
…refs issue #3165) Instead of directly writing the file, a temporary file is created and moved to the final location. This prevents corrupt configuration files being written. Added a fileUtils module with just a FaultTolerantFile context manager for now to facilitate this pattern. Please update miscDependencies submodule to a revision that includes the osmove module.
So that no miscDeps update is required, use moveFileEx through ctypes.
jcsteh
approved these changes
Mar 17, 2017
| @@ -0,0 +1,24 @@ | |||
| #fileUtils.py | |||
| #A part of NonVisual Desktop Access (NVDA) | |||
| #Copyright (C) 2006-2016 NV Access Limited | |||
Contributor
There was a problem hiding this comment.
This should instead be:
+#Copyright (C) 2017 NV Access Limited, Bram Duvigneau
| from logHandler import log | ||
|
|
||
| @contextmanager | ||
| def FaultTolerantFile(name): |
Contributor
There was a problem hiding this comment.
Please add a docstring with details on what this is/does and maybe example usage. The Python term for one of these with things is "context manager".
| f.flush() | ||
| os.fsync(f) | ||
| f.close() | ||
| MOVEFILE_REPLACE_EXISTING = 1 |
Contributor
There was a problem hiding this comment.
Please move this constant outside the function.
|
I could have used this yesterday, when a crash during shut down of windows
resulted in nvda seeming to start like a new install!
Brian
bglists@blueyonder.co.uk
Sent via blueyonder.
Please address personal email to:-
briang1@blueyonder.co.uk, putting 'Brian Gaff'
in the display name field.
----- Original Message -----
From: "Reef Turner" <notifications@github.com>
To: "nvaccess/nvda" <nvda@noreply.github.com>
Cc: "Subscribed" <subscribed@noreply.github.com>
…Sent: Friday, March 17, 2017 5:04 AM
Subject: [nvaccess/nvda] Safer config saving (#6987)
Fixes #3165
This supersedes #5916. Rather than use
[osreplace ](https://pypi.python.org/pypi/pyosreplace/0.1) use
`MoveFileExW` called with `ctypes`
Generally the approach is to lessen the chance that ALL config is lost if
NVDA is interrupted during the saving process. This is done by first
writing to a temporary file and then using this to replace the target
config file.
There is a chance that some recently changed configuration will not be
saved, if the saving process is interrupted while the temp file is being
written. However this will no longer result in ALL config being lost.
You can view, comment on, or merge this pull request online at:
#6987
-- Commit Summary --
* Save configuration profiles and gesture maps as an atomic operation
(refs issue #3165)
* fileUtils: add copyright/license header
* Just use os.rename for the renaming of profiles, osmove.move is not
needed here
* use ctypes to call moveFileEx instead of osreplace
-- File Changes --
M source/config/__init__.py (7)
A source/fileUtils.py (24)
M source/inputCore.py (4)
-- Patch Links --
https://github.com/nvaccess/nvda/pull/6987.patch
https://github.com/nvaccess/nvda/pull/6987.diff
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
#6987
|
feerrenrut
added a commit
that referenced
this pull request
Mar 17, 2017
feerrenrut
added a commit
that referenced
this pull request
Apr 4, 2017
- Deprecated code removed: - PR #6878: `speech.REASON_*` constants, `controlTypes.REASON_*` should be used instead. (issue #6846) - PR #6877: `i18nName` for synth settings, `displayName` and `displayNameWithAccelerator` should be used instead. (issues #6846, #5185) - PR #6876: `config.validateConfig`. (issues #6846, #667) - PR #6875: `config.save`. (issue #6846) - PR #6914: Support Windows Calculator on Windows 10 Enterprise LTSB (Long-Term Servicing Branch) and Server. (issue #6914) - PR #6987: Reduced the chance of configuration file being corrupted when Windows shuts down. Configuration files are now written to a temporary file before replacing the actual configuration file. (issue #3165)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3165
This supersedes #5916. Rather than use osreplace use
MoveFileExWcalled withctypesGenerally the approach is to lessen the chance that ALL config is lost if NVDA is interrupted during the saving process. This is done by first writing to a temporary file and then using this to replace the target config file.
There is a chance that some recently changed configuration will not be saved, if the saving process is interrupted while the temp file is being written. However this will no longer result in ALL config being lost.