From 6946dc518129a3ead156dd216e69b2feee04eed2 Mon Sep 17 00:00:00 2001 From: Kovid Goyal Date: Fri, 18 Oct 2013 10:55:20 +0530 Subject: [PATCH] Windows atomic folder move: Handle multiple hardlinks to the same file in a folder --- src/calibre/db/tests/filesystem.py | 19 +++++++++++++++++++ src/calibre/utils/filenames.py | 30 ++++++++++++++++++++++++++++-- 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/calibre/db/tests/filesystem.py b/src/calibre/db/tests/filesystem.py index 132be6961b..5b84d89496 100644 --- a/src/calibre/db/tests/filesystem.py +++ b/src/calibre/db/tests/filesystem.py @@ -81,6 +81,25 @@ class FilesystemTest(BaseTest): f.close() self.assertNotEqual(cache.field_for('title', 1), 'Moved', 'Title was changed despite file lock') + # Test on folder with hardlinks + from calibre.ptempfile import TemporaryDirectory + from calibre.utils.filenames import hardlink_file, WindowsAtomicFolderMove + raw = b'xxx' + with TemporaryDirectory() as tdir1, TemporaryDirectory() as tdir2: + a, b = os.path.join(tdir1, 'a'), os.path.join(tdir1, 'b') + a = os.path.join(tdir1, 'a') + with open(a, 'wb') as f: + f.write(raw) + hardlink_file(a, b) + wam = WindowsAtomicFolderMove(tdir1) + wam.copy_path_to(a, os.path.join(tdir2, 'a')) + wam.copy_path_to(b, os.path.join(tdir2, 'b')) + wam.delete_originals() + self.assertEqual([], os.listdir(tdir1)) + self.assertEqual({'a', 'b'}, set(os.listdir(tdir2))) + self.assertEqual(raw, open(os.path.join(tdir2, 'a'), 'rb').read()) + self.assertEqual(raw, open(os.path.join(tdir2, 'b'), 'rb').read()) + def test_library_move(self): ' Test moving of library ' from calibre.ptempfile import TemporaryDirectory diff --git a/src/calibre/utils/filenames.py b/src/calibre/utils/filenames.py index a4aee1a9ec..e998a11c7f 100644 --- a/src/calibre/utils/filenames.py +++ b/src/calibre/utils/filenames.py @@ -318,6 +318,7 @@ class WindowsAtomicFolderMove(object): import win32file, winerror from pywintypes import error + from collections import defaultdict if isbytestring(path): path = path.decode(filesystem_encoding) @@ -325,7 +326,13 @@ class WindowsAtomicFolderMove(object): if not os.path.exists(path): return - for x in os.listdir(path): + names = os.listdir(path) + name_to_fileid = {x:windows_get_fileid(os.path.join(path, x)) for x in names} + fileid_to_names = defaultdict(set) + for name, fileid in name_to_fileid.iteritems(): + fileid_to_names[fileid].add(name) + + for x in names: f = os.path.normcase(os.path.abspath(os.path.join(path, x))) if not os.path.isfile(f): continue @@ -340,6 +347,20 @@ class WindowsAtomicFolderMove(object): win32file.FILE_SHARE_DELETE, None, win32file.OPEN_EXISTING, win32file.FILE_FLAG_SEQUENTIAL_SCAN, 0) except error as e: + if getattr(e, 'winerror', 0) == winerror.ERROR_SHARING_VIOLATION: + # The file could be a hardlink to an already opened file, + # in which case we use the same handle for both files + fileid = name_to_fileid[x] + found = False + for other in fileid_to_names[fileid]: + other = os.path.normcase(os.path.abspath(os.path.join(path, other))) + if other in self.handle_map: + self.handle_map[f] = self.handle_map[other] + found = True + break + if found: + continue + self.close_handles() if getattr(e, 'winerror', 0) == winerror.ERROR_SHARING_VIOLATION: err = IOError(errno.EACCES, @@ -370,6 +391,8 @@ class WindowsAtomicFolderMove(object): return except: pass + + win32file.SetFilePointer(handle, 0, win32file.FILE_BEGIN) with lopen(dest, 'wb') as f: while True: hr, raw = win32file.ReadFile(handle, 1024*1024) @@ -380,6 +403,7 @@ class WindowsAtomicFolderMove(object): f.write(raw) def release_file(self, path): + ' Release the lock on the file pointed to by path. Will also release the lock on any hardlinks to path ' key = None for p, h in self.handle_map.iteritems(): if samefile_windows(path, p): @@ -388,7 +412,9 @@ class WindowsAtomicFolderMove(object): if key is not None: import win32file win32file.CloseHandle(key[1]) - self.handle_map.pop(key[0]) + remove = [f for f, h in self.handle_map.iteritems() if h is key[1]] + for x in remove: + self.handle_map.pop(x) def close_handles(self): import win32file