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

Conversion to async #62

Closed
wants to merge 2 commits into from
Closed

Conversation

sebastienros
Copy link

Fixes #53

I gave it a try to convert anything that would require async. I didn't go with duplicating methods.
All tests pass, if not run concurrently ;) there must be some wrong locking of maybe I missed an await and some race condition happens. Running groups of them work fine.

Hopefully it gives an idea of the required changes, and the usage. Would be a major version bump, losing .NET 4.0 target too. I decided to not use the 'Async' suffix has it made less work, but I would suggest to add it.

I hope someone will try and improve it to fix the concurrency issues, but I won't be offended if you close the PR, I don't think I will put up much time on it, though didn't want to go to waste either.

@xoofx
Copy link
Owner

xoofx commented Oct 31, 2021

Thanks Seb, yeah I can see that it is quite some work... So as I said in the issue above:

It would require to duplicate the interface and implement all the Async methods

I would prefer that we introduce a new IFileSystemAsync interface and let the existing implementation redirect the new XXXAsync methods to their sync counterpart. This should be possible to do that on the base class without having to rewrite for everything.

Also, today, none of the .NET API (and so Win32 APIs) exposed by IFileSystem are providing Async alternatives, so it would be mostly for remote IFileSystem scenarios.

@xoofx xoofx deleted the branch xoofx:master February 4, 2022 17:42
@xoofx xoofx closed this Feb 4, 2022
@GerardSmit GerardSmit mentioned this pull request Oct 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Async File Systems
2 participants