CSHARP-2373: GridFSForwardOnlyUploadStream not closing properly on .NET Standard 2+ when cast to Stream before calling Close.

Author: rstam

src/MongoDB.Driver.GridFS/GridFSDownloadStreamBase.cs

@@ -26,6 +26,7 @@ internal abstract class GridFSDownloadStreamBase<TFileId> : GridFSDownloadStream
         // private fields
         private readonly IReadBinding _binding;
         private readonly IGridFSBucket<TFileId> _bucket;
+        private bool _closed;
         private bool _disposed;
         private readonly GridFSFileInfo<TFileId> _fileInfo;
 
@@ -73,20 +74,28 @@ protected IGridFSBucket<TFileId> Bucket
         }
 
         // public methods
-        public override void Close()
-        {
-            Close(CancellationToken.None);
-        }
-
         public override void Close(CancellationToken cancellationToken)
         {
-            base.Close();
+            try
+            {
+                CloseIfNotAlreadyClosed(cancellationToken);
+            }
+            finally
+            {
+                Dispose();
+            }
         }
 
-        public override Task CloseAsync(CancellationToken cancellationToken = default(CancellationToken))
+        public override async Task CloseAsync(CancellationToken cancellationToken = default(CancellationToken))
         {
-            base.Close();
-            return Task.FromResult(true);
+            try
+            {
+                await CloseIfNotAlreadyClosedAsync(cancellationToken).ConfigureAwait(false);
+            }
+            finally
+            {
+                Dispose();
+            }
         }
 
         public override void Flush()
@@ -115,8 +124,37 @@ public override Task WriteAsync(byte[] buffer, int offset, int count, Cancellati
         }
 
         // protected methods
+        protected void CloseIfNotAlreadyClosedFromDispose(bool disposing)
+        {
+            if (disposing)
+            {
+                try
+                {
+                    CloseIfNotAlreadyClosed(CancellationToken.None);
+                }
+                catch
+                {
+                    // ignore exceptions when calling CloseIfNotAlreadyClosed from Dispose
+                }
+            }
+        }
+
+        protected virtual void CloseImplementation(CancellationToken cancellationToken)
+        {
+            // override in subclass if there is anything to do
+        }
+
+        protected virtual Task CloseImplementationAsync(CancellationToken cancellationToken)
+        {
+            // override in subclass if there is anything to do
+            CloseImplementation(cancellationToken);
+            return Task.FromResult(true);
+        }
+
         protected override void Dispose(bool disposing)
         {
+            CloseIfNotAlreadyClosedFromDispose(disposing);
+
             if (!_disposed)
             {
                 if (disposing)
@@ -137,5 +175,36 @@ protected virtual void ThrowIfDisposed()
                 throw new ObjectDisposedException(GetType().Name);
             }
         }
+
+        // private methods
+        private void CloseIfNotAlreadyClosed(CancellationToken cancellationToken)
+        {
+            if (!_closed)
+            {
+                try
+                {
+                    CloseImplementation(cancellationToken);
+                }
+                finally
+                {
+                    _closed = true;
+                }
+            }
+        }
+
+        private async Task CloseIfNotAlreadyClosedAsync(CancellationToken cancellationToken)
+        {
+            if (!_closed)
+            {
+                try
+                {
+                    await CloseImplementationAsync(cancellationToken).ConfigureAwait(false);
+                }
+                finally
+                {
+                    _closed = true;
+                }
+            }
+        }
     }
 }

src/MongoDB.Driver.GridFS/GridFSForwardOnlyDownloadStream.cs

@@ -36,7 +36,6 @@ internal class GridFSForwardOnlyDownloadStream<TFileId> : GridFSDownloadStreamBa
         private List<BsonDocument> _batch;
         private long _batchPosition;
         private readonly bool _checkMD5;
-        private bool _closed;
         private IAsyncCursor<BsonDocument> _cursor;
         private bool _disposed;
         private readonly BsonValue _idAsBsonValue;
@@ -92,18 +91,6 @@ public override long Position
         }
 
         // methods
-        public override void Close(CancellationToken cancellationToken)
-        {
-            CloseHelper();
-            base.Close(cancellationToken);
-        }
-
-        public override Task CloseAsync(CancellationToken cancellationToken = default(CancellationToken))
-        {
-            CloseHelper();
-            return base.CloseAsync(cancellationToken);
-        }
-
         public override int Read(byte[] buffer, int offset, int count)
         {
             Ensure.IsNotNull(buffer, nameof(buffer));
@@ -158,8 +145,24 @@ public override long Seek(long offset, SeekOrigin origin)
         }
 
         // protected methods
