Skip to content

Add Mac specific FileBasedLock implementation #213

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
wilbaker merged 8 commits into from
Aug 24, 2018
Merged

Add Mac specific FileBasedLock implementation #213

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2ab1b2b
Cleanup FileBasedLock and make it Windows specific
wilbaker Aug 23, 2018
ddc5f85
Rename FileBasedLock.cs to WindowsFileBasedLock.cs and move to Window…
wilbaker Aug 23, 2018
7b17d52
More cleanup, switch from interface to abstract class
wilbaker Aug 23, 2018
3027507
Initial MacFileBasedLock implementation
wilbaker Aug 23, 2018
eca1bd2
Fix StyleCop errors
wilbaker Aug 23, 2018
540aaf5
Cleanup and changes for PR feedback
wilbaker Aug 24, 2018
394b337
Fix StyleCop errors
wilbaker Aug 24, 2018
467c653
Remove code for writing signature
wilbaker Aug 24, 2018
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
293 changes: 10 additions & 283 deletions GVFS/GVFS.Common/FileBasedLock.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,300 +1,27 @@
using GVFS.Common.FileSystem;
using GVFS.Common.Tracing;
using System;
using System.ComponentModel;
using System.IO;
using System.Text;

namespace GVFS.Common
{
public class FileBasedLock : IDisposable
public abstract class FileBasedLock : IDisposable
{
private const int HResultErrorSharingViolation = -2147024864; // -2147024864 = 0x80070020 = ERROR_SHARING_VIOLATION
private const int HResultErrorFileExists = -2147024816; // -2147024816 = 0x80070050 = ERROR_FILE_EXISTS
private const int DefaultStreamWriterBufferSize = 1024; // Copied from: http://referencesource.microsoft.com/#mscorlib/system/io/streamwriter.cs,5516ce201dc06b5f
private const long InvalidFileLength = -1;
private const string EtwArea = nameof(FileBasedLock);
private static readonly Encoding UTF8NoBOM = new UTF8Encoding(false, true); // Default encoding used by StreamWriter

private readonly object deleteOnCloseStreamLock = new object();
private readonly PhysicalFileSystem fileSystem;
private readonly string lockPath;
private ITracer tracer;
private Stream deleteOnCloseStream;
private bool overwriteExistingLock;

/// <summary>
/// FileBasedLock constructor
/// </summary>
/// <param name="lockPath">Path to lock file</param>
/// <param name="signature">Text to write in lock file</param>
/// <param name="overwriteExistingLock">
/// If true, FileBasedLock will attempt to overwrite an existing lock file (if one exists on disk) when
/// acquiring the lock file.
/// </param>
/// <remarks>
/// GVFS keeps an exclusive write handle open to lock files that it creates with FileBasedLock. This means that
/// FileBasedLock still ensures exclusivity when "overwriteExistingLock" is true if the lock file is only used for
/// coordination between multiple GVFS processes.
/// </remarks>
public FileBasedLock(
PhysicalFileSystem fileSystem,
ITracer tracer,
string lockPath,
string signature,
bool overwriteExistingLock)
{
this.fileSystem = fileSystem;
this.tracer = tracer;
this.lockPath = lockPath;
this.Signature = signature;
this.overwriteExistingLock = overwriteExistingLock;
}

public string Signature { get; private set; }

public bool TryAcquireLockAndDeleteOnClose()
{
try
{
lock (this.deleteOnCloseStreamLock)
{
if (this.IsOpen())
{
return true;
}

this.fileSystem.CreateDirectory(Path.GetDirectoryName(this.lockPath));

this.deleteOnCloseStream = this.fileSystem.OpenFileStream(
this.lockPath,
this.overwriteExistingLock ? FileMode.Create : FileMode.CreateNew,
FileAccess.ReadWrite,
FileShare.Read,
FileOptions.DeleteOnClose,
callFlushFileBuffers: false);

// Pass in true for leaveOpen to ensure that lockStream stays open
using (StreamWriter writer = new StreamWriter(
this.deleteOnCloseStream,
UTF8NoBOM,
DefaultStreamWriterBufferSize,
leaveOpen: true))
{
this.WriteSignatureAndMessage(writer, message: null);
}

return true;
}
}
catch (IOException e)
{
// HResultErrorFileExists is expected when the lock file exists
// HResultErrorSharingViolation is expected when the lock file exists and we're in this.overwriteExistingLock mode, as
// another GVFS process has likely acquired the lock file
if (e.HResult != HResultErrorFileExists &&
!(this.overwriteExistingLock && e.HResult == HResultErrorSharingViolation))
{
EventMetadata metadata = this.CreateLockMetadata(e);
this.tracer.RelatedWarning(metadata, "TryAcquireLockAndDeleteOnClose: IOException caught while trying to acquire lock");
}

this.DisposeStream();
return false;
}
catch (UnauthorizedAccessException e)
{
EventMetadata metadata = this.CreateLockMetadata(e);
this.tracer.RelatedWarning(metadata, "TryAcquireLockAndDeleteOnClose: UnauthorizedAccessException caught while trying to acquire lock");

this.DisposeStream();
return false;
}
catch (Win32Exception e)
{
EventMetadata metadata = this.CreateLockMetadata(e);
this.tracer.RelatedWarning(metadata, "TryAcquireLockAndDeleteOnClose: Win32Exception caught while trying to acquire lock");

this.DisposeStream();
return false;
}
catch (Exception e)
{
EventMetadata metadata = this.CreateLockMetadata(e);
this.tracer.RelatedError(metadata, "TryAcquireLockAndDeleteOnClose: Unhandled exception caught while trying to acquire lock");

this.DisposeStream();
throw;
}
}

public bool TryReleaseLock()
{
if (this.DisposeStream())
{
return true;
}

LockData lockData = this.GetLockDataFromDisk();
if (lockData == null || lockData.Signature != this.Signature)
{
if (lockData == null)
{
throw new LockFileDoesNotExistException(this.lockPath);
}

throw new LockSignatureDoesNotMatchException(this.lockPath, this.Signature, lockData.Signature);
}

try
{
this.fileSystem.DeleteFile(this.lockPath);
}
catch (IOException e)
{
EventMetadata metadata = this.CreateLockMetadata(e);
this.tracer.RelatedWarning(metadata, "TryReleaseLock: IOException caught while trying to release lock");

return false;
}

return true;
}

public bool IsOpen()
{
return this.deleteOnCloseStream != null;
}

public void Dispose()
{
this.Dispose(true);
GC.SuppressFinalize(this);
}

protected void Dispose(bool disposing)
{
if (disposing)
{
this.DisposeStream();
}
}

private LockData GetLockDataFromDisk()
string lockPath)
{
if (this.LockFileExists())
{
string existingSignature;
string existingMessage;
this.ReadLockFile(out existingSignature, out existingMessage);
return new LockData(existingSignature, existingMessage);
}

return null;
}

private void ReadLockFile(out string existingSignature, out string lockerMessage)
{
using (Stream fs = this.fileSystem.OpenFileStream(this.lockPath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite | FileShare.Delete, callFlushFileBuffers: false))
using (StreamReader reader = new StreamReader(fs, UTF8NoBOM))
{
existingSignature = reader.ReadLine();
lockerMessage = reader.ReadLine();
}

existingSignature = existingSignature ?? string.Empty;
lockerMessage = lockerMessage ?? string.Empty;
}

private bool LockFileExists()
{
return this.fileSystem.FileExists(this.lockPath);
}

private void WriteSignatureAndMessage(StreamWriter writer, string message)
{
writer.WriteLine(this.Signature);
if (message != null)
{
writer.Write(message);
}
}

private EventMetadata CreateLockMetadata()
{
EventMetadata metadata = new EventMetadata();
metadata.Add("Area", EtwArea);
metadata.Add("LockPath", this.lockPath);
metadata.Add("Signature", this.Signature);

return metadata;
}

private EventMetadata CreateLockMetadata(Exception exception = null)
{
EventMetadata metadata = this.CreateLockMetadata();
if (exception != null)
{
metadata.Add("Exception", exception.ToString());
}

return metadata;
this.FileSystem = fileSystem;
this.Tracer = tracer;
this.LockPath = lockPath;
}

private bool DisposeStream()
{
lock (this.deleteOnCloseStreamLock)
{
if (this.deleteOnCloseStream != null)
{
this.deleteOnCloseStream.Dispose();
this.deleteOnCloseStream = null;
return true;
}
}
protected PhysicalFileSystem FileSystem { get; }
protected string LockPath { get; }
protected ITracer Tracer { get; }

return false;
}
public abstract bool TryAcquireLock();

public class LockException : Exception
{
public LockException(string messageFormat, params string[] args)
: base(string.Format(messageFormat, args))
{
}
}

public class LockFileDoesNotExistException : LockException
{
public LockFileDoesNotExistException(string lockPath)
: base("Lock file {0} does not exist", lockPath)
{
}
}

public class LockSignatureDoesNotMatchException : LockException
{
public LockSignatureDoesNotMatchException(string lockPath, string expectedSignature, string actualSignature)
: base(
"Lock file {0} does not contain expected signature '{1}' (existing signature: '{2}')",
lockPath,
expectedSignature,
actualSignature)
{
}
}

public class LockData
{
public LockData(string signature, string message)
{
this.Signature = signature;
this.Message = message;
}

public string Signature { get; }

public string Message { get; }
}
public abstract void Dispose();
Copy link
Copy Markdown
Contributor

@sanoursa sanoursa Aug 24, 2018

Choose a reason for hiding this comment

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

I don't think you need this method definition here. The interface should already be enough to require subclasses to implement this right?

Copy link
Copy Markdown
Member Author

@wilbaker wilbaker Aug 24, 2018

Choose a reason for hiding this comment

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

Yeah, this might be leftover from when I was moving things around, let me try removing it

Copy link
Copy Markdown
Member Author

@wilbaker wilbaker Aug 24, 2018

Choose a reason for hiding this comment

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

It ends up the abstract Dispose is required, without it we get the error:

FileBasedLock.cs(7,43): error CS0535: 'FileBasedLock' does not implement interface member 'IDisposable.Dispose()' [/Users/wilbaker/Repos/VFSForGit/src/GVFS/GVFS.Common/GVFS.Common.csproj]

Copy link
Copy Markdown
Contributor

@sanoursa sanoursa Aug 24, 2018

Choose a reason for hiding this comment

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

Yea you're right. I must be spending too much time in C++ these days :-)

}
}
5 changes: 5 additions & 0 deletions GVFS/GVFS.Common/GVFSPlatform.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ public static void Register(GVFSPlatform platform)

