Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[browser][io] Emscripten does not support file locking. #40065

Open
kjpou1 opened this issue Jul 29, 2020 · 7 comments
Open

[browser][io] Emscripten does not support file locking. #40065

kjpou1 opened this issue Jul 29, 2020 · 7 comments
Assignees
Labels
arch-wasm WebAssembly architecture area-System.IO
Milestone

Comments

@kjpou1
Copy link
Contributor

kjpou1 commented Jul 29, 2020

Advisory lock on an open file is provided by calling flock on the filestream.
https://github.com/dotnet/runtime/blob/master/src/libraries/Native/Unix/System.Native/pal_io.c#L641-L646

This does not work correctly in the filesystem interface running under wasm.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Jul 29, 2020
@marek-safar marek-safar removed area-CoreLib-mono untriaged New issue has not been triaged by the area owner labels Jul 31, 2020
@marek-safar marek-safar added this to the 5.0.0 milestone Jul 31, 2020
@marek-safar
Copy link
Contributor

marek-safar commented Jul 31, 2020

@kjpou1 what do mean it does not work correctly? Does it work only for some operations or not at all?

@kjpou1
Copy link
Contributor Author

kjpou1 commented Aug 3, 2020

It does not seem to work at all. What does not work it that it does not throw an error.

            using (File.Create(path))
            {
                Assert.Throws<IOException>(() => File.WriteAllBytes(path, bytes));
                Assert.Throws<IOException>(() => File.ReadAllBytes(path));
            }

The test above does not throw an error and in this case will write to the file and then read the bytes from the file.

@marek-safar
Copy link
Contributor

That's strange and it looks like emscripten bug. The API is implemented and it's used by some of emscripten internal tooling (https://github.com/emscripten-core/emscripten/blob/master/tools/filelock.py#L363). Could you report a bug?

@kjpou1
Copy link
Contributor Author

kjpou1 commented Aug 3, 2020

Issue opened here: emscripten-core/emscripten#11797

@SamMonoRT
Copy link
Member

Assigning to @kjpou1 - not sure if there were more discussions since your last comment with emscripten issue linked.

@steveisok
Copy link
Member

Not sure that file locking matters in the vfs context. Also doesn't seem to be that high of a priority on the emscripten side. I would recommend closing and adjusting the tests accordingly.

Note that the current emscripten implementation is just going to blanket succeed.

https://github.com/emscripten-core/emscripten/blob/0d013b6af862948f322b330a8f83159e50b486e1/src/library.js#L108

@steveisok steveisok modified the milestones: 5.0.0, Future Aug 21, 2020
@kjpou1
Copy link
Contributor Author

kjpou1 commented Aug 24, 2020

Steve, I think this really is a bug and should be fixed. Of course lower priority but still should work as designed if someones code is relying on the file locking. Right now the tests use ActiveIssue so that the tests are marked and continue to be documented.

I that is not what everyone wants could I be provided more of a description of how we should adjust the tests if we are to close this issue.

Not use ActiveIssue and do a platform specific test as follows:

[PlatformSpecific(~TestPlatforms.Browser)] // Browser VFS platform does not support file locking at this time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-wasm WebAssembly architecture area-System.IO
Projects
None yet
Development

No branches or pull requests

5 participants