Runtime: Add async alternatives to File.ReadAll*, File.AppendAll* and File.WriteAll*

[khellang]

Is there a reason why there's no ReadAll*Async, AppendAll*Async and WriteAll*Async methods on File?

The existing methods are very convenient and I think it makes sense to have async versions available.

I propose the following additions:

public static partial class File
{
     public static void AppendAllLines(string path, IEnumerable<string> contents) { }
     public static void AppendAllLines(string path, IEnumerable<string> contents, Encoding encoding) { }
+    public static Task AppendAllLinesAsync(string path, IEnumerable<string> contents, CancellationToken cancellationToken = default(CancellationToken)) { return default(Task); }
+    public static Task AppendAllLinesAsync(string path, IEnumerable<string> contents, Encoding encoding, CancellationToken cancellationToken = default(CancellationToken)) { return default(Task); }

     public static void AppendAllText(string path, string contents) { }
     public static void AppendAllText(string path, string contents, Encoding encoding) { }
+    public static Task AppendAllTextAsync(string path, string contents, CancellationToken cancellationToken = default(CancellationToken)) { return default(Task); }
+    public static Task AppendAllTextAsync(string path, string contents, Encoding encoding, CancellationToken cancellationToken = default(CancellationToken)) { return default(Task); }

     public static byte[] ReadAllBytes(string path) { return default(byte[]); }
+    public static Task<byte[]> ReadAllBytesAsync(string path, CancellationToken cancellationToken = default(CancellationToken)) { return default(Task<byte[]>); }

     public static string[] ReadAllLines(string path) { return default(string[]); }
     public static string[] ReadAllLines(string path, System.Text.Encoding encoding) { return default(string[]); }
+    public static Task<string[]> ReadAllLinesAsync(string path, CancellationToken cancellationToken = default(CancellationToken)) { return default(Task<string[]>); }
+    public static Task<string[]> ReadAllLinesAsync(string path, Encoding encoding, CancellationToken cancellationToken = default(CancellationToken)) { return default(Task<string[]>); }

     public static string ReadAllText(string path) { return default(string); }
     public static string ReadAllText(string path, System.Text.Encoding encoding) { return default(string); }
+    public static Task<string> ReadAllTextAsync(string path, CancellationToken cancellationToken = default(CancellationToken)) { return default(Task<string>); }
+    public static Task<string> ReadAllTextAsync(string path, Encoding encoding, CancellationToken cancellationToken = default(CancellationToken)) { return default(Task<string>); }

     public static void WriteAllBytes(string path, byte[] bytes) { }
+    public static Task WriteAllBytesAsync(string path, byte[] bytes, CancellationToken cancellationToken = default(CancellationToken)) { return default(Task); }

     public static void WriteAllLines(string path, IEnumerable<string> contents) { }
     public static void WriteAllLines(string path, IEnumerable<string> contents, Encoding encoding) { }
+    public static Task WriteAllLinesAsync(string path, IEnumerable<string> contents, CancellationToken cancellationToken = default(CancellationToken)) { return default(Task); }
+    public static Task WriteAllLinesAsync(string path, IEnumerable<string> contents, Encoding encoding, CancellationToken cancellationToken = default(CancellationToken)) { return default(Task); }

     public static void WriteAllText(string path, string contents) { }
     public static void WriteAllText(string path, string contents, Encoding encoding) { }
+    public static Task WriteAllTextAsync(string path, string contents, CancellationToken cancellationToken = default(CancellationToken)) { return default(Task); }
+    public static Task WriteAllTextAsync(string path, string contents, Encoding encoding, CancellationToken cancellationToken = default(CancellationToken)) { return default(Task); }
}

I guess the ReadLines methods are candidates for this as well, but I don't know how you'd handle the async enumeration part yet.

[NickCraver]

Note: there's the relevant File.ReadLines with the async IEnumerable work...depending on how that progresses as well as all the Set___ methods, like File.SetAccessControl. All would be welcome additions, especially for network share files and such. Even File.Exists() and pretty much all others are candidates, IMO.

[khellang]

Note: there's also the relevant File.ReadLines with the async IEnumerable work... depending on how that progresses. It'd be a welcome addition as well, especially for network share files and such.

Yup, hence the comment:

I guess the ReadLines methods are candidates for this as well, but I don't know how you'd handle the async enumeration part yet.

:wink:

[NickCraver]

Here's the async sequences discussion for those that want to follow: https://github.com/dotnet/roslyn/issues/261

[khellang]

What does "api-needs-work" mean? Are you interested in a PR for this? Does it need a review first? I'd love to submit a PR, if you can walk me through the process :smile:

[stephentoub]

