Race conditions with Expo FS
chmac opened this issue · 4 comments
It occurs to me that if I send 2 log messages at the same time, there will be a race condition here:
If I have 2 simultaneous writes, they will both read the original file, and they'll both write each having added 1 line, and whichever write succeeds last will overwrite the first write.
I've tested this in our app like so:
log.debug("First message #UKxKek");
log.debug("Second message #UKxKek");
log.debug("Third message #UKxKek");
log.debug("Fourth message #UKxKek");We only get the fourth message in our log files. Unfortunately this is a bit of a hard problem to solve. I'd assume that because RNFS exposes an appendFile() API that it doesn't suffer from this problem.
Yes the problem is that, unlike RNFS, Expo Fs does not expose the 'appendFile' method. So, for every log, i need to read the contents of the file and write it in full, this obviously causes the problem you described.
There are two ways to fix it:
-
make the procedure synchronous (but this would cause a lot of performance problems)
-
Rewrite the module to work with a queue, string all write requests, and execute them one at a time.
This last option, however, requires a lot of work, I hope they will soon add to expo fs a potion to add lines to a file ...
in the coming weeks I will consider what to do.
@alessandro-bottamedi Yeah, it's a tough one.
I looked a little into RNFS and they have this "append to file" implemented in native code. What about we open a feature request with Expo to add this option? Seems like the code already exists from RNFS so perhaps it's not so much work for Expo to add this option.
Otherwise, I also considered the queue option, and that seems like it adds a lot of complexity. I considered adding it in our logs, because then I could also delay sending logs until we'd initialised our logging settings. That would let me store log levels on the filesystem in the phone for example. But, it's complicated and has the potential to introduce horrible, hard to trace bugs.
One thing to note, I've used a few async / Promise queues, and there are some battle tested libraries out there. Maybe using an existing queue library would be achievable without introducing so much extra code into this package, but adding extra dependencies (which bring their own complications of course).
Anyway, thanks for the considered response. I'll be curious to hear what you think makes sense here.
@chmac @alessandro-bottamedi Hello, please tell me how you solved this problem
@chmac With v 3.0.4 i think I was able to create a custom queue for the ExpoFS module to avoid race conditions issues. I could not use existing queue management packages since they all make use of a DB (sql or realm) and are not compatible with expo.
Please try it and let me know if there are any problems.