Skip to content

use fstatfs to detect whether current file system supports shared locks for files opened for writing #55256

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
adamsitnik merged 16 commits into from
Jul 9, 2021
Merged

use fstatfs to detect whether current file system supports shared locks for files opened for writing #55256

Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
71d131a
use fstatfs to detect whether current file system supports shared loc…
adamsitnik Jul 7, 2021
7980cc8
improve comments
adamsitnik Jul 7, 2021
97c1544
cast in explicit way
adamsitnik Jul 7, 2021
5d95af8
hopefully fix the BSD build(s)
adamsitnik Jul 7, 2021
d64374f
fix OSX build
adamsitnik Jul 7, 2021
c687e6a
assume we should not acquire the lock if we don't know the File System
adamsitnik Jul 7, 2021
9c481ed
try to fix the maccatalyst build
adamsitnik Jul 7, 2021
1c890ec
always use fstatfs as it's available on every platform and can get us…
adamsitnik Jul 8, 2021
04eced6
handle SMB2_SUPER_MAGIC as well (#53182)
adamsitnik Jul 8, 2021
79b6def
introduce a flag that allows to disable file locking on Unix
adamsitnik Jul 8, 2021
ba6d027
Emscripten does not support file locking, so there is no need to chec…
adamsitnik Jul 8, 2021
731e65c
some polishing
adamsitnik Jul 8, 2021
d31f6e1
Merge remote-tracking branch 'upstream/main' into superMagic
adamsitnik Jul 8, 2021
c752b93
reuse the UnixFileSystemTypes enum, move it to a separate file so Cor…
adamsitnik Jul 9, 2021
7443a5e
add SMB2_SUPER_MAGIC to the enum
adamsitnik Jul 9, 2021
1e3878c
don't try to unlock the file if it has not been locked
adamsitnik Jul 9, 2021
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
don't try to unlock the file if it has not been locked
  • Loading branch information
@adamsitnik
adamsitnik committed Jul 9, 2021
commit 1e3878c005b95c4638cd5b095d329d0deca07ed4
13 changes: 7 additions & 6 deletions src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ public sealed partial class SafeFileHandle : SafeHandleZeroOrMinusOneIsInvalid
// not using bool? as it's not thread safe
private volatile NullableBool _canSeek = NullableBool.Undefined;
private bool _deleteOnClose;
private bool _isLocked;

public SafeFileHandle() : this(ownsHandle: true)
{
Expand Down Expand Up @@ -124,11 +125,11 @@ protected override bool ReleaseHandle()
// an advisory lock. This lock should be removed via closing the file descriptor, but close can be
// interrupted, and we don't retry closes. As such, we could end up leaving the file locked,
// 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.)
if (!DisableFileLocking)
// try to release the lock before we close the handle.
if (_isLocked)
{
Interop.Sys.FLock(handle, Interop.Sys.LockOperations.LOCK_UN); // ignore any errors
_isLocked = false;
}

// If DeleteOnClose was requested when constructed, delete the file now.
Expand Down Expand Up @@ -265,7 +266,7 @@ private void Init(string path, FileMode mode, FileAccess access, FileShare share
// 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 (CanLockTheFile(lockOperation, access) && Interop.Sys.FLock(this, lockOperation | Interop.Sys.LockOperations.LOCK_NB) < 0)
if (CanLockTheFile(lockOperation, access) && !(_isLocked = Interop.Sys.FLock(this, lockOperation | Interop.Sys.LockOperations.LOCK_NB) >= 0))
{
Comment on lines +269 to 270
Copy link
Member

@jozkee jozkee Jul 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be more clear.

Suggested change
if (CanLockTheFile(lockOperation, access) && !(_isLocked = Interop.Sys.FLock(this, lockOperation | Interop.Sys.LockOperations.LOCK_NB) >= 0))
{
if (CanLockTheFile(lockOperation, access) && Interop.Sys.FLock(this, lockOperation | Interop.Sys.LockOperations.LOCK_NB) < 0))
{
_isLocked = true;

// 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
Expand Down Expand Up @@ -350,9 +351,9 @@ private bool CanLockTheFile(Interop.Sys.LockOperations lockOperation, FileAccess

switch (unixFileSystemType)
{
case Interop.Sys.UnixFileSystemTypes.nfs:
case Interop.Sys.UnixFileSystemTypes.nfs: // #44546
case Interop.Sys.UnixFileSystemTypes.smb:
case Interop.Sys.UnixFileSystemTypes.smb2: // (#53182)
case Interop.Sys.UnixFileSystemTypes.smb2: // #53182
case Interop.Sys.UnixFileSystemTypes.cifs:
return false; // LOCK_SH is not OK when writing to NFS, CIFS or SMB
default:
Expand Down