@terrajobst can correct me if I misapplied it; my understanding is that it essentially means "an API proposal that's still being discussed". Some point when the owner (aka @JeremyKuhne) deems it ready for review, the label changes to api-ready-for-review. The process is outlined here: https://github.com/dotnet/corefx/blob/master/Documentation/project-docs/api-review-process.md

[khellang]

@stephentoub Thanks. Yeah, I spoke too soon. Found it in the process document right after I asked 😝 I'll await (no pun intended) @JeremyKuhne's response 😄

[JeremyKuhne]

I think it's reasonable to pursue this. Can you flesh out with usage examples and error conditions? I'll mark it for review after that.

[khellang]

Can you flesh out with usage examples and error conditions?

You mean show some sample code calling some of these methods?

These are async counterparts to the existing sync methods, so the error conditions would be the same as those, I assume.

[JeremyKuhne]

@khellang, yes, sample code. Nothing super complicated, just something to help clearly illustrate the value here. And it is fine to say that we follow the error conditions in the sync methods with a callout to what we do with the CancellationToken.

[khellang]

OK, an example. Deserializing JSON configuration:

public async Task<TConfiguration> ReadConfigurationAsync<TConfiguration>(string path)
{
    var json = await File.ReadAllTextAsync(path);
    return JsonConvert.DeserializeObject<TConfiguration>(json);
}

If you need more examples on usage of File.ReadAllText or File.ReadAllLines etc., you can also do a quick GitHub search:

• File.ReadAllLines (45,457 hits)
• File.ReadAllText (97,843 hits)

The usage would be exactly the same, only in an async world.

[khellang]

Also, FYI; the reason I filed this is that I'm getting upvotes on an answer I gave on Stack Overflow almost weekly. The question currently has 7,753 views.

[khellang]

I amended the proposal to remove the CancellationToken arguments for the [Read|Write|Append]AllTextAsync APIs, cause I couldn't see a good way to handle cancellation for those (the underlying StreamReader.ReadToEndAsync doesn't take a CancellationToken).

If you want to look at my initial stab at an implementation, see https://github.com/dotnet/corefx/compare/master...khellang:async-file-apis~1.

For the underlying (typically StreamReader) APIs accepting a CancellationToken, I've just passed the token along. For the others (typically those that loop through lines) I've just called ThrowIfCancellationRequested.

[karelz]

@JeremyKuhne what else do we need to move this API approval forward? @khellang has implementation ready, waiting just on us ...

[karelz]

@JeremyKuhne ping?
cc: @ianhays

[karelz]

@JeremyKuhne @ianhays ping?

[ianhays]

I like these. There are a few places where we have helpers like this that lack async equivalents e.g. ZipFile. I doubt we'll face much resistance to adding these, so lets take it to the next API review session.

[stephentoub]

@khellang, how did you decide whether to have an overload that does / does not take a CancellationToken?

[khellang]

how did you decide whether to have an overload that does / does not take a CancellationToken?

First I wanted all of them to take a CancellationToken, but since most of the methods deal with FileStream and a Stream[Reader|Writer], I was restricted to either checking cancellation in a loop, or pass the token along to the reader/writer API and let it handle cancellation.

Unfortunately, the StreamReader.ReadToEndAsync and StreamWriter.WriteAsync methods doesn't take a ´CancellationToken´.

[stephentoub]

@khellang, thanks.

Unfortunately, the StreamReader.ReadToEndAsync and StreamWriter.WriteAsync methods doesn't take a ´CancellationToken´.

We don't need to be constrained by what's currently available on StreamReader/StreamWriter... we don't even need to use StreamReader/Writer in the implementation if we don't want to, it's an implementation detail. Let's design the APIs we want and then figure out how to implement them.

since most of the methods deal with FileStream and a Stream[Reader|Writer]

What's the limitation with FileStream?

First I wanted all of them to take a CancellationToken

I think it makes sense for there to be an overload for each that takes a CancellationToken. Can you revise it accordingly, based on what you think the ideal is?

[khellang]

we don't even need to use StreamReader/Writer in the implementation if we don't want to, it's an implementation detail

Yeah, I just followed the implementation of what was already there (the sync overloads). Is there a reason why those reader/writer methods don't take a CancellationToken in the first place?

What's the limitation with FileStream?

Nothing. Where I've used the FileStream directly, I've used methods that take a CancellationToken.

I think it makes sense for there to be an overload for each that takes a CancellationToken. Can you revise it accordingly, based on what you think the ideal is?

Alright. I'll change it back to include cancellation tokens for the [Read|Write|Append]AllTextAsync APIs :smile:

[stephentoub]

Is there a reason why those reader/writer methods don't take a CancellationToken in the first place?

