From 2375cb78ff540d1cf0b294836c33379f1888ea1c Mon Sep 17 00:00:00 2001 From: The one with the braid Date: Mon, 24 Apr 2023 15:49:20 +0200 Subject: [PATCH 1/3] chore: use existing open file in Frame Helper - use existing read Random Access File in FrameIOHelper - avoid conflicts opening the hive file on some filesystems Signed-off-by: The one with the braid --- .../src/backend/vm/storage_backend_vm.dart | 47 ++++++++++++------- hive/lib/src/io/frame_io_helper.dart | 41 ++++++++++------ .../memory/random_access_buffer_test.dart | 1 - .../backend/vm/storage_backend_vm_test.dart | 3 +- hive/test/tests/io/frame_io_helper_test.dart | 18 +++---- 5 files changed, 65 insertions(+), 45 deletions(-) diff --git a/hive/lib/src/backend/vm/storage_backend_vm.dart b/hive/lib/src/backend/vm/storage_backend_vm.dart index db09c5270..591cee058 100644 --- a/hive/lib/src/backend/vm/storage_backend_vm.dart +++ b/hive/lib/src/backend/vm/storage_backend_vm.dart @@ -1,4 +1,5 @@ import 'dart:async'; +import 'dart:developer'; import 'dart:io'; import 'package:hive/hive.dart'; @@ -38,7 +39,7 @@ class StorageBackendVm extends StorageBackend { /// Not part of public API @visibleForTesting - late RandomAccessFile lockRaf; + RandomAccessFile? lockRaf; /// Not part of public API @visibleForTesting @@ -78,28 +79,38 @@ class StorageBackendVm extends StorageBackend { TypeRegistry registry, Keystore keystore, bool lazy) async { this.registry = registry; - lockRaf = await _lockFile.open(mode: FileMode.write); if ((Hive as HiveImpl).useLocks) { + final lockRaf = this.lockRaf = await _lockFile.open(mode: FileMode.write); await lockRaf.lock(); } - int recoveryOffset; - if (!lazy) { - recoveryOffset = - await _frameHelper.framesFromFile(path, keystore, registry, _cipher); - } else { - recoveryOffset = await _frameHelper.keysFromFile(path, keystore, _cipher); - } - - if (recoveryOffset != -1) { - if (_crashRecovery) { - print('Recovering corrupted box.'); - await writeRaf.truncate(recoveryOffset); - await writeRaf.setPosition(recoveryOffset); - writeOffset = recoveryOffset; + try { + int recoveryOffset; + if (!lazy) { + recoveryOffset = await _frameHelper.framesFromFile( + readRaf, keystore, registry, _cipher); } else { - throw HiveError('Wrong checksum in hive file. Box may be corrupted.'); + recoveryOffset = + await _frameHelper.keysFromFile(readRaf, keystore, _cipher); + } + + if (recoveryOffset != -1) { + if (_crashRecovery) { + print('Recovering corrupted box.'); + await writeRaf.truncate(recoveryOffset); + await writeRaf.setPosition(recoveryOffset); + writeOffset = recoveryOffset; + } else { + throw HiveError('Wrong checksum in hive file. Box may be corrupted.'); + } } + } catch (e, s) { + log( + 'Error checking hive recovery offset', + error: e, + stackTrace: s, + name: 'hive', + ); } } @@ -217,7 +228,7 @@ class StorageBackendVm extends StorageBackend { await readRaf.close(); await writeRaf.close(); - await lockRaf.close(); + await lockRaf?.close(); await _lockFile.delete(); } diff --git a/hive/lib/src/io/frame_io_helper.dart b/hive/lib/src/io/frame_io_helper.dart index d7cb14760..b501f5fe2 100644 --- a/hive/lib/src/io/frame_io_helper.dart +++ b/hive/lib/src/io/frame_io_helper.dart @@ -1,3 +1,4 @@ +import 'dart:async'; import 'dart:io'; import 'dart:typed_data'; @@ -12,34 +13,44 @@ import 'package:meta/meta.dart'; /// Not part of public API class FrameIoHelper extends FrameHelper { /// Not part of public API + /// + /// helps allowing to override this method in tests @visibleForTesting - Future openFile(String path) { - return File(path).open(); + FutureOr overridableRAF(RandomAccessFile raf) { + return raf; } /// Not part of public API + /// + /// helps allowing to override this method in tests @visibleForTesting - Future> readFile(String path) { - return File(path).readAsBytes(); + Future overridableReadFile(RandomAccessFile raf) async { + final previousPosition = await raf.position(); // likely 0 in any case + + final length = await raf.length(); + final bytes = await raf.read(length); + + await raf.setPosition(previousPosition); + + return bytes; } /// Not part of public API Future keysFromFile( - String path, Keystore keystore, HiveCipher? cipher) async { - var raf = await openFile(path); - var fileReader = BufferedFileReader(raf); - try { - return await _KeyReader(fileReader).readKeys(keystore, cipher); - } finally { - await raf.close(); - } + RandomAccessFile raf, Keystore keystore, HiveCipher? cipher) async { + // only used for testing override + final file = await overridableRAF(raf); + final fileReader = BufferedFileReader(file); + + return await _KeyReader(fileReader).readKeys(keystore, cipher); } /// Not part of public API - Future framesFromFile(String path, Keystore keystore, + Future framesFromFile(RandomAccessFile raf, Keystore keystore, TypeRegistry registry, HiveCipher? cipher) async { - var bytes = await readFile(path); - return framesFromBytes(bytes as Uint8List, keystore, registry, cipher); + // only used for testing override + final bytes = await overridableReadFile(raf); + return framesFromBytes(bytes, keystore, registry, cipher); } } diff --git a/hive/test/tests/backend/memory/random_access_buffer_test.dart b/hive/test/tests/backend/memory/random_access_buffer_test.dart index 32591190a..cb4d9b8c5 100644 --- a/hive/test/tests/backend/memory/random_access_buffer_test.dart +++ b/hive/test/tests/backend/memory/random_access_buffer_test.dart @@ -4,7 +4,6 @@ import 'package:hive/src/binary/frame.dart'; import 'package:hive/src/registry/type_registry_impl.dart'; import 'package:test/test.dart'; - void main() { group('RandomAccessBuffer', () { test('empty random access buffer', () { diff --git a/hive/test/tests/backend/vm/storage_backend_vm_test.dart b/hive/test/tests/backend/vm/storage_backend_vm_test.dart index 3f8ad6de5..9b00f3a30 100644 --- a/hive/test/tests/backend/vm/storage_backend_vm_test.dart +++ b/hive/test/tests/backend/vm/storage_backend_vm_test.dart @@ -252,8 +252,7 @@ void main() { .thenAnswer((i) => Future.value(writeRaf)); when(() => writeRaf.writeFrom(bytes)) .thenAnswer((i) => Future.value(writeRaf)); - when(() => writeRaf.flush()) - .thenAnswer((i) => Future.value(writeRaf)); + when(() => writeRaf.flush()).thenAnswer((i) => Future.value(writeRaf)); var backend = _getBackend(writeRaf: writeRaf) // The registry needs to be initialized before writing values, and diff --git a/hive/test/tests/io/frame_io_helper_test.dart b/hive/test/tests/io/frame_io_helper_test.dart index 1aae25884..039df6901 100644 --- a/hive/test/tests/io/frame_io_helper_test.dart +++ b/hive/test/tests/io/frame_io_helper_test.dart @@ -23,12 +23,12 @@ class _FrameIoHelperTest extends FrameIoHelper { _FrameIoHelperTest(this.bytes); @override - Future openFile(String path) async { + Future overridableRAF(RandomAccessFile raf) async { return getTempRaf(bytes); } @override - Future readFile(String path) async { + Future overridableReadFile(RandomAccessFile raf) async { return bytes; } } @@ -39,8 +39,8 @@ void main() { test('frame', () async { var keystore = Keystore.debug(); var ioHelper = _FrameIoHelperTest(_getBytes(frameBytes)); - var recoveryOffset = - await ioHelper.keysFromFile('null', keystore, null); + var recoveryOffset = await ioHelper.keysFromFile( + await getTempRaf(const []), keystore, null); expect(recoveryOffset, -1); var testKeystore = Keystore.debug( @@ -53,8 +53,8 @@ void main() { test('encrypted', () async { var keystore = Keystore.debug(); var ioHelper = _FrameIoHelperTest(_getBytes(frameBytesEncrypted)); - var recoveryOffset = - await ioHelper.keysFromFile('null', keystore, testCipher); + var recoveryOffset = await ioHelper.keysFromFile( + await getTempRaf(const []), keystore, testCipher); expect(recoveryOffset, -1); var testKeystore = Keystore.debug( @@ -73,8 +73,8 @@ void main() { test('frame', () async { var keystore = Keystore.debug(); var ioHelper = _FrameIoHelperTest(_getBytes(frameBytes)); - var recoveryOffset = - await ioHelper.framesFromFile('null', keystore, testRegistry, null); + var recoveryOffset = await ioHelper.framesFromFile( + await getTempRaf(const []), keystore, testRegistry, null); expect(recoveryOffset, -1); var testKeystore = Keystore.debug( @@ -88,7 +88,7 @@ void main() { var keystore = Keystore.debug(); var ioHelper = _FrameIoHelperTest(_getBytes(frameBytesEncrypted)); var recoveryOffset = await ioHelper.framesFromFile( - 'null', keystore, testRegistry, testCipher); + await getTempRaf(const []), keystore, testRegistry, testCipher); expect(recoveryOffset, -1); var testKeystore = Keystore.debug( From 6fbc99d34485f6cde1d13045eb75ff114042bef7 Mon Sep 17 00:00:00 2001 From: The one with the braid Date: Tue, 25 Apr 2023 14:21:05 +0200 Subject: [PATCH 2/3] chore: add rethrow Signed-off-by: The one with the braid --- hive/lib/src/backend/vm/storage_backend_vm.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/hive/lib/src/backend/vm/storage_backend_vm.dart b/hive/lib/src/backend/vm/storage_backend_vm.dart index 591cee058..ca5bb44da 100644 --- a/hive/lib/src/backend/vm/storage_backend_vm.dart +++ b/hive/lib/src/backend/vm/storage_backend_vm.dart @@ -111,6 +111,7 @@ class StorageBackendVm extends StorageBackend { stackTrace: s, name: 'hive', ); + rethrow; } } From 92cdb14942221c896a9ab30ee117fc598ff2d1f9 Mon Sep 17 00:00:00 2001 From: The one with the braid Date: Tue, 25 Apr 2023 14:30:32 +0200 Subject: [PATCH 3/3] fix: check if lock file open before closing Signed-off-by: The one with the braid --- hive/lib/src/backend/vm/storage_backend_vm.dart | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hive/lib/src/backend/vm/storage_backend_vm.dart b/hive/lib/src/backend/vm/storage_backend_vm.dart index ca5bb44da..ff099d000 100644 --- a/hive/lib/src/backend/vm/storage_backend_vm.dart +++ b/hive/lib/src/backend/vm/storage_backend_vm.dart @@ -230,7 +230,9 @@ class StorageBackendVm extends StorageBackend { await writeRaf.close(); await lockRaf?.close(); - await _lockFile.delete(); + if (await _lockFile.exists()) { + await _lockFile.delete(); + } } @override