Skip to content

Commit

Permalink
Dont cache file length when handle has been exposed (#50424)
Browse files Browse the repository at this point in the history
  • Loading branch information
adamsitnik committed Mar 31, 2021
1 parent f181620 commit 4f81910
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,34 @@ public async Task NoDataIsLostWhenWritingToFile(ReadWriteMode mode)
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System.Diagnostics;
using System.Threading.Tasks;
using Microsoft.Win32.SafeHandles;
using System.Runtime.CompilerServices;

namespace System.IO.Strategies
{
Expand All @@ -28,6 +27,7 @@ internal abstract class WindowsFileStreamStrategy : FileStreamStrategy
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)
{
Expand All @@ -37,6 +37,7 @@ internal WindowsFileStreamStrategy(SafeFileHandle handle, FileAccess access, Fil
// 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.
Expand Down Expand Up @@ -77,15 +78,29 @@ internal WindowsFileStreamStrategy(string path, FileMode mode, FileAccess access

// 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;
Expand Down Expand Up @@ -120,6 +135,10 @@ internal sealed override SafeFileHandle SafeFileHandle
// 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;
}
}
Expand Down

0 comments on commit 4f81910

Please sign in to comment.