[release/6.0] SafeFileHandle.Unix: don't DeleteOnClose when lock is not successful.

src/libraries/System.IO.FileSystem/tests/FileStream/SafeFileHandle.cs

@@ -78,6 +78,18 @@ public async Task ThrowWhenHandlePositionIsChanged_async()
             await ThrowWhenHandlePositionIsChanged(useAsync: true);
         }
 
+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsFileLockingEnabled))]
+        public void DeleteOnClose_FailedShareDoesNotDeleteFile()
+        {
+            string fileName = GetTestFilePath();
+
+            using SafeFileHandle handle = File.OpenHandle(fileName, FileMode.OpenOrCreate, FileAccess.Write, FileShare.None);
+
+            Assert.Throws<IOException>(() => File.OpenHandle(fileName, FileMode.Open, FileAccess.Write, FileShare.None, FileOptions.DeleteOnClose));
+
+            Assert.True(File.Exists(fileName));
+        }
+
         private async Task ThrowWhenHandlePositionIsChanged(bool useAsync)
         {
             string fileName = GetTestFilePath();

src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs

@@ -121,17 +121,6 @@ private static bool DirectoryExists(string fullPath)
 
         protected override bool ReleaseHandle()
         {
-            // When the SafeFileHandle was opened, we likely issued an flock on the created descriptor in order to add
-            // 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 (_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.
             // (Unix doesn't directly support DeleteOnClose, so we mimic it here.)
             if (_deleteOnClose)
@@ -143,6 +132,17 @@ protected override bool ReleaseHandle()
                 Interop.Sys.Unlink(_path); // ignore errors; it's valid that the path may no longer exist
             }
 
+            // When the SafeFileHandle was opened, we likely issued an flock on the created descriptor in order to add
+            // 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 (_isLocked)
+            {
+                Interop.Sys.FLock(handle, Interop.Sys.LockOperations.LOCK_UN); // ignore any errors
+                _isLocked = 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
             // be in use elsewhere in the process.  Instead, we simply check whether the call was successful.
@@ -274,7 +274,6 @@ private static Interop.Sys.OpenFlags PreOpenConfigurationFromOptions(FileMode mo
         private void Init(string path, FileMode mode, FileAccess access, FileShare share, FileOptions options, long preallocationSize)
         {
             IsAsync = (options & FileOptions.Asynchronous) != 0;
-            _deleteOnClose = (options & FileOptions.DeleteOnClose) != 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,
@@ -293,6 +292,10 @@ private void Init(string path, FileMode mode, FileAccess access, FileShare share
                 }
             }
 
+            // Enable DeleteOnClose when we've succesfully locked the file.
+            // On Windows, the locking happens atomically as part of opening the file.
+            _deleteOnClose = (options & FileOptions.DeleteOnClose) != 0;
+
             // These provide hints around how the file will be accessed.  Specifying both RandomAccess
             // and Sequential together doesn't make sense as they are two competing options on the same spectrum,
             // so if both are specified, we prefer RandomAccess (behavior on Windows is unspecified if both are provided).