File.ReadAllTextAsync not behaving asynchronously when called on inaccessible network files

[cjwainwright]

When accessing a file on a network share that for some reason cannot be accessed (e.g. server not existent), File.ReadAllTextAsync appears to block, rather than running asynchronously, until the file request fails. This can be reproduced with the following console app:

using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Threading.Tasks;

namespace ReadAllTextAsync
{
    class Program
    {
        public static void Main(string[] args)
        {
            Console.WriteLine("Beginning");

            int count = 20;
            var files = Enumerable.Range(0, count).Select(i => $@"\\server_{i}\file.txt").ToList();
            var tasks = new List<Task>();

            foreach (string file in files)
            {
                tasks.Add(_ReadFile(file));
            }

            Console.WriteLine("Waiting...");

            Task.WhenAll(tasks).Wait();

            Console.WriteLine("Ending");
        }

        private static async Task _ReadFile(string file)
        {
            try
            {
                // await Task.Delay(1); // Uncomment this line allows calls to execute in parallel
                string text = await File.ReadAllTextAsync(file);
            }
            catch
            {
                Console.WriteLine($"Error reading file: {file}");
            }
        }
    }
}

The result of running the above code is a regular stream of (the expected) "Error reading file" messages but they come in slowly one after the other.

When uncommenting the await Task.Delay(1) line, the messages all come in nearly simulataneously. I would have expected this behaviour without this line.

I believe this is to do with the behaviour of the new FileStream object in https://github.com/dotnet/corefx/blob/release/2.0.0/src/System.IO.FileSystem/src/System/IO/File.cs

        private static StreamReader AsyncStreamReader(string path, Encoding encoding)
        {
            FileStream stream = new FileStream(
                path, FileMode.Open, FileAccess.Read, FileShare.Read, DefaultBufferSize,
                FileOptions.Asynchronous | FileOptions.SequentialScan);

            return new StreamReader(stream, encoding, detectEncodingFromByteOrderMarks: true);
        }

which is called before any code is awaited.

[danmoseley]

cc @JonHanna who added these (8afa432), in case he is interested.

[JonHanna]

Yeah, the problem being in the ctor would make sense.

Ideally we'd have a ObtainFileStreamAsync() we could use (does something like that already exist?). Failing that we could await Task.Yield() just before we get stream, or perhaps do so if it's a UNC path and not otherwise.

[svick]

@JonHanna

Failing that we could await Task.Yield() just before we get stream

I don't think that would be a good workaround, since it would still block the calling synchronization context. This matters especially in GUI apps, where it would still block the UI thread.

Doing something like var stream = await Task.Run(() => new FileStream(…)); should be better, since it's guaranteed to block a threadpool thread.

[JonHanna]

My idea about about only doing this for a UNC path isn't a good one, as mapped drives hide UNC and there can be other slow-to-access files.

Is there anything better than var stream = await Task.Run(() => new FileStream(…)); that anyone knows of? Otherwise I'll apply that?

[stephentoub]

Otherwise I'll apply that?

Any caller of this method could do the same thing if they need to, but doing it in the implementation means it's being forced on to everyone.

[JonHanna]

Not really, as obtaining the FileStream is within the method, so there's no way of either opting in or opting out of this.

If there isn't a better way, maybe that's the real problem here, and this should wait on such a better way being implemented first.

[stephentoub]

Not really, as obtaining the FileStream is within the method, so there's no way of either opting in or opting out of this.

Sure there is:

Task<string> task = Task.Run(() => File.ReadAllTextAsync(path));

or if you want to avoid the closure (though if you're ok reading the whole string and you're doing this because the operation is going to be blocking for a while, this allocation is a drop in the bucket):

Task<string> task = Task.Factory.StartNew(state => File.ReadAllTextAsync((string)state), path,
    CancellationToken.None, TaskCreationOptions.None, TaskScheduler.Default).Unwrap();

maybe that's the real problem here

Right, Win32 doesn't provide any API that enables performing the open asynchronously.

[danmoseley]

-> Future as this seems to haev a workaround. cc @pjanotti

[lostmsu]

Just wasted >1h debugging my app only to discover this.

Guys, this is a popular method, and this issue violates the most basic expectation about it. The whole point of using it is to ensure the calling thread is not blocked.

[adamsitnik]

Guys, this is a popular method, and this issue violates the most basic expectation about it. The whole point of using it is to ensure the calling thread is not blocked.

The problem here is that none of the OSes that we support provides a way to asynchronously open a file. Reads and writes can be async, but not the opening.

https://stackoverflow.com/questions/41300873/how-to-open-a-file-in-windows-asynchronously

[lostmsu]

@adamsitnik is there any reason this can't be done by queuing a task on the default threadpool?

[stephentoub]

@adamsitnik is there any reason this can't be done by queuing a task on the default threadpool?

You can do the same:

Task.Run(() => File.ReadAllTextAsync(...))

As a general policy we don't queue synchronous portions of APIs. In the extreme it would mean wrapping the body of every async method with a Task.Run or equivalent, which is something we don't do in general as well.
https://devblogs.microsoft.com/pfxteam/should-i-expose-asynchronous-wrappers-for-synchronous-methods/

[lostmsu]

I don't see how the logic in this post applies to this case. Nobody is suggesting wrapping a synchronous FileStream constructor in FileStream.CreateAsync. This is a different case, as ReadAllTextAsync is already asynchronous method, and it breaks the async expectation with the current implementation.

[stephentoub]

and it breaks the async expectation with the current implementation

It does what it can do with async I/O. It doesn't artificially do any async-over-sync work. The point was we generally don't do that.

[jozkee]

Closing as this seems to be non-actionable.