Skip to content

Avoid GVFS command locks for read-only Git operations #2138

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

Open
Tyrie Vella (tyrielv) wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Avoid GVFS command locks for read-only Git operations #2138

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
27 changes: 26 additions & 1 deletion GVFS/GVFS.FunctionalTests.LockHolder/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,36 @@
using System.CommandLine;
using System;
using System.CommandLine;
using System.Threading;

namespace GVFS.FunctionalTests.LockHolder
{
public class Program
{
public static void Main(string[] args)
{
if (args.Length > 0 && args[0] == "--wait-for-event")
{
string eventName = Environment.GetEnvironmentVariable("GVFS_TEST_HOOK_WAIT_EVENT_NAME");
if (string.IsNullOrEmpty(eventName))
{
Console.Error.WriteLine("The test did not supply the hook event name.");
Environment.Exit(1);
}

using (EventWaitHandle ready = EventWaitHandle.OpenExisting(eventName + ".Ready"))
using (EventWaitHandle release = EventWaitHandle.OpenExisting(eventName + ".Release"))
{
ready.Set();
if (!release.WaitOne(TimeSpan.FromSeconds(30)))
{
Console.Error.WriteLine("Timed out before the test released the command hook.");
Environment.Exit(1);
}
}

return;
}

RootCommand rootCommand = AcquireGVFSLockVerb.BuildRootCommand();
rootCommand.Parse(args).Invoke();
}
Expand Down
81 changes: 75 additions & 6 deletions GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/GitReadAndGitLockTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using GVFS.Tests.Should;
using NUnit.Framework;
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Threading;
Expand Down Expand Up @@ -48,14 +49,82 @@ public void GitBranch()
"remotes/origin/" + Properties.Settings.Default.Commitish);
}

[TestCase, Order(4)]
public void GitCommandWaitsWhileAnotherIsRunning()
[TestCase("status")]
[TestCase("for-each-repo")]
[Order(4)]
public void GitCommandWaitsWhileAnotherIsRunning(string command)
{
int pid;
GitHelpers.AcquireGVFSLock(this.Enlistment, out pid, resetTimeout: 3000);
string emptyConfigPath = null;
Dictionary<string, string> environmentVariables = null;
try
{
if (command == "for-each-repo")
{
emptyConfigPath = Path.Combine(this.Enlistment.EnlistmentRoot, "empty-git-config-" + Guid.NewGuid().ToString("N"));
File.WriteAllText(emptyConfigPath, string.Empty);
environmentVariables = new Dictionary<string, string>
{
{ "GIT_CONFIG_GLOBAL", emptyConfigPath },
{ "GIT_CONFIG_NOSYSTEM", "1" },
{ "GIT_CONFIG_COUNT", "0" },
{ "GIT_CONFIG_PARAMETERS", string.Empty },
};

ProcessResult alias = GitProcess.InvokeProcess(
this.Enlistment.RepoRoot,
"config --get alias.for-each-repo",
environmentVariables);
alias.ExitCode.ShouldEqual(1);
alias.Output.ShouldBeEmpty();

command = "for-each-repo --config=gvfs.test-lock-policy-repos-" + Guid.NewGuid().ToString("N");
}

ProcessResult statusWait = GitHelpers.InvokeGitAgainstGVFSRepo(this.Enlistment.RepoRoot, "status", removeWaitingMessages: false);
statusWait.Errors.ShouldContain(ExpectedStatusWaitingText);
int pid;
GitHelpers.AcquireGVFSLock(this.Enlistment, out pid, resetTimeout: 3000);

ProcessResult result = GitHelpers.InvokeGitAgainstGVFSRepo(
this.Enlistment.RepoRoot,
command,
environmentVariables,
removeWaitingMessages: false);
result.ExitCode.ShouldEqual(0);
result.Errors.ShouldContain(ExpectedStatusWaitingText);
}
finally
{
if (emptyConfigPath != null)
{
File.Delete(emptyConfigPath);
}
}
}

[TestCase("worktree list")]
[TestCase("worktree list --porcelain")]
[TestCase("worktree list --porcelain -z")]
[TestCase("annotate HEAD -- Readme.md")]
[TestCase("check-ref-format --branch topic")]
[TestCase("cherry HEAD HEAD")]
[TestCase("count-objects -vH")]
[TestCase("ls-remote --get-url origin")]
[TestCase("range-diff HEAD~1..HEAD HEAD~1..HEAD")]
[TestCase("shortlog -sn HEAD")]
[TestCase("show-branch --list")]
public void ReadOnlyCommandDoesNotWaitForLock(string command)
{
GitHelpers.RunWithGVFSLock(
this.Enlistment,
() =>
{
ProcessResult result = GitProcess.InvokeProcess(
this.Enlistment.RepoRoot,
command,
timeoutMs: AcquireGVFSLockTimeout);

result.ExitCode.ShouldEqual(0);
result.Errors.ShouldNotContain(false, ExpectedStatusWaitingText);
});
}

