Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
don't try to aquire an advisory shared lock on Unix as it's not prope…
…rly supported by some file systems like NFS and CIFS
  • Loading branch information
adamsitnik committed Jul 2, 2021
commit fd341bf4f022b25f802ebeaa75fb84b27ee42d63
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,8 @@ private static Version GetICUVersion()

public static bool IsNet5CompatFileStreamEnabled => _net5CompatFileStream.Value;

public static bool IsFileLockingSupported => IsWindows || IsNet5CompatFileStreamEnabled;

private static bool GetIsInContainer()
{
if (IsWindows)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ public void Overwrite()
Assert.Equal(overwriteBytes, File.ReadAllBytes(path));
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/40065", TestPlatforms.Browser)]
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsFileLockingSupported))]
public void OpenFile_ThrowsIOException()
{
string path = GetTestFilePath();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,8 @@ public async Task OverwriteAsync()
Assert.Equal(overwriteBytes, await File.ReadAllBytesAsync(path));
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/40065", TestPlatforms.Browser)]
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsFileLockingSupported))]
public async Task OpenFile_ThrowsIOExceptionAsync()
{
string path = GetTestFilePath();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ public virtual void Overwrite()
Assert.Equal(overwriteLines, Read(path));
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/40065", TestPlatforms.Browser)]
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsFileLockingSupported))]
public void OpenFile_ThrowsIOException()
{
string path = GetTestFilePath();
Expand Down Expand Up @@ -267,8 +267,8 @@ public virtual void Overwrite()
Assert.Equal(overwriteLines, Read(path));
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/40065", TestPlatforms.Browser)]
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsFileLockingSupported))]
public void OpenFile_ThrowsIOException()
{
string path = GetTestFilePath();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ public virtual async Task OverwriteAsync()
Assert.Equal(overwriteLines, await ReadAsync(path));
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/40065", TestPlatforms.Browser)]
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsFileLockingSupported))]
public async Task OpenFile_ThrowsIOExceptionAsync()
{
string path = GetTestFilePath();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ public virtual void Overwrite()
Assert.Equal(overwriteLines, Read(path));
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/40065", TestPlatforms.Browser)]
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsFileLockingSupported))]
public void OpenFile_ThrowsIOException()
{
string path = GetTestFilePath();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ public virtual async Task OverwriteAsync()
Assert.Equal(overwriteLines, await ReadAsync(path));
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/40065", TestPlatforms.Browser)]
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsFileLockingSupported))]
public async Task OpenFile_ThrowsIOExceptionAsync()
{
string path = GetTestFilePath();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ public void FileShareWriteExisting()
}
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/40065", TestPlatforms.Browser)]
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsFileLockingSupported))]
public void FileShareWithoutWriteThrows()
{
string fileName = GetTestFilePath();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ public sealed class SafeFileHandle : SafeHandleZeroOrMinusOneIsInvalid
{
// not using bool? as it's not thread safe
private volatile NullableBool _canSeek = NullableBool.Undefined;
private bool _releaseLock;

public SafeFileHandle() : this(ownsHandle: true)
{
Expand Down Expand Up @@ -120,7 +121,11 @@ protected override bool ReleaseHandle()
// which could prevent subsequent usage of the file until this process dies. To avoid that, we proactively
// try to release the lock before we close the handle. (If it's not locked, there's no behavioral
// problem trying to unlock it.)
Interop.Sys.FLock(handle, Interop.Sys.LockOperations.LOCK_UN); // ignore any errors
if (_releaseLock)
{
Interop.Sys.FLock(handle, Interop.Sys.LockOperations.LOCK_UN); // ignore any errors
_releaseLock = false;
}

// Close the descriptor. Although close is documented to potentially fail with EINTR, we never want
// to retry, as the descriptor could actually have been closed, been subsequently reassigned, and
Expand Down Expand Up @@ -240,20 +245,27 @@ private void Init(string path, FileMode mode, FileAccess access, FileShare share
{
IsAsync = (options & FileOptions.Asynchronous) != 0;

// Lock the file if requested via FileShare. This is only advisory locking. FileShare.None implies an exclusive
// lock on the file and all other modes use a shared lock. While this is not as granular as Windows, not mandatory,
// and not atomic with file opening, it's better than nothing.
Interop.Sys.LockOperations lockOperation = (share == FileShare.None) ? Interop.Sys.LockOperations.LOCK_EX : Interop.Sys.LockOperations.LOCK_SH;
if (Interop.Sys.FLock(this, lockOperation | Interop.Sys.LockOperations.LOCK_NB) < 0)
if (FileStreamHelpers.UseNet5CompatStrategy || share == FileShare.None)
{
// The only error we care about is EWOULDBLOCK, which indicates that the file is currently locked by someone
// else and we would block trying to access it. Other errors, such as ENOTSUP (locking isn't supported) or
// EACCES (the file system doesn't allow us to lock), will only hamper FileStream's usage without providing value,
// given again that this is only advisory / best-effort.
Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo();
if (errorInfo.Error == Interop.Error.EWOULDBLOCK)
// Lock the file if requested via FileShare. This is only advisory locking. FileShare.None implies an exclusive
// lock on the file and all other modes use a shared lock. While this is not as granular as Windows, not mandatory,
// and not atomic with file opening, it's better than nothing.
Interop.Sys.LockOperations lockOperation = (share == FileShare.None) ? Interop.Sys.LockOperations.LOCK_EX : Interop.Sys.LockOperations.LOCK_SH;
if (Interop.Sys.FLock(this, lockOperation | Interop.Sys.LockOperations.LOCK_NB) < 0)
{
// The only error we care about is EWOULDBLOCK, which indicates that the file is currently locked by someone
// else and we would block trying to access it. Other errors, such as ENOTSUP (locking isn't supported) or
// EACCES (the file system doesn't allow us to lock), will only hamper FileStream's usage without providing value,
// given again that this is only advisory / best-effort.
Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo();
if (errorInfo.Error == Interop.Error.EWOULDBLOCK)
{
throw Interop.GetExceptionForIoErrno(errorInfo, path, isDirectory: false);
}
}
else
{
throw Interop.GetExceptionForIoErrno(errorInfo, path, isDirectory: false);
_releaseLock = true;
}
}

Expand Down