From 02f3daebbe966644cc848a4c8d7c0cf78373285d Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 6 Jan 2019 02:11:36 +0100 Subject: [PATCH 1/3] use a contextmanager to ensure correct memoryview release see #4243. --- src/borg/repository.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/borg/repository.py b/src/borg/repository.py index f83e6bedd..e04c0b8cb 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -1395,8 +1395,7 @@ class LoggedIO: with open(backup_filename, 'rb') as backup_fd: # note: file must not be 0 size (windows can't create 0 size mapping) with mmap.mmap(backup_fd.fileno(), 0, access=mmap.ACCESS_READ) as mm: - data = memoryview(mm) - with open(filename, 'wb') as fd: + with memoryview(mm) as data, open(filename, 'wb') as fd: fd.write(MAGIC) while len(data) >= self.header_fmt.size: crc, size, tag = self.header_fmt.unpack(data[:self.header_fmt.size]) @@ -1408,7 +1407,6 @@ class LoggedIO: continue fd.write(data[:size]) data = data[size:] - data.release() def read(self, segment, offset, id, read_data=True): """ From 2910d130553ecf56e988901bac0eddfb8ad87d25 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Tue, 8 Jan 2019 14:39:28 +0100 Subject: [PATCH 2/3] use try/finally to ensure correct memoryview release see #4243. --- src/borg/repository.py | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/borg/repository.py b/src/borg/repository.py index e04c0b8cb..eedeba1ae 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -1395,18 +1395,22 @@ class LoggedIO: with open(backup_filename, 'rb') as backup_fd: # note: file must not be 0 size (windows can't create 0 size mapping) with mmap.mmap(backup_fd.fileno(), 0, access=mmap.ACCESS_READ) as mm: - with memoryview(mm) as data, open(filename, 'wb') as fd: - fd.write(MAGIC) - while len(data) >= self.header_fmt.size: - crc, size, tag = self.header_fmt.unpack(data[:self.header_fmt.size]) - if size < self.header_fmt.size or size > len(data): - data = data[1:] - continue - if crc32(data[4:size]) & 0xffffffff != crc: - data = data[1:] - continue - fd.write(data[:size]) - data = data[size:] + data = memoryview(mm) # didn't use memoryview context manager, it does not work correctly. + try: + with open(filename, 'wb') as fd: + fd.write(MAGIC) + while len(data) >= self.header_fmt.size: + crc, size, tag = self.header_fmt.unpack(data[:self.header_fmt.size]) + if size < self.header_fmt.size or size > len(data): + data = data[1:] + continue + if crc32(data[4:size]) & 0xffffffff != crc: + data = data[1:] + continue + fd.write(data[:size]) + data = data[size:] + finally: + data.release() def read(self, segment, offset, id, read_data=True): """ From 78361744ea15c5f53c8aee9bac7f2950b6f083b1 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Wed, 9 Jan 2019 17:42:33 +0100 Subject: [PATCH 3/3] keep "data" as is, use "d" for slices so that the data.release() call is on the original memoryview and also we can delete the last reference to a slice of it first. --- src/borg/repository.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/borg/repository.py b/src/borg/repository.py index eedeba1ae..914e238e0 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -1395,21 +1395,24 @@ class LoggedIO: with open(backup_filename, 'rb') as backup_fd: # note: file must not be 0 size (windows can't create 0 size mapping) with mmap.mmap(backup_fd.fileno(), 0, access=mmap.ACCESS_READ) as mm: - data = memoryview(mm) # didn't use memoryview context manager, it does not work correctly. + # memoryview context manager is problematic, see https://bugs.python.org/issue35686 + data = memoryview(mm) + d = data try: with open(filename, 'wb') as fd: fd.write(MAGIC) - while len(data) >= self.header_fmt.size: - crc, size, tag = self.header_fmt.unpack(data[:self.header_fmt.size]) - if size < self.header_fmt.size or size > len(data): - data = data[1:] + while len(d) >= self.header_fmt.size: + crc, size, tag = self.header_fmt.unpack(d[:self.header_fmt.size]) + if size < self.header_fmt.size or size > len(d): + d = d[1:] continue - if crc32(data[4:size]) & 0xffffffff != crc: - data = data[1:] + if crc32(d[4:size]) & 0xffffffff != crc: + d = d[1:] continue - fd.write(data[:size]) - data = data[size:] + fd.write(d[:size]) + d = d[size:] finally: + del d data.release() def read(self, segment, offset, id, read_data=True):