umount of inmemory file system doesn't work on first attempt

jvilk / BrowserFS

BrowserFS is an in-browser filesystem that emulates the Node JS filesystem API and supports storing and retrieving files from various backends.

Other

3.07k stars 220 forks source link

umount of inmemory file system doesn't work on first attempt #140

Closed hrj closed 8 years ago

hrj commented 8 years ago

I am not sure if I am doing something wrong, but other filesystems unmount in first attempt but the inmemory filesystem requires a bit of coaxing.

When I do this:

        mfs.mountList.forEach((m) => mfs.umount(m));

..two of the filesystems unmount, but the third one, which is an in-memory filesystem created with:

mfs.mount('/tmp', new BrowserFS.FileSystem.InMemory());

...doesn't unmount.

Calling unmount a second time works though!

jvilk commented 8 years ago

That seems like a bug!

jvilk commented 8 years ago

I would bet this is an issue with nested mount points. What mountpoints are you using? Anything like /foo/bar and /foo? It should be modified to keep /foo/bar when /foo is unmounted, I think.

hrj commented 8 years ago

No, the three mountpoints are non-overlapping AFAICT. Unless, Doppio / BrowserFS adds more mountpoints automatically and doesn't report them in mountList() (maybe because they are nested?)

jvilk commented 8 years ago

@hrj is this still an issue? If so, can you try to make a small, reproducible test case?

hrj commented 8 years ago

@jvilk Yeah, it is still an issue. I tried with doppiojvm:0.4.2 and browserfs:0.5.13

I will try to extract a minimal test-case, but it will take some time. I can describe what I am doing though:

Create an in-memory filesystem and mount it: mfs.mount('/tmp', new BrowserFS.FileSystem.InMemory());
Run doppio with some Java code that compiles user-provided Java code using javax.tools.JavaCompile. The compiled output is written to /tmp.
jvm is started with jvm.runClass() and in the callback-function which is called after the class is run, I call umount on all filesystems. Only the /tmp filesystem doesn't unmount on first attempt.

hrj commented 8 years ago

@jvilk Same results with "browserfs": "^1.1.0" and "doppiojvm": "^0.5.0"

hrj commented 8 years ago

Oh, I see the problem now. In umount you have this code: this.mountList.splice(this.mountList.indexOf(mountPoint), 1);

My code was: mfs.mountList.forEach((m) => mfs.umount(m));

It seems that the splice causes forEach to skip an entry during iteration. Hence one umount call was getting skipped.

I guess I need to clone the mountList before iterating over it. Closing the issue! Sorry for the noise.