+        protected override void CloseImplementation(CancellationToken cancellationToken)
+        {
+            if (_checkMD5 && _position == FileInfo.Length)
+            {
+                var md5 = BsonUtils.ToHexString(_md5.GetHashAndReset());
+                if (!md5.Equals(FileInfo.MD5, StringComparison.OrdinalIgnoreCase))
+                {
+#pragma warning disable 618
+                    throw new GridFSMD5Exception(_idAsBsonValue);
+#pragma warning restore
+                }
+            }
+        }
+
         protected override void Dispose(bool disposing)
         {
+            CloseIfNotAlreadyClosedFromDispose(disposing);
+
             if (!_disposed)
             {
                 if (disposing)
@@ -190,25 +193,6 @@ protected override void ThrowIfDisposed()
         }
 
         // private methods
-        private void CloseHelper()
-        {
-            if (!_closed)
-            {
-                _closed = true;
-
-                if (_checkMD5 && _position == FileInfo.Length)
-                {
-                    var md5 = BsonUtils.ToHexString(_md5.GetHashAndReset());
-                    if (!md5.Equals(FileInfo.MD5, StringComparison.OrdinalIgnoreCase))
-                    {
-#pragma warning disable 618
-                        throw new GridFSMD5Exception(_idAsBsonValue);
-#pragma warning restore
-                    }
-                }
-            }
-        }
-
         private FindOperation<BsonDocument> CreateFirstBatchOperation()
         {
             var chunksCollectionNamespace = Bucket.GetChunksCollectionNamespace();

src/MongoDB.Driver.GridFS/GridFSForwardOnlyUploadStream.cs

@@ -151,45 +151,28 @@ public override long Position
             await operation.ExecuteAsync(_binding, cancellationToken).ConfigureAwait(false);
         }
 
-        public override void Close()
-        {
-            Close(CancellationToken.None);
-        }
-
         public override void Close(CancellationToken cancellationToken)
         {
-            if (_closed)
+            try
             {
-                return;
+                CloseIfNotAlreadyClosed(cancellationToken);
             }
-            ThrowIfDisposed();
-            _closed = true;
-
-            if (!_aborted)
+            finally
             {
-                WriteFinalBatch(cancellationToken);
-                WriteFilesCollectionDocument(cancellationToken);
+                Dispose();
             }
-
-            base.Close();
         }
 
         public override async Task CloseAsync(CancellationToken cancellationToken = default(CancellationToken))
         {
-            if (_closed)
+            try
             {
-                return;
+                await CloseIfNotAlreadyClosedAsync(cancellationToken).ConfigureAwait(false);
             }
-            ThrowIfDisposed();
-            _closed = true;
-
-            if (!_aborted)
+            finally
             {
-                await WriteFinalBatchAsync(cancellationToken).ConfigureAwait(false);
-                await WriteFilesCollectionDocumentAsync(cancellationToken).ConfigureAwait(false);
+                Dispose();
             }
-
-            base.Close();
         }
 
         public override void Flush()
@@ -252,6 +235,69 @@ public override async Task WriteAsync(byte[] buffer, int offset, int count, Canc
         }
 
         // private methods
+        private void CloseIfNotAlreadyClosed(CancellationToken cancellationToken)
+        {
+            if (!_closed)
+            {
+                try
+                {
+                    CloseImplementation(cancellationToken);
+                }
+                finally
+                {
+                    _closed = true;
+                }
+            }
+        }
+
+        private async Task CloseIfNotAlreadyClosedAsync(CancellationToken cancellationToken)
+        {
+            if (!_closed)
+            {
+                try
+                {
+                    await CloseImplementationAsync(cancellationToken).ConfigureAwait(false);
+                }
+                finally
+                {
+                    _closed = true;
+                }
+            }
+        }
+
+        private void CloseIfNotAlreadyClosedFromDispose(bool disposing)
+        {
+            if (disposing)
+            {
+                try
+                {
+                    CloseIfNotAlreadyClosed(CancellationToken.None);
+                }
+                catch
+                {
+                    // ignore any exceptions from CloseIfNotAlreadyClosed when called from Dispose
+                }
+            }
+        }
+
+        private void CloseImplementation(CancellationToken cancellationToken)
+        {
+            if (!_aborted)
+            {
+                WriteFinalBatch(cancellationToken);
+                WriteFilesCollectionDocument(cancellationToken);
+            }
+        }
+
+        private async Task CloseImplementationAsync(CancellationToken cancellationToken = default(CancellationToken))
+        {
+            if (!_aborted)
+            {
+                await WriteFinalBatchAsync(cancellationToken).ConfigureAwait(false);
+                await WriteFilesCollectionDocumentAsync(cancellationToken).ConfigureAwait(false);
+            }
+        }
+
         private BulkMixedWriteOperation CreateAbortOperation()
         {
             var chunksCollectionNamespace = _bucket.GetChunksCollectionNamespace();
@@ -308,6 +354,8 @@ private IEnumerable<BsonDocument> CreateWriteBatchChunkDocuments()
 
         protected override void Dispose(bool disposing)
         {
+            CloseIfNotAlreadyClosedFromDispose(disposing);
+
             if (!_disposed)
             {
                 _disposed = true;