From: Adam Sitnik <adam.sitnik@gmail.com>
Date: Wed, 31 Mar 2021 06:42:18 +0000 (+0200)
Subject: Dont cache file length when handle has been exposed (#50424)
X-Git-Tag: submit/tizen/20210909.063632~2321
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=4f819108ca81965bcac0d9670471f5c6a9140d13;p=platform%2Fupstream%2Fdotnet%2Fruntime.git

Dont cache file length when handle has been exposed (#50424)
---

diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/FileStreamConformanceTests.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/FileStreamConformanceTests.cs
index 271cb28..82b10a8 100644
--- a/src/libraries/System.IO.FileSystem/tests/FileStream/FileStreamConformanceTests.cs
+++ b/src/libraries/System.IO.FileSystem/tests/FileStream/FileStreamConformanceTests.cs
@@ -133,6 +133,34 @@ namespace System.IO.Tests
             byte[] allBytes = File.ReadAllBytes(filePath);
             Assert.Equal(writtenBytes.ToArray(), allBytes);
         }
+
+        [Theory]
+        [InlineData(FileAccess.Write)]
+        [InlineData(FileAccess.ReadWrite)] // FileAccess.Read does not allow for length manipulations
+        public async Task LengthIsNotCachedAfterHandleHasBeenExposed(FileAccess fileAccess)
+        {
+            using FileStream stream = (FileStream)await CreateStream(Array.Empty<byte>(), fileAccess);
+            using FileStream createdFromHandle = new FileStream(stream.SafeFileHandle, fileAccess);
+
+            Assert.Equal(0, stream.Length);
+            Assert.Equal(0, createdFromHandle.Length);
+
+            createdFromHandle.SetLength(1);
+            Assert.Equal(1, createdFromHandle.Length);
+            Assert.Equal(1, stream.Length);
+
+            createdFromHandle.SetLength(2);
+            Assert.Equal(2, createdFromHandle.Length);
+            Assert.Equal(2, stream.Length);
+
+            stream.SetLength(1);
+            Assert.Equal(1, stream.Length);
+            Assert.Equal(1, createdFromHandle.Length);
+
+            stream.SetLength(2);
+            Assert.Equal(2, stream.Length);
+            Assert.Equal(2, createdFromHandle.Length);
+        }
     }
 
     public class UnbufferedSyncFileStreamStandaloneConformanceTests : FileStreamStandaloneConformanceTests
diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/WindowsFileStreamStrategy.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/WindowsFileStreamStrategy.cs
index 8c33d75..d42494c 100644
--- a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/WindowsFileStreamStrategy.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/WindowsFileStreamStrategy.cs
@@ -4,7 +4,6 @@
 using System.Diagnostics;
 using System.Threading.Tasks;
 using Microsoft.Win32.SafeHandles;
-using System.Runtime.CompilerServices;
 
 namespace System.IO.Strategies
 {
@@ -28,6 +27,7 @@ namespace System.IO.Strategies
         protected long _filePosition;
         private long _appendStart; // When appending, prevent overwriting file.
         private long _length = -1; // When the file is locked for writes (_share <= FileShare.Read) cache file length in-memory, negative means that hasn't been fetched.
+        private bool _exposedHandle; // created from handle, or SafeFileHandle was used and the handle got exposed
 
         internal WindowsFileStreamStrategy(SafeFileHandle handle, FileAccess access, FileShare share)
         {
@@ -37,6 +37,7 @@ namespace System.IO.Strategies
             // but we can't as they're readonly.
             _access = access;
             _share = share;
+            _exposedHandle = true;
 
             // As the handle was passed in, we must set the handle field at the very end to
             // avoid the finalizer closing the handle when we throw errors.
@@ -77,15 +78,29 @@ namespace System.IO.Strategies
 
         // When the file is locked for writes we can cache file length in memory
         // and avoid subsequent native calls which are expensive.
-        public unsafe sealed override long Length => _share > FileShare.Read ?
-            FileStreamHelpers.GetFileLength(_fileHandle, _path) :
-            _length < 0 ? _length = FileStreamHelpers.GetFileLength(_fileHandle, _path) : _length;
+        public unsafe sealed override long Length
+        {
+            get
+            {
+                if (_share > FileShare.Read || _exposedHandle)
+                {
+                    return FileStreamHelpers.GetFileLength(_fileHandle, _path);
+                }
+
+                if (_length < 0)
+                {
+                    _length = FileStreamHelpers.GetFileLength(_fileHandle, _path);
+                }
+
+                return _length;
+            }
+        }
 
         protected void UpdateLengthOnChangePosition()
         {
             // Do not update the cached length if the file is not locked
             // or if the length hasn't been fetched.
-            if (_share > FileShare.Read || _length < 0)
+            if (_share > FileShare.Read || _length < 0 || _exposedHandle)
             {
                 Debug.Assert(_length < 0);
                 return;
@@ -120,6 +135,10 @@ namespace System.IO.Strategies
                     // in memory position is out-of-sync with the actual file position.
                     FileStreamHelpers.Seek(_fileHandle, _path, _filePosition, SeekOrigin.Begin);
                 }
+
+                _exposedHandle = true;
+                _length = -1; // invalidate cached length
+
                 return _fileHandle;
             }
         }