Why the synchronous ones don't? There are very few synchronous methods in the framework that take a CancellationToken; the only ones I can think of are on synchronization primitives, e.g. Wait(CancellationToken). While there is some value in adding sync methods that take CancellationToken, it's generally much less valuable than for the async ones, because of usage patterns but also because of the underlying mechanisms being used and whether they support cancellation.

[khellang]

No, I mean StreamReader.ReadToEndAsync and StreamWriter.WriteAsync :smile:

That's a pretty old issue; https://connect.microsoft.com/VisualStudio/feedback/details/790215/textreader-and-streamreader-read-async-methods-are-missing-a-cancellationtoken-parameter

[stephentoub]

No, I mean StreamReader.ReadToEndAsync and StreamWriter.WriteAsync

Oh... no idea why they were introduced without them ;)

[khellang]

Oh... no idea why they were introduced without them ;)

So... do I smell another API request/proposal/PR? :wink:

[stephentoub]

So... do I smell another API request/proposal/PR?

That ones a bit trickier, as there are virtual methods involved; ideally the virtual one would have been the one accepting a CancellationToken. We also don't want things proposed just because there's a theoretical gap, but rather something that is actually needed (I'm not casting judgement on this case, just citing that we only want to spend time on things that actually move the needle). But you're of course welcome to propose something if you think it's important.

[JeremyKuhne]

I'm happy with the current state of the proposal, sorry about the dead air.

[terrajobst]

Any reason we wouldn't make the cancellation token optional?

 public static partial class File
 {
      public static void AppendAllLines(string path, IEnumerable<string> contents);
      public static void AppendAllLines(string path, IEnumerable<string> contents, Encoding encoding);
+     public static Task AppendAllLinesAsync(string path, IEnumerable<string> contents, CancellationToken cancellationToken = default(CancellationToken));
+     public static Task AppendAllLinesAsync(string path, IEnumerable<string> contents, Encoding encoding, CancellationToken cancellationToken = default(CancellationToken)));

      // ...
 }

[khellang]

Any reason we wouldn't make the cancellation token optional?

Nope. I had them optional, but have been back and forth so much, I guess they got dropped at some point 😝

[terrajobst]

Alright, looks good as proposed.

[karelz]

Anyone wants to grab it? Just let me know and I will assign it to you ...

[khellang]

Anyone wants to grab it?

To do the work or the hand holding? 😝

[karelz]

To do the work ;-)

[khellang]

I'll do it 😁

[karelz]

Cool, thanks!

[am11]

Thanks in advance for the async I/O APIs! :)

This reminds me of a very insightful article by @StephenCleary: There is no thread and the quote (in comments):

The method APIs don't lie. They have only synchronous work to do, so they expose a synchronous API.

However, I/O does not fall in that category as under the hood, an asynchronous procedure call would be made for an I/O op (as described in the same blog).

[karelz]

@khellang just curious how's the work going. Let us know if we can help you get unblocked.

[karelz]

@khellang do you still plan to work on it?

[khellang]

@karelz I was, yes, but I've been swamped with work lately, so I probably won't be able to submit a PR until January. If anyone's eager to do this work at MS, go ahead :smile:

[karelz]

Thanks for clarification. Let's see if someone wants to pick it up before that (I would not hold my breath for MS contribution here, we have plenty of other high pri work to do ;)).
I will leave it assigned to you in the meantime.

[JonHanna]

If it's going to go awaiting until January, I'll take it.

[karelz]

Thanks @JonHanna, assigned to you ...

[JonHanna]

We do not recommend using optional args.
https://github.com/dotnet/corefx/issues/470#issuecomment-147809240

I'm using the optional args as spec'd above, but I can easily change that to doubling up on the overloads with simple inline-able methods, and I'm inclined to agree with the argument that I should. What can't be done easily is changing to that after the new API is in.

Thoughts?

[karelz]

That's an old comment. I think we changed our mind since then - @terrajobst @weshaggard can you please confirm?

[khellang]

The engineering guidelines mentions an exception for CancellationTokens.

[JonHanna]

Grand so.

[weshaggard]

I agree we should make them optional parameters. Also @JonHanna it is much easier to start with one API overload with an optional parameter and add another overload later if necessary, the other way is much harder to undo because we cannot remove an API.

[JonHanna]

PTAL @ dotnet/corefx#14557

[JonHanna]

Okay. There's something I'm not getting about the different targets. I thought at least part of the problem was conflicts with mscorlib but it seems from https://github.com/dotnet/coreclr/pull/8690#issuecomment-268278233 that I was wrong on that.

I'm giving up. If someone wants to take the changes in the PR above as a starting point, I was pretty happy with the actual implementation and test code.

[JonHanna]

I was hoping that the recent engineering changes would have made my difficulty earlier irrelevant, and it seems it has. PTAL at dotnet/corefx#15372.