diff --git a/docs_src/changes.md b/docs_src/changes.md index 6f9b25c0..e1b73963 100644 --- a/docs_src/changes.md +++ b/docs_src/changes.md @@ -6,6 +6,9 @@ WeeWX change history Fix problem where `[StdReport]` does not include `HTML_ROOT`. Fixes [Issue #1082](https://github.com/weewx/weewx/issues/1082). +Install extensions atomically, never overwrite in place. +[PR #1104](https://github.com/weewx/weewx/pull/1104). Thanks to user John K! + ### 5.4.0 06/16/2026 diff --git a/src/weecfg/extension.py b/src/weecfg/extension.py index 53e1e23d..d0ce89aa 100644 --- a/src/weecfg/extension.py +++ b/src/weecfg/extension.py @@ -282,7 +282,25 @@ class ExtensionEngine: os.makedirs(os.path.dirname(destination_path)) except OSError: pass - shutil.copy(source_path, destination_path) + # Copy to a temporary file, then rename it into place, so + # that the destination is replaced atomically. Overwriting + # it in place would truncate it under a running weewxd, + # which may have the old copy open or memory-mapped (a + # mapped file, such as a .bsp ephemeris, kills the process + # with SIGBUS when truncated underneath it). + tmp_fd, tmp_path = tempfile.mkstemp( + dir=os.path.dirname(destination_path), + prefix=os.path.basename(destination_path) + '.tmp') + os.close(tmp_fd) + try: + shutil.copy(source_path, tmp_path) + os.replace(tmp_path, destination_path) + except BaseException: + try: + os.unlink(tmp_path) + except OSError: + pass + raise N += 1 if self.dry_run: diff --git a/src/weecfg/tests/test_config.py b/src/weecfg/tests/test_config.py index 8582d0c0..6d68312a 100644 --- a/src/weecfg/tests/test_config.py +++ b/src/weecfg/tests/test_config.py @@ -450,6 +450,35 @@ class TestExtensionInstall: # It should be the same as our original: assert test_dict == self.config_dict + @pytest.mark.skipif(sys.platform == 'win32', + reason="st_ino and rename-over-open-file semantics are POSIX") + def test_reinstall_replaces_files_atomically(self): + """Installing over an existing extension must replace each file by + renaming a new file into place, never by overwriting it in place: a + running weewxd may hold the old file open or memory-mapped (a mapped + file, such as a .bsp ephemeris, kills the process with SIGBUS when + truncated underneath it).""" + self.engine.install_extension('./pmon.tgz', no_confirm=True) + installed_path = os.path.join(self.engine.root_dict['USER_DIR'], 'pmon.py') + with open(installed_path, 'rb') as held: + original = held.read() + ino_before = os.fstat(held.fileno()).st_ino + # Install again while the file is held open, as a running + # weewxd would hold it: + self.engine.install_extension('./pmon.tgz', no_confirm=True) + # The held file must be untouched -- same inode, full content... + assert os.fstat(held.fileno()).st_ino == ino_before + held.seek(0) + assert held.read() == original + # ... while the destination path is a new file, swapped in whole: + assert os.stat(installed_path).st_ino != ino_before + with open(installed_path, 'rb') as f: + assert f.read() == original + # No temporary files were left behind: + leftovers = [f for f in os.listdir(os.path.dirname(installed_path)) + if '.tmp' in f] + assert leftovers == [] + # ############# Utilities #################