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

Support streams #124

Open jvilk opened 9 years ago

jvilk commented 9 years ago

We can trivially pull in the browserify module for streams (which is essentially identical to Node's stream module) and support streams on the file system.

raptor commented 7 years ago

Hi,

I'm using BrowserFS via 'the emularity' for an old DOSBox+Win3.1 game called Stars! My port to the browser fails with DOSBox throwing an error saying 'file creation failed', and I think it's because it's attempting to use a file stream in a certain instance (other file creation works just fine).

When you say 'trivially pull in the browserify module', is this something I could possibly help out with? Or, could there be a workaround that is easy to implement?

Thanks!

Swivelgames commented 7 years ago

@raptor I'm not a maintainer, but this would be huge! :)

jvilk commented 7 years ago

@raptor Sorry for the extreme delay; I thought I had responded to this.

I wasn't aware that Emscripten-compiled programs used NodeJS file streams. Is this true?

It is a rather trivial change to hook up basic streams; I could add a generic top-level stream interface that loads the entire file into memory and sends it through the stream at once.

raptor commented 7 years ago

Hi,

The 'em-dosbox' project doesn't look like it uses NodeJS file streams, but I still have one very specific failure when running my game under it that happens only on a BrowserFS filesystem. I thought it was due to lack of stream support, but I'm unsure now.

jvilk commented 7 years ago

Do you have a publicly available demo that has the failure so I can take a look?

jvilk commented 7 years ago

Also, make sure you're using the latest version of BFS, which fixed an issue with . and .. directories in directory listings in Emscripten.

raptor commented 7 years ago

Hi, yes, I just updated the repo:

https://github.com/stars-4x/stars-browser

To replicate the error:

  1. Clone the project above
  2. Open stars.html in a browser, wait for it to load
  3. Use the following serial number: ECN101WP
  4. Click 'New Game'. Click OK to accept defaults.
  5. Save the game anywhere on the C: drive. This is a BrowserFS drive through the Emularity. (If you save to Z: the test should work as that is emscripten-only)
  6. Once the game window appears and finishes loading, use the menu option Turn -> Generate
  7. Here the game fails with error message Unable to create host file...

If you exit Windows 3.11 and go to the DOS prompt, you can see that some of the game files are created in the folder, but not the ones that errored. One interesting thing is that the files that errored are created successfully in the subfolder BACKUP first, just not afterwards in the game save folder.

Thank you for being willing to take a look.

jvilk commented 7 years ago

OK, the issue has nothing to do with streams! Emscripten has its own internal "stream" abstraction that it implements using standard Node file operations.

The code is opening C/GAME.m3 with flags set to 0x8002. This value maps to the Node permission string "r+", which has the following meaning:

Open file for reading and writing. An exception occurs if the file does not exist.

C/GAME.m3 does not exist, so it throws an exception. (The directory listing contains ["WINDOWS", "GAME.XY", "GAME.H1", "GAME.H2", "GAME.H3", "BACKUP", "NOTEBOOK", "STARS", "runstars.bat"])

So, BrowserFS appears to be doing everything correctly. There's something wrong further up the chain.

Possibilities:

  1. The application expects this failure, but with a different error code than ENOENT, which is the error code BrowserFS gives it.
  2. The application has somehow been led to believe that the file already exists.
  3. The application expects that a previous FS command created the file. Possibly the application forgot to close the file descriptor.
  4. Emscripten's flags to permission string mapping is incorrect. Possible, since the Node FS doesn't appear to be as well-tested as other backends.
  5. There's an incorrect default flag value somewhere in the pipeline.
  6. (Unlikely) The code "just works" on existing file systems, which are not as rigorous at implementing this particular flag.

Will look into more later. I'd like to see what other FS operations the program is doing.

jvilk commented 7 years ago

@raptor this is what stars is doing:

renameSync /c/GAME.X1 /c/BACKUP/GAME.X1
renameSync /c/GAME.M1 /c/BACKUP/GAME.M1
renameSync /c/GAME.X2 /c/BACKUP/GAME.X2
renameSync /c/GAME.M2 /c/BACKUP/GAME.M2
renameSync /c/GAME.X3 /c/BACKUP/GAME.X3
renameSync /c/GAME.M3 /c/BACKUP/GAME.M3
renameSync /c/GAME.HST /c/BACKUP/GAME.HST
openSync /c/GAME.M1 r+

openSync fails because /c/GAME.M1 no longer exists, and r+ doesn't create the file.

Does Stars work under native DOSBox?

jvilk commented 7 years ago

This is weird. The DOSBox code should do a wb+ after the failed r+, but I'm not seeing that happen. Setting breakpoints in FS functions seems to cause Emscripten to throw a SimulateInfiniteLoop error, so it's difficult to debug...

I've reached the limits of what I can look into tonight, but something strange is happening. :-)

raptor commented 7 years ago

Stars! runs just fine in normal DOSBox, which you can see with this bundle from the Stars! community:

https://github.com/stars-4x/starsbox/releases

I'm not sure it matters, but I'm using the em-dosbox-svn-sdl2 branch of em-dosbox.

(Should this conversation be taken to another issue? I didn't mean to hijack the thread.)

jvilk commented 7 years ago

@raptor The issue was subtle, and is present in Emscripten's official nodefs as well.

I have fixed your issue in v1.2.1. Please let me know if you have any further trouble.

raptor commented 7 years ago

Thanks! It works!

james-pre commented 1 year ago

Please use https://github.com/browser-fs/core/issues/4