From 76e916a07e659c9259aeb95a02052af6f59f5091 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Tue, 4 Mar 2025 16:11:25 +0100 Subject: [PATCH] Make sure to close file stream after backup upload (#5723) * Make sure to close file stream after backup upload Currently the file stream does not get closed before importing the file stream. It seems the test case didn't catch that, presumably because it is a race condition if the bytes get flushed to disk or not. Properly close the stream before continue handling the file. * Close file stream in executor * Add comment about closing twice is fine --- supervisor/api/backups.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/supervisor/api/backups.py b/supervisor/api/backups.py index 715c262b4..382a3e12c 100644 --- a/supervisor/api/backups.py +++ b/supervisor/api/backups.py @@ -531,6 +531,8 @@ class APIBackups(CoreSysAttributes): def close_backup_file() -> None: if backup_file_stream: + # Make sure it got closed, in case of exception. It is safe to + # close the file stream twice. backup_file_stream.close() if temp_dir: temp_dir.cleanup() @@ -541,6 +543,7 @@ class APIBackups(CoreSysAttributes): tar_file = await self.sys_run_in_executor(open_backup_file) while chunk := await contents.read_chunk(size=2**16): await self.sys_run_in_executor(backup_file_stream.write, chunk) + await self.sys_run_in_executor(backup_file_stream.close) backup = await asyncio.shield( self.sys_backups.import_backup( @@ -563,8 +566,7 @@ class APIBackups(CoreSysAttributes): return False finally: - if temp_dir or backup: - await self.sys_run_in_executor(close_backup_file) + await self.sys_run_in_executor(close_backup_file) if backup: return {ATTR_SLUG: backup.slug}