diff --git a/src/calibre/utils/copy_files.py b/src/calibre/utils/copy_files.py index 62282b8939..2025aeebcd 100644 --- a/src/calibre/utils/copy_files.py +++ b/src/calibre/utils/copy_files.py @@ -90,11 +90,12 @@ class WindowsFileCopier: def _open_file(self, path: str, retry_on_sharing_violation: bool = True, is_folder: bool = False) -> 'winutil.Handle': flags = winutil.FILE_FLAG_BACKUP_SEMANTICS if is_folder else winutil.FILE_FLAG_SEQUENTIAL_SCAN + access_flags = winutil.GENERIC_READ if self.delete_all: - flags |= winutil.FILE_FLAG_DELETE_ON_CLOSE + access_flags |= winutil.DELETE + share_flags = winutil.FILE_SHARE_DELETE if self.allow_move else 0 try: - return winutil.create_file(make_long_path_useable(path), winutil.GENERIC_READ, - winutil.FILE_SHARE_DELETE if self.allow_move else 0, winutil.OPEN_EXISTING, flags) + return winutil.create_file(make_long_path_useable(path), access_flags, share_flags, winutil.OPEN_EXISTING, flags) except OSError as e: if e.winerror == winutil.ERROR_SHARING_VIOLATION: # The file could be a hardlink to an already opened file, @@ -106,13 +107,12 @@ class WindowsFileCopier: if retry_on_sharing_violation: time.sleep(WINDOWS_SLEEP_FOR_RETRY_TIME) return self._open_file(path, False, is_folder) - err = IOError(errno.EACCES, - _('File is open in another program')) + err = IOError(errno.EACCES, _('File {} is open in another program').format(path)) err.filename = path raise err from e raise - def __enter__(self) -> None: + def open_all_handles(self) -> None: for path, file_id in self.path_to_fileid_map.items(): self.fileid_to_paths_map[file_id].add(path) for src in self.copy_map: @@ -120,11 +120,36 @@ class WindowsFileCopier: for path in self.folders: self.folder_to_handle_map[path] = self._open_file(path, is_folder=True) + def __enter__(self) -> None: + try: + self.open_all_handles() + except OSError: + self.close_all_handles() + raise + + def close_all_handles(self, delete_on_close: bool = False) -> None: + while self.path_to_handle_map: + path, h = next(iter(self.path_to_handle_map.items())) + if delete_on_close: + winutil.set_file_handle_delete_on_close(h, True) + h.close() + self.path_to_handle_map.pop(path) + while self.folder_to_handle_map: + path, h = next(reversed(self.folder_to_handle_map.items())) + if delete_on_close: + try: + winutil.set_file_handle_delete_on_close(h, True) + except OSError as err: + # Ignore dir not empty errors. Should never happen but we + # ignore it as the UNIX semantics are to no delete folders + # during __exit__ anyway and we dont want to leak the handle. + if err.winerror != winutil.ERROR_DIR_NOT_EMPTY: + raise + h.close() + self.folder_to_handle_map.pop(path) + def __exit__(self, exc_type, exc_val, exc_tb) -> None: - for h in self.path_to_handle_map.values(): - h.close() - for h in reversed(self.folder_to_handle_map.values()): - h.close() + self.close_all_handles(delete_on_close=self.delete_all and exc_val is None) def copy_all(self) -> None: for src_path, dest_path in self.copy_map.items(): diff --git a/src/calibre/utils/copy_files_test.py b/src/calibre/utils/copy_files_test.py index a73a103cb4..7b201eaca5 100644 --- a/src/calibre/utils/copy_files_test.py +++ b/src/calibre/utils/copy_files_test.py @@ -95,16 +95,27 @@ class TestCopyFiles(unittest.TestCase): self.reset() src, dest = self.s(), self.d() if iswindows: - with open(self.s('sub/a')) as locked: + os.mkdir(self.s('lockdir')) + open(self.s('lockdir/lockfile'), 'w').close() + before = frozenset(walk(src)) + with open(self.s('lockdir/lockfile')) as locked: locked - self.assertRaises(IOError, copy_tree, src, dest) - self.ae(os.listdir(self.d()), ['sub']) + self.assertRaises(IOError, copy_tree, src, dest, delete_source=True) + self.ae(set(os.listdir(self.d())), {'sub', 'lockdir'}) self.assertFalse(tuple(walk(self.d()))) - h = winutil.create_file(self.s('sub'), winutil.GENERIC_READ, 0, winutil.OPEN_EXISTING, winutil.FILE_FLAG_BACKUP_SEMANTICS) + self.ae(before, frozenset(walk(src)), 'Source files were deleted despite there being an error') + + shutil.rmtree(dest) + os.mkdir(dest) + h = winutil.create_file( + self.s('lockdir'), winutil.GENERIC_READ|winutil.GENERIC_WRITE|winutil.DELETE, + winutil.FILE_SHARE_READ|winutil.FILE_SHARE_WRITE|winutil.FILE_SHARE_DELETE, winutil.OPEN_EXISTING, + winutil.FILE_FLAG_BACKUP_SEMANTICS) with closing(h): - self.assertRaises(IOError, copy_tree, src, dest) - self.ae(os.listdir(self.d()), ['sub']) + self.assertRaises(IOError, copy_tree, src, dest, delete_source=True) + self.ae(set(os.listdir(self.d())), {'sub', 'lockdir'}) self.assertFalse(tuple(walk(self.d()))) + self.ae(before, frozenset(walk(src)), 'Source files were deleted despite there being an error') def find_tests(): return unittest.defaultTestLoader.loadTestsFromTestCase(TestCopyFiles) diff --git a/src/calibre/utils/windows/winutil.cpp b/src/calibre/utils/windows/winutil.cpp index 6f7a5dab4f..7295a2886c 100644 --- a/src/calibre/utils/windows/winutil.cpp +++ b/src/calibre/utils/windows/winutil.cpp @@ -490,6 +490,17 @@ winutil_set_file_pointer(PyObject *self, PyObject *args) { return PyLong_FromLongLong(ans.QuadPart); } +static PyObject* +winutil_set_file_handle_delete_on_close(PyObject *self, PyObject *args) { + HANDLE handle; + int delete_on_close; + if (!PyArg_ParseTuple(args, "O&p", convert_handle, &handle, &delete_on_close)) return NULL; + FILE_DISPOSITION_INFO di; + di.DeleteFile = delete_on_close; + if (!SetFileInformationByHandle(handle, FileDispositionInfo, &di, sizeof(FILE_DISPOSITION_INFO))) return set_error_from_handle(args); + Py_RETURN_NONE; +} + static PyObject* winutil_create_file(PyObject *self, PyObject *args) { wchar_raii path; @@ -1255,6 +1266,10 @@ static PyMethodDef winutil_methods[] = { "set_file_pointer(handle, pos, method=FILE_BEGIN)\n\nWrapper for SetFilePointer" }, + {"set_file_handle_delete_on_close", (PyCFunction)winutil_set_file_handle_delete_on_close, METH_VARARGS, + "set_file_handle_delete_on_close(handle, delete_on_close)\n\nSet the delete on close flag on the specified handle. Note only works if CreateFile() is called with DELETE generic access, and without FILE_FLAG_DELETE_ON_CLOSE." + }, + {"read_file", (PyCFunction)winutil_read_file, METH_VARARGS, "read_file(handle, chunk_size=16KB)\n\nWrapper for ReadFile" }, @@ -1467,6 +1482,7 @@ exec_module(PyObject *m) { A(ERROR_ALREADY_EXISTS); A(ERROR_BROKEN_PIPE); A(ERROR_PIPE_BUSY); + A(ERROR_DIR_NOT_EMPTY); A(NormalHandle); A(ModuleHandle); A(IconHandle);