[TestCase, Order(5)]
Expand Down
76 changes: 76 additions & 0 deletions GVFS/GVFS.FunctionalTests/Tests/GitCommands/CreatePlaceholderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
using GVFS.FunctionalTests.Properties;
using GVFS.FunctionalTests.Should;
using GVFS.FunctionalTests.Tools;
using GVFS.Tests.Should;
using NUnit.Framework;
using System;
using System.Diagnostics;
using System.IO;
using System.Runtime.InteropServices;
using System.Threading;
Expand All @@ -19,6 +22,18 @@ public CreatePlaceholderTests(Settings.ValidateWorkingTreeMode validateWorkingTr
{
}

[TestCase]
public void AllowsPlaceholderCreationForDefaultAnnotate()
{
ProcessResult result = GitHelpers.InvokeGitAgainstGVFSRepo(
this.Enlistment.RepoRoot,
$"annotate -- \"{GitHelpers.ConvertPathToGitFormat(FileToRead)}\"");

result.ExitCode.ShouldEqual(0);
result.Errors.ShouldBeEmpty();
string.IsNullOrWhiteSpace(result.Output).ShouldBeFalse("Annotate did not return file annotations.");
}

[TestCase("check-attr --stdin --all")]
[TestCase("check-ignore --stdin")]
[TestCase("check-mailmap --stdin")]
Expand All @@ -44,6 +59,67 @@ public void BlocksPlaceholderCreationWhileGitCommandIsRunning(string commandToRu
this.CheckPlaceholderCreation(commandToRun, shouldAllow: false);
}

[TestCase("worktree list")]
[TestCase("worktree list --porcelain")]
[TestCase("worktree list --porcelain -z")]
public void AllowsPlaceholderCreationWhileWorktreeListIsRunning(string command)
{
string hookListPath = this.Enlistment.GetVirtualPathTo(".git", "hooks", "post-command.hooks");
string originalHookList = File.ReadAllText(hookListPath);
string eventName = "GVFS.WorktreeList." + Guid.NewGuid().ToString("N");
string expectedContent = File.ReadAllText(Path.Combine(this.ControlGitRepo.RootPath, FileToRead));

using (EventWaitHandle ready = new EventWaitHandle(false, EventResetMode.ManualReset, eventName + ".Ready"))
using (EventWaitHandle release = new EventWaitHandle(false, EventResetMode.ManualReset, eventName + ".Release"))
using (Process git = new Process())
{
git.StartInfo = new ProcessStartInfo(Settings.Default.PathToGit, command)
{
WorkingDirectory = this.Enlistment.RepoRoot,
UseShellExecute = false,
RedirectStandardOutput = true,
RedirectStandardError = true,
};
git.StartInfo.Environment["GVFS_TEST_HOOK_WAIT_EVENT_NAME"] = eventName;

bool started = false;
try
{
// Pause the post-command chain before GVFS can release an incorrectly acquired lock.
string waitHook = $"\"{GitHelpers.LockHolderCommandPath}\" --wait-for-event";
File.WriteAllText(hookListPath, waitHook + Environment.NewLine + originalHookList);

started = git.Start();
started.ShouldBeTrue();
bool reachedHook = ready.WaitOne(TimeSpan.FromSeconds(30));
if (!reachedHook && git.HasExited)
{
Assert.Fail($"Git exited before the wait hook. Exit code: {git.ExitCode}. Errors: {git.StandardError.ReadToEnd()}");
}

reachedHook.ShouldBeTrue("Git did not reach the post-command hook.");

File.ReadAllText(this.Enlistment.GetVirtualPathTo(FileToRead)).ShouldEqual(expectedContent);

release.Set();
git.WaitForExit(10000).ShouldBeTrue("Git did not exit after the test released the hook.");
git.ExitCode.ShouldEqual(0);
git.StandardError.ReadToEnd().ShouldBeEmpty();
}
finally
{
release.Set();
if (started && !git.WaitForExit(10000))
{
git.Kill(entireProcessTree: true);
git.WaitForExit();
}

File.WriteAllText(hookListPath, originalHookList);
}
}
}

private void CheckPlaceholderCreation(string command, bool shouldAllow)
{
string eofCharacter = "\x04";
Expand Down
42 changes: 41 additions & 1 deletion GVFS/GVFS.FunctionalTests/Tools/GitHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public static class GitHelpers
private const string WindowsPathSeparator = "\\";
private const string GitPathSeparator = "/";

private static string LockHolderCommandPath
public static string LockHolderCommandPath
{
get
{
Expand Down Expand Up @@ -189,6 +189,46 @@ public static ManualResetEventSlim AcquireGVFSLock(
out processId);
}

public static void RunWithGVFSLock(GVFSFunctionalTestEnlistment enlistment, Action action)
{
ArgumentNullException.ThrowIfNull(enlistment);
ArgumentNullException.ThrowIfNull(action);

ProcessStartInfo processInfo = new ProcessStartInfo(LockHolderCommandPath)
{
WorkingDirectory = enlistment.RepoRoot,
UseShellExecute = false,
RedirectStandardInput = true,
RedirectStandardOutput = true,
RedirectStandardError = true,
};

using (Process holdingProcess = Process.Start(processInfo))
{
try
{
enlistment.WaitForLock(LockHolderCommandName)
.ShouldBeTrue("The test lock holder did not acquire the GVFS lock.");
action();
}
finally
{
holdingProcess.StandardInput.WriteLine("done");
holdingProcess.StandardInput.Close();

bool exited = holdingProcess.WaitForExit(10000);
if (!exited)
{
holdingProcess.Kill(entireProcessTree: true);
holdingProcess.WaitForExit();
}

exited.ShouldBeTrue("The lock holder did not exit after the test released it.");
holdingProcess.ExitCode.ShouldEqual(0);
}
}
}

/// <summary>
/// Run the specified Git command. This method will return once the GVFSLock has been acquired.
/// </summary>
Expand Down
Loading
Loading