Skip to content
This repository has been archived by the owner on Mar 16, 2019. It is now read-only.

Race condition with config() and cancel() #331

Closed
wcandillon opened this issue Apr 15, 2017 · 4 comments
Closed

Race condition with config() and cancel() #331

wcandillon opened this issue Apr 15, 2017 · 4 comments

Comments

@wcandillon
Copy link

wcandillon commented Apr 15, 2017

@wkh237 First things first, thank you for this amazing library!

I found a race condition between config() and cancel() which seems fairly easy to reproduce.
This race condition leads to broken images such as this one: https://www.dropbox.com/s/euwx03cglbuuu1h/Screenshot%202017-04-15%2010.16.26.png?dl=0 or facebook/react-native#9893.

cache.task = RNFetchBlob.config({ path }).fetch("GET", uri, {});
cache.task.catch(error => {
  console.log(error);
//This below should always return false but doesn't
// in a scenario where we do a lot of fetch() and cancel()
// for example when scrolling quickly through a feed of image
// see: https://hackernoon.com/image-pipeline-with-react-native-listview-b92d4768b17c
  RNFetchBlob.fs.exists(path).then(result => console.log(result));
});
@wkh237
Copy link
Owner

wkh237 commented Apr 15, 2017

@wcandillon , thanks for the feedback. Wondering if there's a sample project for replicate this one?

@wcandillon
Copy link
Author

@wkh237 Not at the moment but maybe in the near future. I think you can reproduce it by start many requests and canceling them. It looks the writing of the image to disk can be interrupted by cancel(), is that something you can potentially identify on the objectivec code?

The way I managed to work around this issue is by doing fs.unlink after canceling: https://github.com/wcandillon/react-native-img-cache/blob/master/src/index.tsx#L106

@wkh237
Copy link
Owner

wkh237 commented Apr 15, 2017

By design, if there's either path or fileCache property in config, the response data chunks will be written to the location directly. When canceling the task, the incomplete file will not be removed since some people may use it later (like resume the download by Range Request).

@wkh237 wkh237 added discussion and removed bug labels Apr 15, 2017
@wcandillon
Copy link
Author

@wkh237 Oh right that makes sense. Thank you for the feedback.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants