.NET CopyToFileAsync reciving "Safe handle has been closed." exception

Question

With the code below I am getting the "Safe handle has been closed." from CopyToFileAsync. This happens even if only a single file operation is taking place. Also, this only happens on large(ish) files.

As you can see the fille(s) are being updated from a post request over HTTP.

Is some sort of "keepalive" option needed? Is CopyToFileAsync inappropriate?

[HttpPost]
public async Task<IActionResult> PostFormData(List<IFormFile> files, CancellationToken cancellationToken = default)
{
    try {
        if (!Request.ContentType?.Contains("multipart/form-data") == true)
        {
            return BadRequest("Unsupported media type.");
        }

        string webRootPath = _webHostEnvironment.WebRootPath;
        string contentRootPath = _webHostEnvironment.ContentRootPath;

        List<Task> taskList = new List<Task>();

        foreach (var file in files)
        {
            if (file.Length > 0)
            {
                var fileName = Path.GetFileName(file.FileName);
                var filePath = Path.Combine(_webHostEnvironment.WebRootPath, fileName);

                using (var stream = new FileStream(filePath, FileMode.Create))
                {
                    taskList.Add(file.CopyToAsync(stream, cancellationToken));
                }
            }
        }

        int filesCopied = 0;
        if (taskList.Count > 0)
        {
            await Task.WhenAll(taskList);

            foreach (var task in taskList)
                if (task.Status == TaskStatus.RanToCompletion) 
                    filesCopied++;
        }

        return Ok(new { message = $"{filesCopied} Files uploaded successfully" });

    }
    catch (TaskCanceledException)
    {
        throw;
    }
    catch (System.Exception ex)
    {
        return BadRequest($"Error: {ex.Message}");
    }
}

