isomorphic-git / lightning-fs

A lean and fast 'fs' for the browser
MIT License
477 stars 47 forks source link

Same fs instance everywhere #16

Closed hsablonniere closed 5 years ago

hsablonniere commented 5 years ago

Hello @wmhilton and team :wink:

One liner

I think the docs should make it very important that your app needs to have one and only one instance of new FS('fs').

Details

We only tested on Firefox, but in one of our codebase we had several instances of fs (in different modules) with const fs = new FS('fs'). We hoped they would be in sync since it's the same IndexedDB behind. It seems like there is a cache layer or something with the transactions. Maybe it comes from how idb-keyval works.

Anyway, if you don't use the same fs instance when you try to readdir or any other fs operations than the instance you used with git.plugins.set('fs', fs); you will have sync problems.

I know the getting started for browser mentions a global var but it does not say much more.

Questions

Thanks :smile:

billiegoose commented 5 years ago
const fs1 = new FS('global')
const fs2 = new FS('third-party')

but indeed would have very strange effects if the two instances are using the same IndexedDB backend! like

const fs1 = new FS('global')
const fs2 = new FS('global')

they'd end up fighting and clobbering each other. :(

I think it's a footgun!

That's probably the quickest solution.

It would be nice if the API made such mistakes impossible though... but if we kept references that would make filesystems impossible to garbage collect... unless we used WeakMaps but then I'm not sure whether that's supported in all the browsers lightning-fs currently works in......

So sorry you ran into this issue. Just in case you're wondering, you run into a similar issue if you try to access the same FS from the main thread and a worker thread (which has had me pondering how to make the fs thread-safe).

I'll try to update the documentation later tonight after my bike ride.

billiegoose commented 5 years ago

oh that's why this wasn't closed. I'd forgot to merge the PR 🤦‍♂

isomorphic-git-bot commented 5 years ago

:tada: This issue has been resolved in version 3.3.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

billiegoose commented 5 years ago

@hsablonniere the situation should be improved now. The latest version of LightningFS is now threadsafe.

Using a single FS instance per thread is still better, because it won't have to compete for the mutex, but the two instances won't get out of sync anymore.