ivoleitao / stash

Key-value store abstraction with plain and cache driven semantics and a pluggable backend architecture.
MIT License
86 stars 16 forks source link

RangeError in FileStore causes read failure. #11

Closed misha closed 3 years ago

misha commented 3 years ago

Describe the bug

I'm trying to use FileStore via newLocalDiskCache, and I periodically get this error when retrieving data from the cache:

Unhandled Exception: RangeError (byteOffset): Invalid value: Valid value range is empty: 0
_ByteDataView.getUint64     (dart:typed_data-patch/typed_data_patch.dart:4762:7)
BytesReader.readUInt64       package:stash/…/codec/bytes_reader.dart:76
DiskStore._readEntry         package:stash_file/…/file/file_store.dart:214
DiskStore._readFileEntry.<anonymous closure> package:stash_file/…/file/file_store.dart:231
_rootRunUnary               (dart:async/zone.dart:1362:47)
<asynchronous suspension>

Here's my definition of the cache:

...
  final _disk = newLocalDiskCache(
          cacheName: 'disk',
          codec: ImageCacheCodec(),
          path: path,
        );
...

I don't believe the ImageCacheCodec has anything to do with the error. The line where the error actually occurs, is where the CacheEntry is being read from bytes, well before the codec has a chance to read anything. But if you need the source anyway feel free to ask; it just reads/writes ZLib-encoded Uint8List bytes.

To Reproduce

  1. Create a cache using newLocalDiskCache.
  2. Hammer it with reads/writes.
  3. Eventually the error above occurs.

Expected behavior

Reads should succeed if the data exists, or fail silently if it doesn't.

Screenshots

No screenshots.

Version

Dart SDK version: 2.13.0-211.14.beta (beta) (Mon May 3 08:08:14 2021 +0200) on "linux_x64"

Additional context

This occurred in a Pixel 4XL emulator running Android 29 with the latest build tools.

ivoleitao commented 3 years ago

Thank for your report, at the time I developed this storage extension I didn't prepare it for concurrency and this seems to be related. Although dart is single threaded multiple async calls could get entangled together and I think that's what's happening in here.

I'm currently working on an important change that I plan to release next weekend but I think I can try to protect the critical regions (they are very well defined on the code) with locks either using synchronized package for example or resorting to file locks. I will add that to the release on the next weekend as I can work on this only then.

It will be useful to have a unit test simulating this scenario as well. I will try also to add it if time permits.

I guess in retrospective it does not help to do the unit tests using file with a in-memory file system...

ivoleitao commented 3 years ago

@misha I've published a new version of stash_file that uses file locks. I was not able to simulate the error above but I hope this version provides enough protection to avoid it.

There is some other changes on this version that could cause some minor compile errors but nothing big, it should be fairly easy to fix. Please import stash_file version 3.0.0-dev.2. When you use the newLocalFileCache like in the example bellow it will automatically activate the locking mechanism

  final cache = newLocalFileCache(path,
      maxEntries: 10,
      eventListenerMode: EventListenerMode.Sync,
      fromEncodable: (json) => Task.fromJson(json))
    ..on<CreatedEntryEvent>().listen(
        (event) => print('Entry key "${event.entry.key}" added to the cache'));
ivoleitao commented 3 years ago

Any update on this one @misha ? I will leave this open till the end of the week then I will close it