0

I try to write app which takes a bunch of URLs and asynchronously saves theirs content in separated files. I wrote that code as synchronous and it worked quite okay so I tried to make it async. The problem is that I get some exceptions: The process cannot access the file because it is being used by another process. I don't know much about streams but is it possible that 2 threads share the same stream and temporarily close "their" files but not fully and that's why I've got that error? If not, what can it be?

public override async Task ExecuteCommandAsync(IEnumerable<string> urls)
{
    string directory = "some directory";
    int i = 0;
    foreach (var url in urls)
    {
        tasks.Add(Task.Run(async () =>
        {
            try
            {
                await DownloadJsonFromUrl(url, directory, i);
            }
            catch (Exception ex)
            {
                Console.WriteLine(ex.Message);
            }
        }));
        i++;
        Console.WriteLine($"task nr {i} started.");
    }
    await Task.WhenAll(tasks);
private async Task DownloadJsonFromUrl(string url, string directory, int fileNumber)
{
    using (var httpClient = _clientFactory.CreateClient())
    using (var response = await httpClient.GetAsync(url,
        HttpCompletionOption.ResponseHeadersRead))
    using (FileStream fileStream = File.Open(directory + fileNumber.ToString() + ".json",
        FileMode.Create, FileAccess.Write, FileShare.None))
    using (var clientStream = await response.Content.ReadAsStreamAsync())
    {
        await clientStream.CopyToAsync(fileStream);
    }
}
Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
Dzemojad
  • 13
  • 2

1 Answers1

0

There are a whole bunch of issues with your code. However, the immediate problem is that you are using i in your lambda, which creates a closure. Closures close over variables, not over values. Ergo, it was trying to write to the same file over and over again concurrently.

Take for instance your original code slightly modified.

foreach (var url in urls)
{
   tasks.Add(Task.Run(async () => 
   {
      try
      {
         Console.WriteLine($"task nr {i} started.");
         await Task.Delay(100);
      }
      catch(Exception ex)
      {
         Console.WriteLine(ex.Message);
      }
   }));
   i++;
}

The output would be

task nr 10 started.
task nr 10 started.
task nr 10 started.
task nr 10 started.
task nr 10 started.
task nr 10 started.
task nr 10 started.
task nr 10 started.
task nr 10 started.
task nr 10 started.

The easiest solution would be to create a copy:

foreach (var url in urls)
{
   var newI = i;

   ...

   tasks.Add(Task.Run(async () =>
     
       ...

       await DownloadJsonFromUrl(url, directory, newI);

       ...

However, let's take this a step further, clean up your tasks and lamdas and make sure you have unique file name. For simplicity let's just use the Select overload which has an index, then project the tasks which can be awaited:

Given

private async Task DownloadJsonFromUrl(string url, string path)
{
   using var httpClient = _clientFactory.CreateClient();

   using var response = await httpClient
      .GetAsync(url, HttpCompletionOption.ResponseHeadersRead)
      .ConfigureAwait(false);

   response.EnsureSuccessStatusCode();

   await using var fileStream = new FileStream(
      path,
      FileMode.Create, 
      FileAccess.Write,
      FileShare.None,
      1024*80,
      FileOptions.Asynchronous);

   await using var clientStream = await response.Content
      .ReadAsStreamAsync()
      .ConfigureAwait(false);

   await clientStream
      .CopyToAsync(fileStream)
      .ConfigureAwait(false);
}

Usage

public async Task ExecuteCommandAsync(IEnumerable<string> urls)
{
   var directory = "some directory";

   async Task DownloadAsync(string url, int i)
   {
      try
      {
         Console.WriteLine($"task nr {i} started.");
         await DownloadJsonFromUrl(url, Path.Combine(directory, $"{i}.json"));
      }
      catch (Exception ex)
      {
         Console.WriteLine(ex.Message);
      }
   }

   var tasks = urls.Select(DownloadAsync);

   await Task.WhenAll(tasks);
}
halfer
  • 19,824
  • 17
  • 99
  • 186
TheGeneral
  • 79,002
  • 9
  • 103
  • 141