1

I am trying to use amazon s3 libs but somehow can't pass stream to upload data to s3.

At the beginning I was using using statements but then rewrote it to try catch dispose and still I can't even dispose the stream, stream is not null but I get exception when trying to dispose that "cannot access closed stream" on the line streamWriter?.Dispose();

var memoryStream = new MemoryStream();
var streamWriter = new StreamWriter(memoryStream, Encoding.UTF8);

try
{
    try
    {
        await streamWriter.WriteAsync(commandWithMetadata.SerializeToString());
        memoryStream.Seek(0, SeekOrigin.Begin);

        var fileName = GetFileName(command);
        var request = new PutObjectRequest
        {
            BucketName = BucketName,
            Key = fileName,
            InputStream = memoryStream
        };

        await client.PutObjectAsync(request);
    }
    finally
    {
        streamWriter?.Dispose();
    }
}
finally
{
    memoryStream?.Dispose();
}
GSerg
  • 76,472
  • 17
  • 159
  • 346
kosnkov
  • 5,609
  • 13
  • 66
  • 107
  • 2
    Are you calling this async function like `= this_function().Result`? Also please go back to using `using` which is the correct way. – GSerg Jan 13 '23 at 08:44
  • 1
    This code can leak because the resources are created *outside* the `try` block. Use `using` instead. There's no benefit from using `try/finally` like this. – Panagiotis Kanavos Jan 13 '23 at 08:51
  • Not part of the actual problem but the StreamWriter will dispose the given stream already (if you don't tell it to leave the stream open via the constructor). Currently you try to dispose the Memorystream twice. Actually a MemoryStream is ignorant to beeing disposed multiple times. In your real code is it a MemoryStream or something else? – Ralf Jan 13 '23 at 08:57
  • @Ralf It is a principle in .NET that `Dispose` [may be safely called](https://stackoverflow.com/a/5306896/11683) on any object multiple times. – GSerg Jan 13 '23 at 08:59
  • @GSerg good to know. Then i learned also now that a great deal of programmers i needed to use there code didn't adhere to that principle. – Ralf Jan 13 '23 at 09:01

1 Answers1

2

Try the following calling await streamWriter.FlushAsync(); before await client.PutObjectAsync(request);.

But I think better approach would be:

using (var memoryStream = new MemoryStream())
{
    using (var streamWriter = new StreamWriter(memoryStream, Encoding.UTF8, leaveOpen: true))
    {
        await streamWriter.WriteAsync(commandWithMetadata.SerializeToString());
    }
    memoryStream.Seek(0, SeekOrigin.Begin);

    //... send
}

By default StreamWriter on dispose tries to flush remining data in buffer and close underlying stream (which seems to be already closed by client.PutObjectAsync(request)) so use leaveOpen: true. Also there is no need to use try-finally-Dispose (and not fully correctly) - there is using statement which will do that for you.

Guru Stron
  • 102,774
  • 10
  • 95
  • 132