You need to wrap the "using" in a task along with the file copy ... Currently, you're "launching", and "disposing" in the next instance. The stream is not being "captured". — Gerry Schmitz
– Gerry Schmitz, Commented Sep 15 at 4:43
Thanks. It was old code that used to be synchronous and I missed the school boy error :-( — McMurphy
– McMurphy, Commented Sep 15 at 5:42
If you try to copy 100 files at once you'll end up taking more time, because the disk has only so much bandwidth and the operations will conflict with each other. You can replace most of this code with await Parallel.ForEachAsync(files,async (file,cancellationToken)=> ....});. It's easier to handle erros by not letting them escape the loop, ie use try/catch. — Panagiotis Kanavos
– Panagiotis Kanavos, Commented Sep 15 at 7:40
Rather than catch (TaskCanceledException){ throw; you can just do catch (System.Exception ex) when (ex is not TaskCanceledException) Also BadRequest is probably the wrong HTTP code for a generalized failure. — Charlieface
– Charlieface, Commented Sep 15 at 11:20

Panagiotis Kanavos · Accepted Answer · 2025-09-16 07:21:17Z

3

As others said, the using block runs in a different thread than the file.CopyToAsync operation so the stream gets disposed before saving completes.

This can be fixed with :

using (var stream = new FileStream(filePath, FileMode.Create))
{
    await file.CopyToAsync(stream, cancellationToken);
}

To parallelize saving, don't rush to fire off 100 save operations. All write to the same disk, which means they conflict with each other, resulting in slower saves overall. If you limit the number of concurrent IO operations you could get some speedup due to OS and disk controller buffering. The easiest way is to use Parallel.ForEachAsync or an ActionBlock with a specific degree of parallelism, eg:

var filesCopied=0;
var fileErrors=0;

await Parallel.ForEachAsync(files,async (file,cancellationToken)=>
{
    if (file.Length>0)
    {
        try
        {
            var filePath = Path.Combine(_webHostEnvironment.WebRootPath, file.FileName);
            using (var stream = new FileStream(filePath, FileMode.Create))
            {
                await file.CopyToAsync(stream, cancellationToken);
            }
            Interlocked.Increment(ref filesCopied);
        }
        catch(Exception ex)
        {
            _logger.LogError(ex,"Error saving {file}",file.FileName);
            Interlocked.Increment(ref fileErrors);
        }
});

By default Parallel.ForEachAsync uses as many worker tasks as there are virtual cores, which can be too much. To reduce or increase them, you can use the ParallelOptions.MaxDegreeOfParallelism property :

var options = new ParallelOptions { MaxDegreeOfParallelism=4};


await Parallel.ForEachAsync(files, options, async (file,cancellationToken)=>
...

To get the best async performance the FileStream must be created with the isAsync flag. As the relevant constructor remarks say:

Using asynchronous I/O correctly can speed up applications by as much as a factor of 10, but using it without redesigning the application for asynchronous I/O can decrease performance by as much as a factor of 10.

The constructors that accept a file path and isAsync are either the most verbose or FileStream(String, FileStreamOptions), where all options are passed as a FileStreamOptions object.

So either:

await using (var stream = new FileStream(filePath, 
                                         FileMode.Create,
                                         FileAccess.ReadWrite,                
                                         FileShare.Read,
                                         4096,
                                         true))
{
...
}

or

var options = new FileStreamOptions() {
    Mode = FileMode.Create,
    Access = FileAccess.ReadWrite,
    Options = FileOptions.Asynchronous,
};

There's no need to specify Share and BufferSize because the defaults are Read and `4096.

Performance for large files can be improved by using the FileStreamOptions.PreallocationSize to allocate all the space needed at the beginning, instead of allocating new pages as the file gets larger :

var options = new FileStreamOptions() {
    PreallocationSize=file.Length,

    Mode = FileMode.Create,
    Access = FileAccess.ReadWrite,
    Options = FileOptions.Asynchronous,
};

edited Sep 16 at 7:21

answered Sep 15 at 7:52

Panagiotis Kanavos

134k16 gold badges211 silver badges270 bronze badges

Sign up to request clarification or add additional context in comments.

7 Comments

McMurphy Sep 15 at 13:05

Wow, two really excellent suggestions! Thank you. Especially identifying the copy du jour if something goes wrong. On that note if the Upload gets Aborted or Timedout do I have to (forget ParallelOptions) re-Await the COPY? IOW the Controller method got the cancel and the COPY is using the same CancelToken but presumably the controller method gets notified first or order is non-deterministic at best. As the main controller thread needs to clean up after the copy do I have to reawait the "current" copy to end and then tidy up?

dbc Sep 15 at 20:54

Would it be worth it to use await using (var stream = instead of using (var stream = and/or pass isAsync : true to the FileStream constructor ?

Panagiotis Kanavos Sep 16 at 6:57

@dbc it would, but the code gets very noisy and I wanted to show ForEachAsync, not the complexities of the FileStream constructor. You have to either specify everything (buffers, sharing etc) or you have to create a FileStreamOptions object

McMurphy Sep 15 at 21:49

Having slept un this, won't AWAITing the file copies simply serialize them?

Panagiotis Kanavos Sep 16 at 6:55

No. The lambdas are executed in parallel, by worker tasks managed by Parallel.ForEachAsync.

McMurphy Sep 16 at 8:49

Yes a very powerful feature! I thought you're first fix suggestion to my Safe Handle problem was just to stick in an AWAIT. But yes Parallel.ForEachAsync is amazing. Thanks again for showing me. Can you confirm that a canceltoken in the parent controller thread will cancel All iterations post cancel if they have the same token. That's how Javascript works. I want to add up runToCompletion, errored, and cancelled.

McMurphy Sep 17 at 1:37

Related quick question; if I don't want to short circuit the ForEachAsync loop just the individual copies, and understand that post cancel, all file status' will be "cancelled") would it be correct just to omit the Parallel Loop cancel token like ** await Parallel.ForEachAsync(files, options, async (file, _) => {} ** and then just check the filesCancelled count or ** if (cancellationToken.IsCancellationRequested) ** after the loop?

Collectives™ on Stack Overflow

.NET CopyToFileAsync reciving "Safe handle has been closed." exception

1 Answer 1

7 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

7 Comments

Your Answer

Sign up or log in

Post as a guest

Related