public abstract bool IsGitStatusCacheSupported();

public abstract FileBasedLock CreateFileBasedLock(
PhysicalFileSystem fileSystem,
ITracer tracer,
string lockPath);

public bool TryGetNormalizedPathRoot(string path, out string pathRoot, out string errorMessage)
{
pathRoot = null;
Expand Down
8 changes: 3 additions & 5 deletions GVFS/GVFS.Common/LocalCacheResolver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,10 @@ public bool TryGetLocalCacheKeyFromLocalConfigOrRemoteCacheServers(
string lockPath = Path.Combine(localCacheRoot, MappingFile + ".lock");
this.fileSystem.CreateDirectory(localCacheRoot);

using (FileBasedLock mappingLock = new FileBasedLock(
using (FileBasedLock mappingLock = GVFSPlatform.Instance.CreateFileBasedLock(
this.fileSystem,
tracer,
lockPath,
this.enlistment.EnlistmentRoot,
overwriteExistingLock: true))
lockPath))
{
if (!this.TryAcquireLockWithRetries(tracer, mappingLock))
{
Expand Down Expand Up @@ -279,7 +277,7 @@ private bool TryAcquireLockWithRetries(ITracer tracer, FileBasedLock mappingLock

for (int i = 0; i < NumRetries; ++i)
{
if (mappingLock.TryAcquireLockAndDeleteOnClose())
if (mappingLock.TryAcquireLock())
{
return true;
}
Expand Down
8 changes: 3 additions & 5 deletions GVFS/GVFS.Common/Prefetch/CommitPrefetcher.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,10 @@ public static bool TryPrefetchCommitsAndTrees(
out string error)
{
List<string> packIndexes;
using (FileBasedLock prefetchLock = new FileBasedLock(
using (FileBasedLock prefetchLock = GVFSPlatform.Instance.CreateFileBasedLock(
fileSystem,
tracer,
Path.Combine(enlistment.GitPackRoot, PrefetchCommitsAndTreesLock),
enlistment.EnlistmentRoot,
overwriteExistingLock: true))
Path.Combine(enlistment.GitPackRoot, PrefetchCommitsAndTreesLock)))
{
WaitUntilLockIsAcquired(tracer, prefetchLock);
long maxGoodTimeStamp;
Expand Down Expand Up @@ -213,7 +211,7 @@ private static bool TrySchedulePostFetchJob(ITracer tracer, string namedPipeName
private static void WaitUntilLockIsAcquired(ITracer tracer, FileBasedLock fileBasedLock)
{
int attempt = 0;
while (!fileBasedLock.TryAcquireLockAndDeleteOnClose())
while (!fileBasedLock.TryAcquireLock())
{
Thread.Sleep(LockWaitTimeMs);
++attempt;
Expand Down
Loading