Skip to content

Commit d4546ac

Browse files
Tyrie VellaCopilot
andcommitted
Address PR feedback: proper exit codes and error classification for mount lock
- Add TryAcquireLock(out Exception) to FileBasedLock so callers can pattern-match on exception type to distinguish lock contention (IOException/sharing violation) from permission or I/O errors. - Replace bare return with FailMountAndExit() when lock acquisition fails, so the process exits with a non-zero exit code. - Add ReturnCode.MountAlreadyRunning (8) for the lock contention case, keeping GenericError (3) for unexpected lock failures. - Add FailMountAndExit(ReturnCode, ...) overload to support per-call exit codes while preserving existing callers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 0606c95 commit d4546ac

5 files changed

Lines changed: 40 additions & 8 deletions

File tree

GVFS/GVFS.Common/FileBasedLock.cs

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,23 @@ public FileBasedLock(
2020
protected string LockPath { get; }
2121
protected ITracer Tracer { get; }
2222

23-
public abstract bool TryAcquireLock();
23+
public bool TryAcquireLock()
24+
{
25+
return this.TryAcquireLock(out _);
26+
}
27+
28+
/// <summary>
29+
/// Attempts to acquire the lock, providing the exception that prevented acquisition.
30+
/// </summary>
31+
/// <param name="lockException">
32+
/// When the method returns false, contains the exception that prevented lock acquisition.
33+
/// Callers can pattern-match on the exception type to distinguish lock contention
34+
/// (e.g. <see cref="System.IO.IOException"/> with a sharing violation HResult) from
35+
/// permission errors (<see cref="UnauthorizedAccessException"/>) or other failures.
36+
/// Null when the method returns true.
37+
/// </param>
38+
/// <returns>True if the lock was acquired, false otherwise.</returns>
39+
public abstract bool TryAcquireLock(out Exception lockException);
2440

2541
public abstract void Dispose();
2642
}

GVFS/GVFS.Common/ReturnCode.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,6 @@ public enum ReturnCode
1010
NullRequestData = 5,
1111
UnableToRegisterForOfflineIO = 6,
1212
DehydrateFolderFailures = 7,
13+
MountAlreadyRunning = 8,
1314
}
1415
}

GVFS/GVFS.Mount/InProcessMount.cs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,14 @@ public void Mount(EventLevel verbosity, Keywords keywords)
9191
this.tracer,
9292
mountLockPath))
9393
{
94-
if (!mountLock.TryAcquireLock())
94+
if (!mountLock.TryAcquireLock(out Exception lockException))
9595
{
96-
this.tracer.RelatedInfo("Mount: Another mount process is already running. Exiting.");
97-
return;
96+
if (lockException is IOException)
97+
{
98+
this.FailMountAndExit(ReturnCode.MountAlreadyRunning, "Mount: Another mount process is already running.");
99+
}
100+
101+
this.FailMountAndExit("Mount: Failed to acquire mount lock: {0}", lockException.Message);
98102
}
99103

100104
this.MountWithLockAcquired(verbosity, keywords);
@@ -314,6 +318,11 @@ private NamedPipeServer StartNamedPipe()
314318
}
315319

316320
private void FailMountAndExit(string error, params object[] args)
321+
{
322+
this.FailMountAndExit(ReturnCode.GenericError, error, args);
323+
}
324+
325+
private void FailMountAndExit(ReturnCode returnCode, string error, params object[] args)
317326
{
318327
this.currentState = MountState.MountFailed;
319328

@@ -330,7 +339,7 @@ private void FailMountAndExit(string error, params object[] args)
330339
this.fileSystemCallbacks = null;
331340
}
332341

333-
Environment.Exit((int)ReturnCode.GenericError);
342+
Environment.Exit((int)returnCode);
334343
}
335344

336345
private T CreateOrReportAndExit<T>(Func<T> factory, string reportMessage)

GVFS/GVFS.Platform.Windows/WindowsFileBasedLock.cs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,9 @@ public WindowsFileBasedLock(
3636
{
3737
}
3838

39-
public override bool TryAcquireLock()
39+
public override bool TryAcquireLock(out Exception lockException)
4040
{
41+
lockException = null;
4142
try
4243
{
4344
lock (this.deleteOnCloseStreamLock)
@@ -63,13 +64,14 @@ public override bool TryAcquireLock()
6364
catch (IOException e)
6465
{
6566
// HResultErrorFileExists is expected when the lock file exists
66-
// HResultErrorSharingViolation is expected when the lock file exists andanother GVFS process has acquired the lock file
67+
// HResultErrorSharingViolation is expected when the lock file exists and another GVFS process has acquired the lock file
6768
if (e.HResult != HResultErrorFileExists && e.HResult != HResultErrorSharingViolation)
6869
{
6970
EventMetadata metadata = this.CreateLockMetadata(e);
7071
this.Tracer.RelatedWarning(metadata, $"{nameof(this.TryAcquireLock)}: IOException caught while trying to acquire lock");
7172
}
7273

74+
lockException = e;
7375
this.DisposeStream();
7476
return false;
7577
}
@@ -78,6 +80,7 @@ public override bool TryAcquireLock()
7880
EventMetadata metadata = this.CreateLockMetadata(e);
7981
this.Tracer.RelatedWarning(metadata, $"{nameof(this.TryAcquireLock)}: UnauthorizedAccessException caught while trying to acquire lock");
8082

83+
lockException = e;
8184
this.DisposeStream();
8285
return false;
8386
}
@@ -86,6 +89,7 @@ public override bool TryAcquireLock()
8689
EventMetadata metadata = this.CreateLockMetadata(e);
8790
this.Tracer.RelatedWarning(metadata, $"{nameof(this.TryAcquireLock)}: Win32Exception caught while trying to acquire lock");
8891

92+
lockException = e;
8993
this.DisposeStream();
9094
return false;
9195
}

GVFS/GVFS.UnitTests/Mock/Common/MockFileBasedLock.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using GVFS.Common;
22
using GVFS.Common.FileSystem;
33
using GVFS.Common.Tracing;
4+
using System;
45

56
namespace GVFS.UnitTests.Mock.Common
67
{
@@ -14,8 +15,9 @@ public MockFileBasedLock(
1415
{
1516
}
1617

17-
public override bool TryAcquireLock()
18+
public override bool TryAcquireLock(out Exception lockException)
1819
{
20+
lockException = null;
1921
return true;
2022
}
2123

0 commit comments

Comments
 (0)