diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index a4ca46cfbf..bce5e1c6be 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -24,7 +24,7 @@ permissions: checks: read env: - GIT_VERSION: ${{ github.event.inputs.git_version || 'v2.54.0.vfs.0.2' }} + GIT_VERSION: ${{ github.event.inputs.git_version || 'v2.54.0.vfs.0.4' }} jobs: validate: diff --git a/AGENTS.md b/AGENTS.md index 4fafb53e9c..e5dd769f3d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -166,6 +166,39 @@ Do not manually delete or re-run vcpkg unless you've changed an overlay port. If you've copied `out\` from another enlistment as a build-time shortcut, vcpkg results come along with it. +## What ships in a public release + +The signed release pipeline builds several artifacts, but the **public GitHub +release only carries the installer and symbols**: `SetupGVFS..exe` +(per arch) and `Symbols.zip`. + +- **FastFetch is NOT shipped publicly.** `FastFetch.exe` is built and + ESRP-signed as a pipeline artifact, but it is not attached to the GitHub + release. Treat FastFetch-only changes as **non-shipping** when scoping a + release or writing changelog notes — they don't reach the installer end + users get. +- **Git is NOT bundled.** VFS for Git does not ship Microsoft Git. PRs titled + "update default Microsoft Git version" change only the CI `GIT_VERSION` in + `.github/workflows/build.yaml` (used to install Git for build + functional + test runs) — they are **non-shipping**. The only shipped Git constraint is + the compiled `MinimumGitVersion` constant in `Version.props`, which is + changed separately and rarely. + +## Feature flags + +Product feature flags are **git config** keys under the `gvfs.` prefix (not a +separate config system). To add one, mirror `gvfs.show-hydration-status`: + +- Declare the key name and default in `GVFS.Common/GVFSConstants.cs` under + `GitConfig` (e.g. `ShowHydrationStatus = GVFSPrefix + "show-hydration-status"` + and `ShowHydrationStatusDefault = false`). +- Read it via `repo.GetConfigBoolOrDefault(name, default)` (or + `LibGit2RepoInvoker.GetConfigBoolOrDefault`) at the point of use. + +Default new gates to `false` and gate the **runtime entry point** into a +feature, not its build, so the code still compiles and ships (and keeps +getting exercised) while its behavior stays off by default. + ## Coding standards See [CONTRIBUTING.md](CONTRIBUTING.md) for StyleCop rules, error-handling diff --git a/GVFS/GVFS.Common/AzDevOpsOrgFromNuGetFeed.cs b/GVFS/GVFS.Common/AzDevOpsOrgFromNuGetFeed.cs deleted file mode 100644 index aaafb682fc..0000000000 --- a/GVFS/GVFS.Common/AzDevOpsOrgFromNuGetFeed.cs +++ /dev/null @@ -1,51 +0,0 @@ -using System.Text.RegularExpressions; - -namespace GVFS.Common -{ - public class AzDevOpsOrgFromNuGetFeed - { - /// - /// Given a URL for a NuGet feed hosted on Azure DevOps, - /// return the organization that hosts the feed. - /// - public static bool TryParseOrg(string packageFeedUrl, out string orgName) - { - // We expect a URL of the form https://pkgs.dev.azure.com/{org} - // and want to convert it to a URL of the form https://{org}.visualstudio.com - Regex packageUrlRegex = new Regex( - @"^https://pkgs.dev.azure.com/(?.+?)/", - RegexOptions.CultureInvariant | RegexOptions.IgnoreCase); - Match urlMatch = packageUrlRegex.Match(packageFeedUrl); - - if (!urlMatch.Success) - { - orgName = null; - return false; - } - - orgName = urlMatch.Groups["org"].Value; - return true; - } - - /// - /// Given a URL for a NuGet feed hosted on Azure DevOps, - /// return a URL that Git Credential Manager can use to - /// query for a credential that is valid for use with the - /// NuGet feed. - /// - public static bool TryCreateCredentialQueryUrl(string packageFeedUrl, out string azureDevOpsUrl, out string error) - { - if (!TryParseOrg(packageFeedUrl, out string org)) - { - azureDevOpsUrl = null; - error = $"Input URL {packageFeedUrl} did not match expected format for an Azure DevOps Package Feed URL"; - return false; - } - - azureDevOpsUrl = $"https://{org}.visualstudio.com"; - error = null; - - return true; - } - } -} diff --git a/GVFS/GVFS.Common/Database/SqliteErrorCodes.cs b/GVFS/GVFS.Common/Database/SqliteErrorCodes.cs index 703074a911..9efd7e9405 100644 --- a/GVFS/GVFS.Common/Database/SqliteErrorCodes.cs +++ b/GVFS/GVFS.Common/Database/SqliteErrorCodes.cs @@ -22,13 +22,21 @@ public static class SqliteErrorCodes public const int NotADatabase = 26; /// - /// Returns true if the error code represents a transient condition - /// that may resolve on retry (I/O errors, locking contention). + /// Returns true if the error code represents a transient locking condition + /// that is safe to retry. Only BUSY and LOCKED qualify: both mean the operation + /// was blocked before executing and was never committed, so retrying is idempotent. + /// + /// SQLITE_IOERR is intentionally excluded. An I/O error can fire during connection + /// disposal AFTER a write has already committed (e.g. WAL flush on close). Retrying + /// in that case would re-execute the read phase on an already-empty table and return + /// stale results — a risk that exists in ExecuteReadThenWrite (RemoveAllEntriesForFolder). + /// IOERR on a genuinely failed write will have been rolled back by SQLite atomically, + /// but we cannot distinguish that case at the catch site without inspecting whether + /// the write had already committed. The safe choice is to surface IOERR immediately. /// public static bool IsTransientError(int sqliteErrorCode) { - return sqliteErrorCode == DiskIOError - || sqliteErrorCode == Busy + return sqliteErrorCode == Busy || sqliteErrorCode == Locked; } } diff --git a/GVFS/GVFS.Common/GVFSConstants.cs b/GVFS/GVFS.Common/GVFSConstants.cs index 8f135786aa..3dd946bff5 100644 --- a/GVFS/GVFS.Common/GVFSConstants.cs +++ b/GVFS/GVFS.Common/GVFSConstants.cs @@ -29,6 +29,7 @@ public static class GitConfig public const string MaxRetriesConfig = GVFSPrefix + "max-retries"; public const string TimeoutSecondsConfig = GVFSPrefix + "timeout-seconds"; public const string GitStatusCacheBackoffConfig = GVFSPrefix + "status-cache-backoff-seconds"; + public const string MaxActiveEnumerationsConfig = GVFSPrefix + "max-active-enumerations"; public const string MountId = GVFSPrefix + "mount-id"; public const string EnlistmentId = GVFSPrefix + "enlistment-id"; public const string CacheServer = GVFSPrefix + "cache-server"; @@ -47,18 +48,32 @@ public static class GitConfig public const string ShowHydrationStatus = GVFSPrefix + "show-hydration-status"; public const bool ShowHydrationStatusDefault = false; + /* Gates the CLI mount-progress display layer (progress phase strings surfaced + * over the named pipe during `gvfs mount`). Disabled by default so stabilization + * builds keep the reliability-relevant early-pipe infrastructure without shipping + * the newer progress-display behavior. The early pipe, HandleRequest Mounting + * guard, volatile currentState, and null-safe GetStatus are always on regardless. */ + public const string MountProgress = GVFSPrefix + "mount-progress"; + public const bool MountProgressDefault = false; + public const string MaxHttpConnectionsConfig = GVFSPrefix + "max-http-connections"; public const string PrefetchUseIdx = GVFSPrefix + "prefetch-use-idx"; public const bool PrefetchUseIdxDefault = false; + + /* Gates the background (fire-and-forget) auth + /gvfs/config behavior on + * mount when a cache server is configured locally. When false (default), + * mount blocks on auth/config synchronously before starting virtualization. + * When true, auth runs in the background so mount is not delayed. */ + public const string BackgroundCacheAuth = GVFSPrefix + "background-cache-auth"; + public const bool BackgroundCacheAuthDefault = false; + + public const string PrefetchOffload = GVFSPrefix + "prefetch-offload"; + public const bool PrefetchOffloadDefault = false; } public static class LocalGVFSConfig { - public const string UpgradeRing = "upgrade.ring"; - public const string UpgradeFeedPackageName = "upgrade.feedpackagename"; - public const string UpgradeFeedUrl = "upgrade.feedurl"; - public const string OrgInfoServerUrl = "upgrade.orgInfoServerUrl"; public const string USNJournalUpdates = "usn.updateDirectories"; } @@ -268,17 +283,5 @@ public static class Unmount public const string SkipLock = "skip-wait-for-lock"; } } - - public static class UpgradeVerbMessages - { - public const string GVFSUpgrade = "`gvfs upgrade`"; - public const string GVFSUpgradeDryRun = "`gvfs upgrade --dry-run`"; - public const string NoUpgradeCheckPerformed = "No upgrade check was performed."; - public const string NoneRingConsoleAlert = "Upgrade ring set to \"None\". " + NoUpgradeCheckPerformed; - public const string NoRingConfigConsoleAlert = "Upgrade ring is not set. " + NoUpgradeCheckPerformed; - public const string InvalidRingConsoleAlert = "Upgrade ring set to unknown value. " + NoUpgradeCheckPerformed; - public const string SetUpgradeRingCommand = "To set or change upgrade ring, run `gvfs config " + LocalGVFSConfig.UpgradeRing + " [\"Fast\"|\"Slow\"|\"None\"]` from a command prompt."; - public const string UnmountRepoWarning = "Upgrade will unmount and remount gvfs repos, ensure you are at a stopping point."; - } } } diff --git a/GVFS/GVFS.Common/GVFSJsonContext.cs b/GVFS/GVFS.Common/GVFSJsonContext.cs index 13b35b27c9..555da7ca57 100644 --- a/GVFS/GVFS.Common/GVFSJsonContext.cs +++ b/GVFS/GVFS.Common/GVFSJsonContext.cs @@ -23,7 +23,6 @@ namespace GVFS.Common [JsonSerializable(typeof(List))] [JsonSerializable(typeof(List))] [JsonSerializable(typeof(ServerGVFSConfig))] - [JsonSerializable(typeof(VersionResponse))] [JsonSerializable(typeof(InternalVerbParameters))] [JsonSerializable(typeof(CacheServerInfo))] [JsonSerializable(typeof(NamedPipeMessages.GetStatus.Response), TypeInfoPropertyName = "GetStatusResponse")] diff --git a/GVFS/GVFS.Common/GVFSLock.cs b/GVFS/GVFS.Common/GVFSLock.cs index c8cac11ffe..9cc680fb06 100644 --- a/GVFS/GVFS.Common/GVFSLock.cs +++ b/GVFS/GVFS.Common/GVFSLock.cs @@ -47,7 +47,8 @@ public bool TryAcquireLockForExternalRequestor( // failure on OpenProcess for a different-integrity caller) we still accept the // lock and fall back to the legacy PID-only orphan check; record the fallback in // telemetry so we can spot if it becomes common. - long? requestorStartTime = GVFSPlatform.Instance.TryGetActiveProcessStartTime(requestor.PID, out long startTime) + long? requestorStartTime = + GVFSPlatform.Instance.TryGetActiveProcessStartTime(requestor.PID, out long startTime) == ProcessStartTimeResult.Success ? startTime : (long?)null; if (requestorStartTime == null) @@ -215,6 +216,17 @@ private bool IsLockAvailable(bool checkExternalHolderOnly, out NamedPipeMessages this.tracer.SetGitCommandSessionId(string.Empty); existingExternalHolder = null; } + else if (terminationReason == "StartTimeIndeterminate" && existingExternalHolder != null) + { + // We could not determine whether the holder is still alive, so we deliberately + // kept the lock rather than risk releasing one that is still held. Record it so + // we can see whether these transient failures ever happen in the field (and, + // if a holder ever gets stuck this way, why the lock was not released). + EventMetadata metadata = new EventMetadata(); + metadata.Add("CurrentLockHolder", existingExternalHolder.ToString()); + metadata.Add("Reason", terminationReason); + this.tracer.RelatedEvent(EventLevel.Verbose, "ExternalHolderLivenessIndeterminate", metadata); + } return existingExternalHolder == null; } @@ -460,17 +472,43 @@ public NamedPipeMessages.LockData GetExternalHolder(out bool externalHolderTermi { // Identity check: confirm the same process still owns this PID by comparing // the OS-supplied process start time we captured at acquisition with the - // current one. A mismatch means the original holder exited and Windows - // recycled the PID to a different process (the bug this code fixes). - if (!GVFSPlatform.Instance.TryGetActiveProcessStartTime(pid, out long currentStartTime)) + // current one. We only conclude the holder is gone (and release its lock) + // when we have positive evidence, so that a transient failure to read the + // start time can never release a lock that is still legitimately held. + ProcessStartTimeResult result = GVFSPlatform.Instance.TryGetActiveProcessStartTime(pid, out long currentStartTime); + switch (result) { - externalHolderTerminatedWithoutReleasingLock = true; - terminationReason = "ProcessNotActive"; - } - else if (currentStartTime != capturedStartTime) - { - externalHolderTerminatedWithoutReleasingLock = true; - terminationReason = "PidRecycled"; + case ProcessStartTimeResult.Success when currentStartTime == capturedStartTime: + // Same process, still alive. Keep the lock. + break; + + case ProcessStartTimeResult.Success: + // A different, still-running process now owns this PID: the original + // holder exited and Windows recycled the PID (the bug this fixes). + externalHolderTerminatedWithoutReleasingLock = true; + terminationReason = "PidRecycled"; + break; + + case ProcessStartTimeResult.ProcessNotFound: + // Positive evidence the holder is gone (no such PID, or it exited). + externalHolderTerminatedWithoutReleasingLock = true; + terminationReason = "ProcessExited"; + break; + + case ProcessStartTimeResult.Inaccessible: + // We could open the holder at acquire time, and that access is stable + // for the process's lifetime, so an access-denied result now means the + // PID refers to a different process: the original holder is gone. + externalHolderTerminatedWithoutReleasingLock = true; + terminationReason = "HolderInaccessible"; + break; + + case ProcessStartTimeResult.Indeterminate: + default: + // We could not determine liveness (e.g. transient resource error). + // Do NOT release: keep the lock and let the next poll re-evaluate. + terminationReason = "StartTimeIndeterminate"; + break; } } else diff --git a/GVFS/GVFS.Common/GVFSPlatform.cs b/GVFS/GVFS.Common/GVFSPlatform.cs index 8935c72b6c..5b232628aa 100644 --- a/GVFS/GVFS.Common/GVFSPlatform.cs +++ b/GVFS/GVFS.Common/GVFSPlatform.cs @@ -68,16 +68,21 @@ public static void Register(GVFSPlatform platform) public abstract bool IsProcessActive(int processId); /// - /// Returns true and writes an opaque, OS-supplied process-identity token (e.g. process - /// creation time on Windows) when the process with the given PID is currently active. - /// The token has no meaning beyond identity comparison: two calls for the same underlying - /// process yield equal tokens, and a call after the OS has recycled the PID to a different - /// process yields a different token. Returns false (and startTime = 0) if the process - /// is no longer running, or if it could not be identified for any reason (e.g. permission - /// failure). Callers must treat a false return as "no identity information available" and - /// fall back to if they still need a liveness check. + /// Attempts to read an opaque, OS-supplied process-identity token (e.g. process creation + /// time on Windows) for the process with the given PID. The token has no meaning beyond + /// identity comparison: two calls for the same underlying process yield equal tokens, and + /// a call after the OS has recycled the PID to a different process yields a different token. /// - public abstract bool TryGetActiveProcessStartTime(int processId, out long startTime); + /// The PID to inspect. + /// The identity token, valid only when the result is . + /// + /// A describing the outcome. Callers making a + /// release-versus-hold decision about a lock holder must only treat + /// and + /// as evidence the original holder is gone; + /// means "unknown" and must not trigger a release. + /// + public abstract ProcessStartTimeResult TryGetActiveProcessStartTime(int processId, out long startTime); public abstract void IsServiceInstalledAndRunning(string name, out bool installed, out bool running); public abstract string GetNamedPipeName(string enlistmentRoot); diff --git a/GVFS/GVFS.Common/Git/GitAuthentication.cs b/GVFS/GVFS.Common/Git/GitAuthentication.cs index 96f47673d1..37c3563b51 100644 --- a/GVFS/GVFS.Common/Git/GitAuthentication.cs +++ b/GVFS/GVFS.Common/Git/GitAuthentication.cs @@ -21,6 +21,14 @@ public class GitAuthentication private readonly ICredentialStore credentialStore; private readonly string repoUrl; + // Signaled when initialization completes (success or failure). Callers of + // TryGetCredentials that arrive before initialization is done wait on this + // rather than throwing. This matters when mount starts virtualization while + // auth initialization is still running in the background (cache-server path): + // a directory enumeration can call TryGetCredentials before the background + // TryInitializeAndQueryGVFSConfig has set isInitialized. + private readonly ManualResetEventSlim initializationComplete = new ManualResetEventSlim(false); + private int numberOfAttempts = 0; private DateTime lastAuthAttempt = DateTime.MinValue; @@ -50,6 +58,14 @@ public bool IsBackingOff public bool IsAnonymous { get; private set; } = true; + /// + /// How long a caller of will wait for + /// initialization to complete before giving up with a retryable error. + /// Defaults to the maximum time initialization itself can take (a + /// background credential fetch). Overridable for tests. + /// + internal int InitializationWaitTimeoutMs { get; set; } = BackgroundCredentialTimeoutMs; + private GitSsl GitSsl { get; } public void ApproveCredentials(ITracer tracer, string credentialString) @@ -157,7 +173,17 @@ public bool TryGetCredentials(ITracer tracer, out string credentialString, out s { if (!this.isInitialized) { - throw new InvalidOperationException("This auth instance must be initialized before it can be used"); + // Initialization may still be running in the background (mount can + // start virtualization before the background auth/config query has + // finished when a cache server is configured). Wait for it to + // complete rather than throwing an unhandled exception into the + // virtualization callback, which would crash the mount process. + if (!this.initializationComplete.Wait(this.InitializationWaitTimeoutMs)) + { + credentialString = null; + errorMessage = "Timed out waiting for authentication to initialize"; + return false; + } } credentialString = this.cachedCredentialString; @@ -243,14 +269,14 @@ public bool TryInitializeAndQueryGVFSConfig( if (configRequestor.TryQueryGVFSConfig(false, out serverGVFSConfig, out httpStatus, out _)) { this.IsAnonymous = true; - this.isInitialized = true; + this.MarkInitialized(); tracer.RelatedInfo("{0}: Anonymous access succeeded, config obtained in one request", nameof(this.TryInitializeAndQueryGVFSConfig)); return true; } if (httpStatus != HttpStatusCode.Unauthorized) { - this.isInitialized = true; + this.MarkInitialized(); errorMessage = "Unable to query /gvfs/config"; tracer.RelatedWarning("{0}: Config query failed with status {1}", nameof(this.TryInitializeAndQueryGVFSConfig), httpStatus?.ToString() ?? "None"); return false; @@ -265,12 +291,12 @@ public bool TryInitializeAndQueryGVFSConfig( // Mark initialized even on failure so TryGetCredentials can // retry later (e.g., when mount proceeds with a cache server // and object downloads need auth). - this.isInitialized = true; + this.MarkInitialized(); tracer.RelatedWarning("{0}: Credential fetch failed: {1}", nameof(this.TryInitializeAndQueryGVFSConfig), errorMessage); return false; } - this.isInitialized = true; + this.MarkInitialized(); // Retry with credentials using the same ConfigHttpRequestor (reuses HttpClient/connection) HttpStatusCode? retryHttpStatus; @@ -303,7 +329,7 @@ internal bool TryInitializeAndRequireAuth(ITracer tracer, out string errorMessag if (this.TryCallGitCredential(tracer, out errorMessage)) { - this.isInitialized = true; + this.MarkInitialized(); return true; } @@ -385,6 +411,14 @@ private void UpdateBackoff() this.numberOfAttempts++; } + private void MarkInitialized() + { + // Publish isInitialized before signaling so a waiter that wakes up + // observes the initialized state. + this.isInitialized = true; + this.initializationComplete.Set(); + } + private bool TryCallGitCredential(ITracer tracer, out string errorMessage, int timeoutMs = -1) { // Serialize credential fetches so only one git-credential-fill diff --git a/GVFS/GVFS.Common/Git/GitProcess.cs b/GVFS/GVFS.Common/Git/GitProcess.cs index cf666bc646..03cc27b417 100644 --- a/GVFS/GVFS.Common/Git/GitProcess.cs +++ b/GVFS/GVFS.Common/Git/GitProcess.cs @@ -15,6 +15,28 @@ public class GitProcess : ICredentialStore { private const int HResultEHANDLE = -2147024890; // 0x80070006 E_HANDLE + /// + /// Upper bound on the number of characters buffered from a git process's stderr. Bounding + /// stderr prevents a pathologically noisy git command (e.g. 'multi-pack-index write' repeatedly + /// reporting "could not load pack N" against a corrupt packfile) from growing the capture buffer + /// until GVFS.Mount hits an OutOfMemoryException. stderr is a diagnostic stream, so truncating it + /// is safe: callers can observe it via and should log it, + /// but must not fail solely because of it. + /// + private const int MaxCapturedStdErrChars = 10 * 1024 * 1024; // ~20 MB of UTF-16 + + /// + /// Upper bound on the number of characters buffered from a git process's stdout when the caller + /// buffers the whole result (i.e. does not stream it line-by-line). Sized to hold the + /// machine-readable (-z) output of the largest real repositories - on the order of 2.8M + /// status/diff records on the biggest os.2020 branches - with headroom, while staying far below + /// the .NET maximum array/string size so the buffer itself cannot OOM. stdout can carry + /// correctness-critical data (e.g. the full staged-file list), so truncation here is surfaced via + /// and those callers fail safe rather than act on a partial + /// result. + /// + private const int MaxCapturedStdOutChars = 128 * 1024 * 1024; // ~256 MB of UTF-16 + private static readonly Encoding UTF8NoBOM = new UTF8Encoding(false); private static bool failedToSetEncoding = false; private static string expireTimeDateString; @@ -967,14 +989,19 @@ protected virtual Result InvokeGitImpl( // Do not perform a synchronous read to the end of both redirected streams. using (this.executingProcess = this.GetGitProcess(command, workingDirectory, dotGitDirectory, useReadObjectHook, gitObjectsDirectory: gitObjectsDirectory, usePreCommandHook: usePreCommandHook)) { - StringBuilder output = new StringBuilder(); - StringBuilder errors = new StringBuilder(); + // Bound how much stdout/stderr we buffer so a pathologically noisy git command + // cannot grow these buffers without limit until GVFS.Mount hits an + // OutOfMemoryException. stderr gets a tight cap (it is diagnostic); stdout gets a + // large cap sized for the biggest real repositories. Truncation is surfaced on the + // Result so callers of correctness-critical commands can fail safe. + BoundedGitOutputBuffer output = new BoundedGitOutputBuffer(MaxCapturedStdOutChars); + BoundedGitOutputBuffer errors = new BoundedGitOutputBuffer(MaxCapturedStdErrChars); this.executingProcess.ErrorDataReceived += (sender, args) => { if (args.Data != null) { - errors.Append(args.Data + "\n"); + errors.AppendLine(args.Data); } }; this.executingProcess.OutputDataReceived += (sender, args) => @@ -987,7 +1014,7 @@ protected virtual Result InvokeGitImpl( } else { - output.Append(args.Data + "\n"); + output.AppendLine(args.Data); } } }; @@ -1026,11 +1053,11 @@ protected virtual Result InvokeGitImpl( { this.executingProcess.Kill(); - return new Result(output.ToString(), "Operation timed out: " + errors.ToString(), Result.GenericFailureCode); + return new Result(output.ToString(), "Operation timed out: " + errors.ToString(), Result.GenericFailureCode, output.Truncated, errors.Truncated); } } - return new Result(output.ToString(), errors.ToString(), this.executingProcess.ExitCode); + return new Result(output.ToString(), errors.ToString(), this.executingProcess.ExitCode, output.Truncated, errors.Truncated); } } catch (Win32Exception e) @@ -1151,16 +1178,38 @@ public class Result public const int GenericFailureCode = 1; public Result(string stdout, string stderr, int exitCode) + : this(stdout, stderr, exitCode, outputTruncated: false, errorsTruncated: false) + { + } + + public Result(string stdout, string stderr, int exitCode, bool outputTruncated, bool errorsTruncated) { this.Output = stdout; this.Errors = stderr; this.ExitCode = exitCode; + this.OutputTruncated = outputTruncated; + this.ErrorsTruncated = errorsTruncated; } public string Output { get; } public string Errors { get; } public int ExitCode { get; } + /// + /// True if the git process produced more stdout than the capture buffer allowed, so + /// is incomplete. Callers whose correctness depends on the full output + /// (e.g. the complete list of staged files) must treat this as a failure rather than acting + /// on a partial result. + /// + public bool OutputTruncated { get; } + + /// + /// True if the git process produced more stderr than the capture buffer allowed, so + /// is incomplete. This is expected for pathologically noisy commands and + /// is safe: callers should log it for diagnostics but should not fail solely because of it. + /// + public bool ErrorsTruncated { get; } + public bool ExitCodeIsSuccess { get { return this.ExitCode == Result.SuccessCode; } @@ -1184,6 +1233,65 @@ public bool StderrContainsErrors() } } + /// + /// Accumulates lines from a git process's redirected stdout or stderr while enforcing a hard + /// upper bound on the number of characters buffered. Once the cap is reached, further input is + /// dropped and a single truncation marker is appended. This prevents a noisy git command from + /// growing an unbounded StringBuilder until GVFS.Mount hits an OutOfMemoryException. Whether + /// truncation occurred is exposed via . + /// + public class BoundedGitOutputBuffer + { + private readonly StringBuilder builder = new StringBuilder(); + private readonly int maxChars; + private bool truncated; + + public BoundedGitOutputBuffer(int maxChars) + { + this.maxChars = maxChars; + } + + /// + /// True once output has exceeded the cap and been truncated. + /// + public bool Truncated => this.truncated; + + /// + /// Appends a single line (followed by a newline) unless doing so would exceed the cap. The + /// first time the cap is reached, as much of the line as fits is kept and a truncation marker + /// is appended; all subsequent lines are dropped. + /// + public void AppendLine(string line) + { + if (this.truncated) + { + return; + } + + // +1 accounts for the trailing newline we append after each line. + if (this.builder.Length + line.Length + 1 > this.maxChars) + { + int remaining = this.maxChars - this.builder.Length; + if (remaining > 0) + { + this.builder.Append(line, 0, Math.Min(remaining, line.Length)); + } + + this.builder.Append("\n[... GVFS truncated git output after " + this.maxChars + " characters to avoid OutOfMemoryException ...]\n"); + this.truncated = true; + return; + } + + this.builder.Append(line); + this.builder.Append('\n'); + } + + public override string ToString() + { + return this.builder.ToString(); + } + } + public class ConfigResult { private readonly Result result; diff --git a/GVFS/GVFS.Common/Git/GitRepo.cs b/GVFS/GVFS.Common/Git/GitRepo.cs index 1dc9550d43..302b0e13e0 100644 --- a/GVFS/GVFS.Common/Git/GitRepo.cs +++ b/GVFS/GVFS.Common/Git/GitRepo.cs @@ -180,6 +180,28 @@ public virtual bool TryGetMissingSubTrees(string treeSha, out string[] subtrees) return succeeded; } + /// + /// Reads a git config value through the in-process libgit2 repo (no git.exe spawn). + /// + /// Config setting name (e.g. "gvfs.max-active-enumerations"). + /// + /// The config value, or null if the setting is unset. Only meaningful when this returns true. + /// + /// + /// true if the libgit2 repo was available and the lookup ran (the setting may still be unset, + /// in which case is null); false if no libgit2 repo is available. + /// + public virtual bool TryGetConfigValue(string name, out string value) + { + value = null; + if (this.libgit2RepoInvoker == null) + { + return false; + } + + return this.libgit2RepoInvoker.TryInvoke(repo => repo.GetConfigString(name), out value); + } + public void Dispose() { if (this.libgit2RepoInvoker != null) diff --git a/GVFS/GVFS.Common/Git/RequiredGitConfig.cs b/GVFS/GVFS.Common/Git/RequiredGitConfig.cs index 3ab6cd0b60..a9159e6638 100644 --- a/GVFS/GVFS.Common/Git/RequiredGitConfig.cs +++ b/GVFS/GVFS.Common/Git/RequiredGitConfig.cs @@ -24,8 +24,14 @@ public static Dictionary GetRequiredSettings(GVFSEnlistment enli // path would split the command. Git's config parser strips double quotes // but preserves single quotes, and bash treats single-quoted strings as // a single token. + // + // Include the platform's executable extension (matching the file actually + // written to disk by HooksInstaller) so the shell can exec it directly + // instead of probing PATHEXT candidates (.COM, .EXE, ...) on every hook + // invocation. string virtualFileSystemPath = "'" + Paths.ConvertPathToGitFormat( - Path.Combine(enlistment.DotGitRoot, GVFSConstants.DotGit.Hooks.RootName, GVFSConstants.DotGit.Hooks.VirtualFileSystemName)) + "'"; + Path.Combine(enlistment.DotGitRoot, GVFSConstants.DotGit.Hooks.RootName, GVFSConstants.DotGit.Hooks.VirtualFileSystemName) + + GVFSPlatform.Instance.Constants.ExecutableExtension) + "'"; string gitStatusCachePath = null; if (!GVFSEnlistment.IsUnattended(tracer: null) && GVFSPlatform.Instance.IsGitStatusCacheSupported()) diff --git a/GVFS/GVFS.Common/Maintenance/GitMaintenanceStep.cs b/GVFS/GVFS.Common/Maintenance/GitMaintenanceStep.cs index 1df648fc46..a39cf33bfa 100644 --- a/GVFS/GVFS.Common/Maintenance/GitMaintenanceStep.cs +++ b/GVFS/GVFS.Common/Maintenance/GitMaintenanceStep.cs @@ -193,6 +193,17 @@ protected GitProcess.Result RunGitCommand(Func wo throw new StoppingException(); } + if (result?.ErrorsTruncated == true) + { + // stderr was too large to buffer fully and was truncated to avoid an + // OutOfMemoryException (e.g. a corrupt packfile producing a flood of errors). This is + // safe and non-fatal - record it for diagnostics but do not fail the command for it. + this.Context.Tracer.RelatedWarning( + metadata: null, + message: $"{this.Area}: Git process {gitCommand} stderr was truncated to bound memory use", + keywords: Keywords.Telemetry); + } + if (result?.ExitCodeIsFailure == true) { string errorMessage = result?.Errors == null ? string.Empty : result.Errors; diff --git a/GVFS/GVFS.Common/OrgInfoApiClient.cs b/GVFS/GVFS.Common/OrgInfoApiClient.cs deleted file mode 100644 index 93dbd97d92..0000000000 --- a/GVFS/GVFS.Common/OrgInfoApiClient.cs +++ /dev/null @@ -1,78 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Net; -using System.Net.Http; -using System.Text; - -namespace GVFS.Common -{ - /// - /// Class that handles communication with a server that contains version information. - /// - public class OrgInfoApiClient - { - private const string VersionApi = "/api/GetLatestVersion"; - - private HttpClient client; - private string baseUrl; - - public OrgInfoApiClient(HttpClient client, string baseUrl) - { - this.client = client; - this.baseUrl = baseUrl; - } - - private string VersionUrl - { - get - { - return this.baseUrl + VersionApi; - } - } - - public Version QueryNewestVersion(string orgName, string platform, string ring) - { - Dictionary queryParams = new Dictionary() - { - { "Organization", orgName }, - { "Platform", platform }, - { "Ring", ring }, - }; - - string responseString = this.client.GetStringAsync(this.ConstructRequest(this.VersionUrl, queryParams)).GetAwaiter().GetResult(); - VersionResponse versionResponse = VersionResponse.FromJsonString(responseString); - - if (string.IsNullOrEmpty(versionResponse.Version)) - { - return null; - } - - return new Version(versionResponse.Version); - } - - private string ConstructRequest(string baseUrl, Dictionary queryParams) - { - StringBuilder sb = new StringBuilder(baseUrl); - - if (queryParams.Any()) - { - sb.Append("?"); - } - - bool isFirst = true; - foreach (KeyValuePair kvp in queryParams) - { - if (!isFirst) - { - sb.Append("&"); - } - - isFirst = false; - sb.Append($"{WebUtility.UrlEncode(kvp.Key)}={WebUtility.UrlEncode(kvp.Value)}"); - } - - return sb.ToString(); - } - } -} diff --git a/GVFS/GVFS.Common/ProcessStartTimeResult.cs b/GVFS/GVFS.Common/ProcessStartTimeResult.cs new file mode 100644 index 0000000000..dbb1c6fa65 --- /dev/null +++ b/GVFS/GVFS.Common/ProcessStartTimeResult.cs @@ -0,0 +1,38 @@ +namespace GVFS.Common +{ + /// + /// Outcome of attempting to read a process's start time for lock-holder identity checks. + /// The distinction between the failure values matters: the orphan-lock detector must only + /// release a lock when it has positive evidence that the original holder is gone, so it + /// treats "we could not determine anything" () differently from + /// evidence that the holder has exited or been replaced. + /// + public enum ProcessStartTimeResult + { + /// + /// The process is active and its start time was read successfully. The out start time is valid. + /// + Success, + + /// + /// Positive evidence that no live process with the requested identity exists: either there is + /// no process at that PID, or a process was opened but has already exited. Safe to treat the + /// original holder as gone. + /// + ProcessNotFound, + + /// + /// A process exists at the PID but we could not open it (access denied). Because we only + /// identity-track holders we successfully opened at acquire time (and that access relationship + /// is stable for a given process's lifetime), an access-denied result here means the PID now + /// refers to a different process than the original holder. + /// + Inaccessible, + + /// + /// A transient or unclassified failure (e.g. resource exhaustion). We genuinely do not know + /// whether the original holder is alive, so callers must NOT treat this as evidence of exit. + /// + Indeterminate, + } +} diff --git a/GVFS/GVFS.Common/VersionResponse.cs b/GVFS/GVFS.Common/VersionResponse.cs deleted file mode 100644 index 08864f5c01..0000000000 --- a/GVFS/GVFS.Common/VersionResponse.cs +++ /dev/null @@ -1,13 +0,0 @@ - -namespace GVFS.Common -{ - public class VersionResponse - { - public string Version { get; set; } - - public static VersionResponse FromJsonString(string jsonString) - { - return GVFSJsonOptions.Deserialize(jsonString); - } - } -} diff --git a/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/HealthTests.cs b/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/HealthTests.cs index 1e9b6926d2..3e4d7ec847 100644 --- a/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/HealthTests.cs +++ b/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/HealthTests.cs @@ -160,6 +160,7 @@ private void ValidateHealthOutputValues( List directoryHydrationLevels, string enlistmentHealthStatus) { + this.Enlistment.WaitForBackgroundOperations(); string rawOutput = this.Enlistment.Health(directory); List healthOutputLines = new List(rawOutput.Split(new[] { '\n', '\r' }, StringSplitOptions.RemoveEmptyEntries)); diff --git a/GVFS/GVFS.FunctionalTests/Tests/MultiEnlistmentTests/ServiceVerbTests.cs b/GVFS/GVFS.FunctionalTests/Tests/MultiEnlistmentTests/ServiceVerbTests.cs index b2a53cd918..72a0494073 100644 --- a/GVFS/GVFS.FunctionalTests/Tests/MultiEnlistmentTests/ServiceVerbTests.cs +++ b/GVFS/GVFS.FunctionalTests/Tests/MultiEnlistmentTests/ServiceVerbTests.cs @@ -1,6 +1,7 @@ using GVFS.FunctionalTests.Tools; using GVFS.Tests.Should; using NUnit.Framework; +using System; namespace GVFS.FunctionalTests.Tests.MultiEnlistmentTests { @@ -9,6 +10,19 @@ namespace GVFS.FunctionalTests.Tests.MultiEnlistmentTests public class ServiceVerbTests : TestsWithMultiEnlistment { private static readonly string[] EmptyRepoList = new string[] { }; + private readonly string fixtureServiceName = "Test.GVFS.Service.ServiceVerbTests." + Guid.NewGuid().ToString("N"); + + [OneTimeSetUp] + public void StartFixtureService() + { + GVFSServiceProcess.InstallService(this.fixtureServiceName); + } + + [OneTimeTearDown] + public void StopFixtureService() + { + GVFSServiceProcess.UninstallService(this.fixtureServiceName); + } [TestCase] public void ServiceCommandsWithNoRepos() @@ -21,20 +35,22 @@ public void ServiceCommandsWithNoRepos() [TestCase] public void ServiceCommandsWithMultipleRepos() { - GVFSFunctionalTestEnlistment enlistment1 = this.CreateNewEnlistment(); - GVFSFunctionalTestEnlistment enlistment2 = this.CreateNewEnlistment(); + GVFSFunctionalTestEnlistment enlistment1 = this.CreateNewEnlistment(serviceName: this.fixtureServiceName); + GVFSFunctionalTestEnlistment enlistment2 = this.CreateNewEnlistment(serviceName: this.fixtureServiceName); string[] repoRootList = new string[] { enlistment1.EnlistmentRoot, enlistment2.EnlistmentRoot }; GVFSProcess gvfsProcess1 = new GVFSProcess( GVFSTestConfig.PathToGVFS, enlistment1.EnlistmentRoot, - enlistment1.LocalCacheRoot); + enlistment1.LocalCacheRoot, + this.fixtureServiceName); GVFSProcess gvfsProcess2 = new GVFSProcess( GVFSTestConfig.PathToGVFS, enlistment2.EnlistmentRoot, - enlistment2.LocalCacheRoot); + enlistment2.LocalCacheRoot, + this.fixtureServiceName); this.RunServiceCommandAndCheckOutput("--list-mounted", expectedRepoRoots: repoRootList); this.RunServiceCommandAndCheckOutput("--unmount-all", expectedRepoRoots: repoRootList); @@ -57,14 +73,15 @@ public void ServiceCommandsWithMultipleRepos() [TestCase] public void ServiceCommandsWithMountAndUnmount() { - GVFSFunctionalTestEnlistment enlistment1 = this.CreateNewEnlistment(); + GVFSFunctionalTestEnlistment enlistment1 = this.CreateNewEnlistment(serviceName: this.fixtureServiceName); string[] repoRootList = new string[] { enlistment1.EnlistmentRoot }; GVFSProcess gvfsProcess1 = new GVFSProcess( GVFSTestConfig.PathToGVFS, enlistment1.EnlistmentRoot, - enlistment1.LocalCacheRoot); + enlistment1.LocalCacheRoot, + this.fixtureServiceName); this.RunServiceCommandAndCheckOutput("--list-mounted", expectedRepoRoots: repoRootList); @@ -88,7 +105,8 @@ private void RunServiceCommandAndCheckOutput(string argument, string[] expectedR GVFSProcess gvfsProcess = new GVFSProcess( GVFSTestConfig.PathToGVFS, enlistmentRoot: null, - localCacheRoot: null); + localCacheRoot: null, + serviceName: this.fixtureServiceName); string result = gvfsProcess.RunServiceVerb(argument); result.ShouldContain(expectedRepoRoots); diff --git a/GVFS/GVFS.FunctionalTests/Tests/MultiEnlistmentTests/TestsWithMultiEnlistment.cs b/GVFS/GVFS.FunctionalTests/Tests/MultiEnlistmentTests/TestsWithMultiEnlistment.cs index 3c9a353272..fd7c371327 100644 --- a/GVFS/GVFS.FunctionalTests/Tests/MultiEnlistmentTests/TestsWithMultiEnlistment.cs +++ b/GVFS/GVFS.FunctionalTests/Tests/MultiEnlistmentTests/TestsWithMultiEnlistment.cs @@ -31,13 +31,15 @@ protected virtual void OnTearDownEnlistmentsDeleted() protected GVFSFunctionalTestEnlistment CreateNewEnlistment( string localCacheRoot = null, string branch = null, - bool skipPrefetch = false) + bool skipPrefetch = false, + string serviceName = null) { GVFSFunctionalTestEnlistment output = GVFSFunctionalTestEnlistment.CloneAndMount( GVFSTestConfig.PathToGVFS, branch, localCacheRoot, - skipPrefetch); + skipPrefetch, + serviceName); this.enlistmentsToDelete.Add(output); return output; } diff --git a/GVFS/GVFS.FunctionalTests/Tools/GVFSFunctionalTestEnlistment.cs b/GVFS/GVFS.FunctionalTests/Tools/GVFSFunctionalTestEnlistment.cs index 15880551b4..4d653f7e23 100644 --- a/GVFS/GVFS.FunctionalTests/Tools/GVFSFunctionalTestEnlistment.cs +++ b/GVFS/GVFS.FunctionalTests/Tools/GVFSFunctionalTestEnlistment.cs @@ -20,12 +20,20 @@ public class GVFSFunctionalTestEnlistment private static readonly string ZeroBackgroundOperations = "Background operations: 0" + Environment.NewLine; private GVFSProcess gvfsProcess; + private readonly string serviceName; - private GVFSFunctionalTestEnlistment(string pathToGVFS, string enlistmentRoot, string repoUrl, string commitish, string localCacheRoot = null) + private GVFSFunctionalTestEnlistment( + string pathToGVFS, + string enlistmentRoot, + string repoUrl, + string commitish, + string localCacheRoot = null, + string serviceName = null) { this.EnlistmentRoot = enlistmentRoot; this.RepoUrl = repoUrl; this.Commitish = commitish; + this.serviceName = serviceName; if (localCacheRoot == null) { @@ -43,7 +51,7 @@ private GVFSFunctionalTestEnlistment(string pathToGVFS, string enlistmentRoot, s } this.LocalCacheRoot = localCacheRoot; - this.gvfsProcess = new GVFSProcess(pathToGVFS, this.EnlistmentRoot, this.LocalCacheRoot); + this.gvfsProcess = new GVFSProcess(pathToGVFS, this.EnlistmentRoot, this.LocalCacheRoot, this.serviceName); } public string EnlistmentRoot @@ -57,6 +65,7 @@ public string RepoUrl } public string LocalCacheRoot { get; } + public string ServiceName => this.serviceName; public string RepoBackingRoot { @@ -102,24 +111,25 @@ public static GVFSFunctionalTestEnlistment CloneAndMountWithPerRepoCache(string { string enlistmentRoot = GVFSFunctionalTestEnlistment.GetUniqueEnlistmentRoot(); string localCache = GVFSFunctionalTestEnlistment.GetRepoSpecificLocalCacheRoot(enlistmentRoot); - return CloneAndMount(pathToGvfs, enlistmentRoot, null, localCache, skipPrefetch); + return CloneAndMount(pathToGvfs, enlistmentRoot, null, localCache, skipPrefetch, serviceName: null); } public static GVFSFunctionalTestEnlistment CloneAndMount( string pathToGvfs, string commitish = null, string localCacheRoot = null, - bool skipPrefetch = false) + bool skipPrefetch = false, + string serviceName = null) { string enlistmentRoot = GVFSFunctionalTestEnlistment.GetUniqueEnlistmentRoot(); - return CloneAndMount(pathToGvfs, enlistmentRoot, commitish, localCacheRoot, skipPrefetch); + return CloneAndMount(pathToGvfs, enlistmentRoot, commitish, localCacheRoot, skipPrefetch, serviceName); } - public static GVFSFunctionalTestEnlistment CloneAndMountEnlistmentWithSpacesInPath(string pathToGvfs, string commitish = null) + public static GVFSFunctionalTestEnlistment CloneAndMountEnlistmentWithSpacesInPath(string pathToGvfs, string commitish = null, string serviceName = null) { string enlistmentRoot = GVFSFunctionalTestEnlistment.GetUniqueEnlistmentRootWithSpaces(); string localCache = GVFSFunctionalTestEnlistment.GetRepoSpecificLocalCacheRoot(enlistmentRoot); - return CloneAndMount(pathToGvfs, enlistmentRoot, commitish, localCache); + return CloneAndMount(pathToGvfs, enlistmentRoot, commitish, localCache, skipPrefetch: false, serviceName: serviceName); } public static string GetUniqueEnlistmentRoot() @@ -387,14 +397,21 @@ public string GetObjectPathTo(string objectHash) objectHash.Substring(2)); } - private static GVFSFunctionalTestEnlistment CloneAndMount(string pathToGvfs, string enlistmentRoot, string commitish, string localCacheRoot, bool skipPrefetch = false) + private static GVFSFunctionalTestEnlistment CloneAndMount( + string pathToGvfs, + string enlistmentRoot, + string commitish, + string localCacheRoot, + bool skipPrefetch = false, + string serviceName = null) { GVFSFunctionalTestEnlistment enlistment = new GVFSFunctionalTestEnlistment( pathToGvfs, enlistmentRoot ?? GetUniqueEnlistmentRoot(), GVFSTestConfig.RepoToClone, commitish ?? Properties.Settings.Default.Commitish, - localCacheRoot ?? GVFSTestConfig.LocalCacheRoot); + localCacheRoot ?? GVFSTestConfig.LocalCacheRoot, + serviceName); try { diff --git a/GVFS/GVFS.FunctionalTests/Tools/GVFSHelpers.cs b/GVFS/GVFS.FunctionalTests/Tools/GVFSHelpers.cs index 8fdf7fe7fd..d1b356f3d8 100644 --- a/GVFS/GVFS.FunctionalTests/Tools/GVFSHelpers.cs +++ b/GVFS/GVFS.FunctionalTests/Tools/GVFSHelpers.cs @@ -197,9 +197,13 @@ public static void ModifiedPathsShouldNotContain(GVFSFunctionalTestEnlistment en } } - public static string GetInternalParameter(string maintenanceJob = "null", string packfileMaintenanceBatchSize = "null") + public static string GetInternalParameter( + string maintenanceJob = "null", + string packfileMaintenanceBatchSize = "null", + string serviceName = null) { - return $"\"{{\\\"ServiceName\\\":\\\"{GVFSServiceProcess.TestServiceName}\\\"," + + string effectiveServiceName = string.IsNullOrWhiteSpace(serviceName) ? GVFSServiceProcess.TestServiceName : serviceName; + return $"\"{{\\\"ServiceName\\\":\\\"{effectiveServiceName}\\\"," + "\\\"StartedByService\\\":false," + $"\\\"MaintenanceJob\\\":{maintenanceJob}," + $"\\\"PackfileMaintenanceBatchSize\\\":{packfileMaintenanceBatchSize}}}\""; diff --git a/GVFS/GVFS.FunctionalTests/Tools/GVFSProcess.cs b/GVFS/GVFS.FunctionalTests/Tools/GVFSProcess.cs index 5d7f415d7a..7e632a279b 100644 --- a/GVFS/GVFS.FunctionalTests/Tools/GVFSProcess.cs +++ b/GVFS/GVFS.FunctionalTests/Tools/GVFSProcess.cs @@ -14,17 +14,23 @@ public class GVFSProcess private readonly string pathToGVFS; private readonly string enlistmentRoot; private readonly string localCacheRoot; + private readonly string serviceName; public GVFSProcess(GVFSFunctionalTestEnlistment enlistment) - : this(GVFSTestConfig.PathToGVFS, enlistment.EnlistmentRoot, Path.Combine(enlistment.EnlistmentRoot, GVFSTestConfig.DotGVFSRoot)) + : this( + GVFSTestConfig.PathToGVFS, + enlistment.EnlistmentRoot, + Path.Combine(enlistment.EnlistmentRoot, GVFSTestConfig.DotGVFSRoot), + enlistment.ServiceName) { } - public GVFSProcess(string pathToGVFS, string enlistmentRoot, string localCacheRoot) + public GVFSProcess(string pathToGVFS, string enlistmentRoot, string localCacheRoot, string serviceName = null) { this.pathToGVFS = pathToGVFS; this.enlistmentRoot = enlistmentRoot; this.localCacheRoot = localCacheRoot; + this.serviceName = serviceName; } public void Clone(string repositorySource, string branchToCheckout, bool skipPrefetch) @@ -141,13 +147,13 @@ public string LooseObjectStep() return this.CallGVFS( "dehydrate \"" + this.enlistmentRoot + "\"", expectedExitCode: SuccessExitCode, - internalParameter: GVFSHelpers.GetInternalParameter("\\\"LooseObjects\\\"")); + internalParameter: GVFSHelpers.GetInternalParameter("\\\"LooseObjects\\\"", serviceName: this.serviceName)); } public string PackfileMaintenanceStep(long? batchSize) { string sizeString = batchSize.HasValue ? $"\\\"{batchSize.Value}\\\"" : "null"; - string internalParameter = GVFSHelpers.GetInternalParameter("\\\"PackfileMaintenance\\\"", sizeString); + string internalParameter = GVFSHelpers.GetInternalParameter("\\\"PackfileMaintenance\\\"", sizeString, this.serviceName); return this.CallGVFS( "dehydrate \"" + this.enlistmentRoot + "\"", expectedExitCode: SuccessExitCode, @@ -156,7 +162,7 @@ public string PackfileMaintenanceStep(long? batchSize) public string PostFetchStep() { - string internalParameter = GVFSHelpers.GetInternalParameter("\\\"PostFetch\\\""); + string internalParameter = GVFSHelpers.GetInternalParameter("\\\"PostFetch\\\"", serviceName: this.serviceName); return this.CallGVFS( "dehydrate \"" + this.enlistmentRoot + "\"", expectedExitCode: SuccessExitCode, @@ -246,7 +252,7 @@ private string CallGVFS(string args, int expectedExitCode = DoNotCheckExitCode, if (internalParameter == null) { - internalParameter = GVFSHelpers.GetInternalParameter(); + internalParameter = GVFSHelpers.GetInternalParameter(serviceName: this.serviceName); } processInfo.Arguments = args + " " + TestConstants.InternalUseOnlyFlag + " " + internalParameter; diff --git a/GVFS/GVFS.FunctionalTests/Tools/GVFSServiceProcess.cs b/GVFS/GVFS.FunctionalTests/Tools/GVFSServiceProcess.cs index 2ac384629e..022504abbe 100644 --- a/GVFS/GVFS.FunctionalTests/Tools/GVFSServiceProcess.cs +++ b/GVFS/GVFS.FunctionalTests/Tools/GVFSServiceProcess.cs @@ -2,6 +2,7 @@ using System; using System.Diagnostics; using System.IO; +using System.Collections.Generic; using System.Linq; using System.Runtime.InteropServices; using System.ServiceProcess; @@ -11,8 +12,8 @@ namespace GVFS.FunctionalTests.Tools { public static class GVFSServiceProcess { - private static readonly string ServiceNameArgument = "--servicename=" + TestServiceName; - private static Process consoleServiceProcess; + private static readonly Dictionary ConsoleServiceProcesses = + new Dictionary(System.StringComparer.OrdinalIgnoreCase); public static string TestServiceName { @@ -24,77 +25,98 @@ public static string TestServiceName } public static void InstallService() + { + InstallService(TestServiceName); + } + + public static void InstallService(string serviceName) { if (GVFSTestConfig.IsDevMode) { - StartServiceAsConsoleProcess(); + StartServiceAsConsoleProcess(serviceName); } else { - InstallWindowsService(); + InstallWindowsService(serviceName); } } public static void UninstallService() + { + UninstallService(TestServiceName); + } + + public static void UninstallService(string serviceName) { if (GVFSTestConfig.IsDevMode) { - StopConsoleServiceProcess(); - CleanupServiceData(); + StopConsoleServiceProcess(serviceName); + CleanupServiceData(serviceName); } else { - UninstallWindowsService(); + UninstallWindowsService(serviceName); } } public static void StartService() + { + StartService(TestServiceName); + } + + public static void StartService(string serviceName) { if (GVFSTestConfig.IsDevMode) { - StartServiceAsConsoleProcess(); + StartServiceAsConsoleProcess(serviceName); } else { - StartWindowsService(); + StartWindowsService(serviceName); } } public static void StopService() + { + StopService(TestServiceName); + } + + public static void StopService(string serviceName) { if (GVFSTestConfig.IsDevMode) { - StopConsoleServiceProcess(); + StopConsoleServiceProcess(serviceName); } else { - StopWindowsService(); + StopWindowsService(serviceName); } } - private static void StartServiceAsConsoleProcess() + private static void StartServiceAsConsoleProcess(string serviceName) { - StopConsoleServiceProcess(); + StopConsoleServiceProcess(serviceName); string pathToService = GetPathToService(); Console.WriteLine("Starting test service in console mode: " + pathToService); ProcessStartInfo startInfo = new ProcessStartInfo(pathToService); - startInfo.Arguments = $"--console {ServiceNameArgument}"; + startInfo.Arguments = $"--console --servicename={serviceName}"; startInfo.UseShellExecute = false; startInfo.CreateNoWindow = true; startInfo.RedirectStandardOutput = true; startInfo.RedirectStandardError = true; - consoleServiceProcess = Process.Start(startInfo); + Process consoleServiceProcess = Process.Start(startInfo); consoleServiceProcess.ShouldNotBeNull("Failed to start test service process"); + ConsoleServiceProcesses[serviceName] = consoleServiceProcess; // Consume output asynchronously to prevent buffer deadlock consoleServiceProcess.BeginOutputReadLine(); consoleServiceProcess.BeginErrorReadLine(); // Wait for the service to start listening on its named pipe - string pipeName = TestServiceName + ".pipe"; + string pipeName = serviceName + ".pipe"; int retries = 50; while (retries-- > 0) { @@ -116,39 +138,43 @@ private static void StartServiceAsConsoleProcess() throw new System.TimeoutException("Timed out waiting for test service pipe: " + pipeName); } - private static void StopConsoleServiceProcess() + private static void StopConsoleServiceProcess(string serviceName) { - if (consoleServiceProcess != null && !consoleServiceProcess.HasExited) + Process consoleServiceProcess; + if (ConsoleServiceProcesses.TryGetValue(serviceName, out consoleServiceProcess)) { - try - { - Console.WriteLine("Stopping test service console process (PID: " + consoleServiceProcess.Id + ")"); - consoleServiceProcess.Kill(); - consoleServiceProcess.WaitForExit(5000); - } - catch (InvalidOperationException) + if (consoleServiceProcess != null && !consoleServiceProcess.HasExited) { - // Process already exited + try + { + Console.WriteLine("Stopping test service console process (PID: " + consoleServiceProcess.Id + ")"); + consoleServiceProcess.Kill(); + consoleServiceProcess.WaitForExit(5000); + } + catch (InvalidOperationException) + { + // Process already exited + } } - consoleServiceProcess = null; + ConsoleServiceProcesses.Remove(serviceName); } } - private static void CleanupServiceData() + private static void CleanupServiceData(string serviceName) { string commonAppDataRoot = Environment.GetEnvironmentVariable("GVFS_COMMON_APPDATA_ROOT"); string serviceData; if (!string.IsNullOrEmpty(commonAppDataRoot)) { - serviceData = Path.Combine(commonAppDataRoot, TestServiceName); + serviceData = Path.Combine(commonAppDataRoot, serviceName); } else { serviceData = Path.Combine( Environment.GetFolderPath(Environment.SpecialFolder.CommonApplicationData), "GVFS", - TestServiceName); + serviceName); } DirectoryInfo serviceDataDir = new DirectoryInfo(serviceData); @@ -158,14 +184,14 @@ private static void CleanupServiceData() } } - private static void InstallWindowsService() + private static void InstallWindowsService(string serviceName) { - Console.WriteLine("Installing " + TestServiceName); + Console.WriteLine("Installing " + serviceName); - UninstallWindowsService(); + UninstallWindowsService(serviceName); // Wait for delete to complete. If the services control panel is open, this will never complete. - while (RunScCommand("query", TestServiceName).ExitCode == 0) + while (RunScCommand("query", serviceName).ExitCode == 0) { Thread.Sleep(1000); } @@ -178,23 +204,23 @@ private static void InstallWindowsService() string createServiceArguments = string.Format( "{0} binPath= \"{1}\"", - TestServiceName, + serviceName, pathToService); ProcessResult result = RunScCommand("create", createServiceArguments); result.ExitCode.ShouldEqual(0, "Failure while running sc create " + createServiceArguments + "\r\n" + result.Output); - StartWindowsService(); + StartWindowsService(serviceName); } - private static void UninstallWindowsService() + private static void UninstallWindowsService(string serviceName) { - StopWindowsService(); + StopWindowsService(serviceName); - RunScCommand("delete", TestServiceName); + RunScCommand("delete", serviceName); // Make sure to delete any test service data state - string serviceData = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.CommonApplicationData), "GVFS", TestServiceName); + string serviceData = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.CommonApplicationData), "GVFS", serviceName); DirectoryInfo serviceDataDir = new DirectoryInfo(serviceData); if (serviceDataDir.Exists) { @@ -202,24 +228,24 @@ private static void UninstallWindowsService() } } - private static void StartWindowsService() + private static void StartWindowsService(string serviceName) { - ServiceController testService = ServiceController.GetServices().SingleOrDefault(service => service.ServiceName == TestServiceName); - testService.ShouldNotBeNull($"{TestServiceName} does not exist as a service"); + ServiceController testService = ServiceController.GetServices().SingleOrDefault(service => service.ServiceName == serviceName); + testService.ShouldNotBeNull($"{serviceName} does not exist as a service"); - using (ServiceController controller = new ServiceController(TestServiceName)) + using (ServiceController controller = new ServiceController(serviceName)) { - controller.Start(new[] { ServiceNameArgument }); + controller.Start(new[] { "--servicename=" + serviceName }); controller.WaitForStatus(ServiceControllerStatus.Running, TimeSpan.FromSeconds(10)); controller.Status.ShouldEqual(ServiceControllerStatus.Running); } } - private static void StopWindowsService() + private static void StopWindowsService(string serviceName) { try { - ServiceController testService = ServiceController.GetServices().SingleOrDefault(service => service.ServiceName == TestServiceName); + ServiceController testService = ServiceController.GetServices().SingleOrDefault(service => service.ServiceName == serviceName); if (testService != null) { if (testService.Status == ServiceControllerStatus.Running) diff --git a/GVFS/GVFS.Hooks/GVFS.Hooks.csproj b/GVFS/GVFS.Hooks/GVFS.Hooks.csproj index c960ac4300..69988ac802 100644 --- a/GVFS/GVFS.Hooks/GVFS.Hooks.csproj +++ b/GVFS/GVFS.Hooks/GVFS.Hooks.csproj @@ -71,6 +71,9 @@ Common\ProcessResult.cs + + Common\ProcessStartTimeResult.cs + Common\WorktreeCommandParser.cs diff --git a/GVFS/GVFS.Installers/Setup.iss b/GVFS/GVFS.Installers/Setup.iss index 4393da41f2..a2e31a75d5 100644 --- a/GVFS/GVFS.Installers/Setup.iss +++ b/GVFS/GVFS.Installers/Setup.iss @@ -624,89 +624,6 @@ begin Result := True; end; -type - UpgradeRing = (urUnconfigured, urNone, urFast, urSlow); - -function GetConfiguredUpgradeRing(): UpgradeRing; -var - ResultCode: integer; - ResultString: ansiString; -begin - Result := urUnconfigured; - if ExecWithResult('gvfs.exe', 'config upgrade.ring', '', SW_HIDE, ewWaitUntilTerminated, ResultCode, ResultString) then begin - if ResultCode = 0 then begin - ResultString := AnsiLowercase(Trim(ResultString)); - Log('GetConfiguredUpgradeRing: upgrade.ring is ' + ResultString); - if CompareText(ResultString, 'none') = 0 then begin - Result := urNone; - end else if CompareText(ResultString, 'fast') = 0 then begin - Result := urFast; - end else if CompareText(ResultString, 'slow') = 0 then begin - Result := urSlow; - end else begin - Log('GetConfiguredUpgradeRing: Unknown upgrade ring: ' + ResultString); - end; - end else begin - Log('GetConfiguredUpgradeRing: Call to gvfs config upgrade.ring failed with ' + SysErrorMessage(ResultCode)); - end; - end else begin - Log('GetConfiguredUpgradeRing: Call to gvfs config upgrade.ring failed with ' + SysErrorMessage(ResultCode)); - end; -end; - -function IsConfigured(ConfigKey: String): Boolean; -var - ResultCode: integer; - ResultString: ansiString; -begin - Result := False - if ExecWithResult('gvfs.exe', Format('config %s', [ConfigKey]), '', SW_HIDE, ewWaitUntilTerminated, ResultCode, ResultString) then begin - ResultString := AnsiLowercase(Trim(ResultString)); - Log(Format('IsConfigured(%s): value is %s', [ConfigKey, ResultString])); - Result := Length(ResultString) > 1 - end -end; - -procedure SetIfNotConfigured(ConfigKey: String; ConfigValue: String); -var - ResultCode: integer; - ResultString: ansiString; -begin - if IsConfigured(ConfigKey) = False then begin - if ExecWithResult('gvfs.exe', Format('config %s %s', [ConfigKey, ConfigValue]), '', SW_HIDE, ewWaitUntilTerminated, ResultCode, ResultString) then begin - Log(Format('SetIfNotConfigured: Set %s to %s', [ConfigKey, ConfigValue])); - end else begin - Log(Format('SetIfNotConfigured: Failed to set %s with %s', [ConfigKey, SysErrorMessage(ResultCode)])); - end; - end else begin - Log(Format('SetIfNotConfigured: %s is configured, not overwriting', [ConfigKey])); - end; -end; - -procedure SetNuGetFeedIfNecessary(); -var - ConfiguredRing: UpgradeRing; - RingName: String; - TargetFeed: String; - FeedPackageName: String; -begin - ConfiguredRing := GetConfiguredUpgradeRing(); - if ConfiguredRing = urFast then begin - RingName := 'Fast'; - end else if (ConfiguredRing = urSlow) or (ConfiguredRing = urNone) then begin - RingName := 'Slow'; - end else begin - Log('SetNuGetFeedIfNecessary: No upgrade ring configured. Not configuring NuGet feed.') - exit; - end; - - TargetFeed := Format('https://pkgs.dev.azure.com/microsoft/_packaging/VFSForGit-%s/nuget/v3/index.json', [RingName]); - FeedPackageName := 'Microsoft.VfsForGitEnvironment'; - - SetIfNotConfigured('upgrade.feedurl', TargetFeed); - SetIfNotConfigured('upgrade.feedpackagename', FeedPackageName); -end; - // Below are EVENT FUNCTIONS -> The main entry points of InnoSetup into the code region // Documentation : http://www.jrsoftware.org/ishelp/index.php?topic=scriptevents @@ -905,7 +822,6 @@ begin NeedsRestart := False; KeepMountsRunning := False; Result := ''; - SetNuGetFeedIfNecessary(); // Check for mounted repos by querying the service, and also check for // running GVFS processes (a mount can be running without being registered diff --git a/GVFS/GVFS.Mount/InProcessMount.cs b/GVFS/GVFS.Mount/InProcessMount.cs index b88af37fc0..8cf3c386fc 100644 --- a/GVFS/GVFS.Mount/InProcessMount.cs +++ b/GVFS/GVFS.Mount/InProcessMount.cs @@ -58,6 +58,12 @@ public class InProcessMount private volatile MountState currentState; private volatile string mountProgressMessage; + + // When false (default), the mount process does not surface progress phase + // strings over the named pipe, so the CLI falls back to its static spinner. + // This gates only the display layer; the early-pipe reliability infrastructure + // (early pipe start, HandleRequest Mounting guard, null-safe GetStatus) is unaffected. + private bool reportMountProgress; private HeartbeatThread heartbeat; private ManualResetEvent unmountEvent; @@ -132,19 +138,27 @@ private void MountWithLockAcquired(EventLevel verbosity, Keywords keywords) Stopwatch parallelTimer = Stopwatch.StartNew(); bool hasCacheServer = this.cacheServer != null && !string.IsNullOrWhiteSpace(this.cacheServer.Url); + // Fire-and-forget background auth is gated behind gvfs.background-cache-auth + // (default off). When the flag is off, mount blocks on auth/config + // synchronously even if a cache server is configured (pre-existing behavior). + bool useBackgroundAuth = hasCacheServer && this.IsBackgroundCacheAuthEnabled(); + this.tracer.RelatedEvent( EventLevel.Informational, "MountPhase", new EventMetadata { { "Phase", "ParallelMountStarted" }, + { "HasCacheServer", hasCacheServer }, + { "UseBackgroundAuth", useBackgroundAuth }, { "ElapsedMs", mountPhaseTimer.ElapsedMilliseconds }, }, Keywords.Telemetry); - // When a cache server is configured locally, auth/config is best-effort: - // mount can proceed without it. We still attempt it so GCM can pop up a - // renewal prompt for stale tokens, but we don't block mount on the result. + // With background auth enabled, auth/config is best-effort: mount can + // proceed without it. We still attempt it so GCM can pop up a renewal + // prompt for stale tokens, but we don't block mount on the result. A + // longer credential timeout is acceptable only because we don't block. var networkTask = Task.Run(() => { Stopwatch sw = Stopwatch.StartNew(); @@ -155,7 +169,7 @@ private void MountWithLockAcquired(EventLevel verbosity, Keywords keywords) if (!this.enlistment.Authentication.TryInitializeAndQueryGVFSConfig( this.tracer, this.enlistment, this.retryConfig, out config, out authConfigError, out isAuthFailure, - credentialTimeoutMs: hasCacheServer ? GitAuthentication.BackgroundCredentialTimeoutMs : GitAuthentication.DefaultCredentialTimeoutMs)) + credentialTimeoutMs: useBackgroundAuth ? GitAuthentication.BackgroundCredentialTimeoutMs : GitAuthentication.DefaultCredentialTimeoutMs)) { if (hasCacheServer) { @@ -233,6 +247,11 @@ private void MountWithLockAcquired(EventLevel verbosity, Keywords keywords) this.enlistment.InitializeCachePaths(localCacheRoot, gitObjectsRoot, blobSizesRoot); + // Read the display-layer flag once before the pipe opens so the very first + // GetStatus request is served consistently. Only gates the progress strings; + // the pipe still starts early regardless (reliability infrastructure). + this.reportMountProgress = this.ShouldReportMountProgress(); + // Start the pipe server early so MountVerb can connect and poll progress // during the parallel validation phase. Only GetStatus requests are // handled while currentState == Mounting (see HandleRequest guard). @@ -291,9 +310,9 @@ private void MountWithLockAcquired(EventLevel verbosity, Keywords keywords) try { - if (hasCacheServer) + if (useBackgroundAuth) { - // With a cache server, don't block mount on the network task. + // With background auth, don't block mount on the network task. // Auth runs in the background to warm credentials / pop GCM, // but mount proceeds immediately using the local cache server URL. localTask.Wait(); @@ -327,7 +346,10 @@ private void MountWithLockAcquired(EventLevel verbosity, Keywords keywords) }, Keywords.Telemetry); - ServerGVFSConfig serverGVFSConfig = hasCacheServer ? null : networkTask.Result; + // Only background auth leaves the network task potentially unfinished; + // in that case we resolve the cache server from local config (null). + // Otherwise we waited on the network task and can use its result. + ServerGVFSConfig serverGVFSConfig = useBackgroundAuth ? null : networkTask.Result; this.mountProgressMessage = "Resolving cache server"; CacheServerResolver cacheServerResolver = new CacheServerResolver(this.tracer, this.enlistment); @@ -462,6 +484,31 @@ private GVFSContext CreateContext() return new GVFSContext(this.tracer, fileSystem, gitRepo, this.enlistment); } + private bool IsBackgroundCacheAuthEnabled() + { + // Read the flag via libgit2 (in-process) rather than spawning git.exe. + // The GVFSContext (and its shared libgit2 repo) is not created until + // later in mount, so open a short-lived repo here just for the config + // read. Default to off on any failure. + try + { + using (LibGit2Repo repo = new LibGit2Repo(this.tracer, this.enlistment.WorkingDirectoryBackingRoot)) + { + return repo.GetConfigBool(GVFSConstants.GitConfig.BackgroundCacheAuth) + ?? GVFSConstants.GitConfig.BackgroundCacheAuthDefault; + } + } + catch (Exception e) + { + this.tracer.RelatedWarning( + "Failed to read {0} config, defaulting to {1}: {2}", + GVFSConstants.GitConfig.BackgroundCacheAuth, + GVFSConstants.GitConfig.BackgroundCacheAuthDefault, + e.Message); + return GVFSConstants.GitConfig.BackgroundCacheAuthDefault; + } + } + private void ValidateMountPoints() { DirectoryInfo workingDirectoryRootInfo = new DirectoryInfo(this.enlistment.WorkingDirectoryBackingRoot); @@ -581,6 +628,27 @@ private NamedPipeServer StartNamedPipe() } } + private bool ShouldReportMountProgress() + { + // Read via a transient LibGit2Repo rather than the context's shared + // LibGit2RepoInvoker: the invoker isn't created until CreateContext (after the + // pipe starts), and its constructor force-loads the object store, which we must + // not do on the pre-pipe critical path. A raw LibGit2Repo opens only the repo + // handle + config (no object store) and gives native git-bool parsing. + try + { + using (LibGit2Repo repo = new LibGit2Repo(this.tracer, this.enlistment.WorkingDirectoryBackingRoot)) + { + return repo.GetConfigBool(GVFSConstants.GitConfig.MountProgress) ?? GVFSConstants.GitConfig.MountProgressDefault; + } + } + catch (Exception e) + { + this.tracer.RelatedWarning($"Failed to read {GVFSConstants.GitConfig.MountProgress} config, using default: {e.Message}"); + return GVFSConstants.GitConfig.MountProgressDefault; + } + } + private void FailMountAndExit(string error, params object[] args) { this.FailMountAndExit(ReturnCode.GenericError, error, args); @@ -707,6 +775,7 @@ private void HandleRequest(ITracer tracer, string request, NamedPipeServer.Conne metadata.Add("Header", message.Header); metadata.Add("Exception", e.ToString()); this.tracer.RelatedError(metadata, "HandleRequest: Unhandled exception in request handler"); + throw; } } @@ -1377,7 +1446,11 @@ private void HandleGetStatusRequest(NamedPipeServer.Connection connection) { case MountState.Mounting: response.MountStatus = NamedPipeMessages.GetStatus.Mounting; - response.MountProgress = this.mountProgressMessage; + if (this.reportMountProgress) + { + response.MountProgress = this.mountProgressMessage; + } + break; case MountState.Ready: diff --git a/GVFS/GVFS.Platform.Windows/ActiveEnumeration.cs b/GVFS/GVFS.Platform.Windows/ActiveEnumeration.cs index 0baa27a4ce..c56af839c8 100644 --- a/GVFS/GVFS.Platform.Windows/ActiveEnumeration.cs +++ b/GVFS/GVFS.Platform.Windows/ActiveEnumeration.cs @@ -1,6 +1,7 @@ -using GVFS.Common; +using GVFS.Common; using GVFS.Virtualization.Projection; using Microsoft.Windows.ProjFS; +using System; using System.Collections.Generic; namespace GVFS.Platform.Windows @@ -20,10 +21,28 @@ public ActiveEnumeration(List fileInfos) this.fileInfoEnumerator = new ProjectedFileInfoEnumerator(fileInfos); this.ResetEnumerator(); this.MoveNext(); + this.RecordActivity(); } public delegate bool FileNamePatternMatcher(string name, string pattern); + /// + /// Monotonic tick count (, milliseconds) of the most + /// recent activity (creation or enumeration read) on this enumeration. Used to identify + /// enumerations that ProjFS never ended, so their memory can be reclaimed. A monotonic + /// source is used (rather than wall-clock time) so that NTP/clock adjustments cannot cause a + /// live-but-idle enumeration to be misjudged as stale. + /// + public long LastActivityTickCount { get; private set; } + + /// + /// Records that this enumeration was just touched, so it is not treated as abandoned. + /// + public void RecordActivity() + { + this.LastActivityTickCount = Environment.TickCount64; + } + /// /// true if Current refers to an element in the enumeration, false if Current is past the end of the collection /// diff --git a/GVFS/GVFS.Platform.Windows/InternalsVisibleTo.cs b/GVFS/GVFS.Platform.Windows/InternalsVisibleTo.cs new file mode 100644 index 0000000000..9fd1d13ff0 --- /dev/null +++ b/GVFS/GVFS.Platform.Windows/InternalsVisibleTo.cs @@ -0,0 +1,3 @@ +using System.Runtime.CompilerServices; + +[assembly: InternalsVisibleTo("GVFS.UnitTests")] diff --git a/GVFS/GVFS.Platform.Windows/ProjFSFilter.cs b/GVFS/GVFS.Platform.Windows/ProjFSFilter.cs index ec092e9846..3d49968180 100644 --- a/GVFS/GVFS.Platform.Windows/ProjFSFilter.cs +++ b/GVFS/GVFS.Platform.Windows/ProjFSFilter.cs @@ -87,7 +87,7 @@ public static bool TryAttach(string enlistmentRoot, out string errorMessage) /// /// Attempts to attach the ProjFS filter driver to the volume containing the enlistment. - /// If FilterAttach returns ACCESS_DENIED but the ProjFS service is already running, + /// If an unelevated caller gets ACCESS_DENIED but the ProjFS service is already running, /// the filter is presumed to already be attached and the call succeeds. /// public static bool TryAttachToVolume(string enlistmentRoot, ITracer tracer, out string errorMessage) @@ -98,14 +98,22 @@ public static bool TryAttachToVolume(string enlistmentRoot, ITracer tracer, out } // FilterAttach requires SE_LOAD_DRIVER_PRIVILEGE, which is typically only - // granted to administrators. When the caller lacks this privilege but the - // ProjFS service is confirmed running, the filter is probably already - // attached to the volume — treat ACCESS_DENIED as success in that case. + // granted to administrators. An UNELEVATED caller therefore always gets + // ACCESS_DENIED regardless of whether the filter is actually attached, so + // when the ProjFS service is confirmed running we presume the (already + // elevated) service attached the filter and treat this as success. + // + // We intentionally do NOT tolerate ACCESS_DENIED for an ELEVATED caller + // (e.g. GVFS.Service, which runs as LocalSystem): an elevated caller holds + // the privilege, so ACCESS_DENIED there is a genuine failure (AV/EDR lock, + // Group Policy, or driver corruption) that must surface rather than be + // silently swallowed and leave the mount in a broken state. if (errorMessage != null && errorMessage.Contains(AccessDeniedResult.ToString()) + && !WindowsPlatform.IsElevatedImplementation() && IsServiceRunning(tracer)) { - tracer.RelatedInfo($"{nameof(TryAttachToVolume)}: FilterAttach returned ACCESS_DENIED, but ProjFS service is running. Proceeding."); + tracer.RelatedInfo($"{nameof(TryAttachToVolume)}: FilterAttach returned ACCESS_DENIED to an unelevated caller, but ProjFS service is running. Proceeding."); errorMessage = null; return true; } diff --git a/GVFS/GVFS.Platform.Windows/WindowsFileSystem.cs b/GVFS/GVFS.Platform.Windows/WindowsFileSystem.cs index 7ba732a136..0fe4d781f6 100644 --- a/GVFS/GVFS.Platform.Windows/WindowsFileSystem.cs +++ b/GVFS/GVFS.Platform.Windows/WindowsFileSystem.cs @@ -327,6 +327,10 @@ public bool IsFileSystemSupported(string path, out string error) /// Also, even if the fix were in place, automount would still fail because it runs under SYSTEM account. /// /// This method ensures that the directory is owned by the current user (which is verified to work for SYSTEM account for automount). + /// + /// Exception: under Windows "Administrator protection" (AP), an elevated process runs as a profile-separated shadow admin + /// account (MACHINE\admin_<user>) whose SID differs from the real (non-elevated) user's. In that case the directory owner is + /// left as the Administrators group rather than being reassigned to the shadow admin (see the AP note inline). /// public void EnsureDirectoryIsOwnedByCurrentUser(string directoryPath) { @@ -339,6 +343,25 @@ public void EnsureDirectoryIsOwnedByCurrentUser(string directoryPath) SecurityIdentifier administratorsSid = new SecurityIdentifier(WellKnownSidType.BuiltinAdministratorsSid, null); if (directoryOwner == administratorsSid) { + // Under Windows "Administrator protection" (AP), an elevated process runs as a profile-separated shadow + // admin account (MACHINE\admin_) whose SID differs from the real (non-elevated) user's. Reassigning + // ownership to the current user here would set the owner to that shadow admin, causing the real user to hit + // "fatal: detected dubious ownership" — git's Administrators-membership grace does not cover another specific + // user's SID. The shadow admin also cannot SetOwner to the real user's SID. Leaving the owner as the + // Administrators group is accepted by modern git and the libgit2 non-elevated-admin-owner overlay for any + // Administrators member (real user, shadow admin, and SYSTEM for automount alike), so skip the reassignment. + // + // Note: a libgit2 consumer that lacks the non-elevated-admin-owner patch (i.e. stock libgit2 without the + // Administrators-membership fix) will not be able to open an Administrators-owned repo. Addressing that would + // require setting git's safe.directory for the real user, but safe.directory is only honored from global/system + // config (never repo-local), and an elevated clone runs as the shadow admin — so covering the real user would + // mean mutating the real user's global gitconfig or machine-wide system config from the shadow admin. After + // consideration, GVFS is not the right place to do that; unpatched third-party libgit2 consumers are out of scope. + if (WindowsPlatform.IsCurrentUserAdminProtectionShadowAccount()) + { + return; + } + WindowsIdentity currentUser = WindowsIdentity.GetCurrent(); directorySecurity.SetOwner(currentUser.User); directoryInfo.SetAccessControl(directorySecurity); diff --git a/GVFS/GVFS.Platform.Windows/WindowsFileSystemVirtualizer.cs b/GVFS/GVFS.Platform.Windows/WindowsFileSystemVirtualizer.cs index f39128e7af..a4bea90680 100644 --- a/GVFS/GVFS.Platform.Windows/WindowsFileSystemVirtualizer.cs +++ b/GVFS/GVFS.Platform.Windows/WindowsFileSystemVirtualizer.cs @@ -33,12 +33,39 @@ public class WindowsFileSystemVirtualizer : FileSystemVirtualizer, IRequiredCall private ConcurrentDictionary activeEnumerations; private ConcurrentDictionary activeCommands; + // ProjFS does not always deliver EndDirectoryEnumeration for a StartDirectoryEnumeration + // (for example when an enumeration is cancelled), which leaks the corresponding + // ActiveEnumeration - and the projected item list it pins - in this.activeEnumerations. Over + // long-lived mounts this unbounded growth is a suspected contributor to the memory pressure + // that ultimately crashes GVFS.Mount in the native ProjFS command-completion path. + // + // Eviction of stale (never-ended) enumerations is OFF by default and enabled only when the + // gvfs.max-active-enumerations git config is set to a positive value: once activeEnumerations + // grows past that threshold, a throttled sweep evicts entries idle longer than a timeout. + // The count telemetry below is always emitted on the Heartbeat event so the leak can be + // observed in the field before eviction is turned on anywhere. + private static readonly TimeSpan ActiveEnumerationStaleTimeout = TimeSpan.FromMinutes(5); + private static readonly TimeSpan ActiveEnumerationEvictionSweepInterval = TimeSpan.FromMinutes(1); + + // Effective staleness cutoff for a never-ended enumeration. Defaults to the constant above; + // overridable by tests via ActiveEnumerationStaleTimeoutForTest. + private TimeSpan activeEnumerationStaleTimeout = ActiveEnumerationStaleTimeout; + + // 0 disables eviction (the default). When > 0, it is the count of live enumerations above + // which a sweep evicts stale ones. Read from git config (gvfs.max-active-enumerations). + private int maxActiveEnumerations; + + // Monotonic (Environment.TickCount64, milliseconds) timestamp of the last eviction sweep, so + // the throttle cannot be disturbed by wall-clock adjustments. + private long lastEnumerationEvictionSweepTickCount = Environment.TickCount64; + public WindowsFileSystemVirtualizer(GVFSContext context, GVFSGitObjects gitObjects) : this( context, gitObjects, virtualizationInstance: null, - numWorkerThreads: FileSystemVirtualizer.DefaultNumWorkerThreads) + numWorkerThreads: FileSystemVirtualizer.DefaultNumWorkerThreads, + maxActiveEnumerations: ReadMaxActiveEnumerationsFromConfig(context)) { } @@ -46,7 +73,8 @@ public WindowsFileSystemVirtualizer( GVFSContext context, GVFSGitObjects gitObjects, IVirtualizationInstance virtualizationInstance, - int numWorkerThreads) + int numWorkerThreads, + int maxActiveEnumerations = 0) : base(context, gitObjects, numWorkerThreads) { List notificationMappings = new List() @@ -72,10 +100,146 @@ public WindowsFileSystemVirtualizer( this.activeEnumerations = new ConcurrentDictionary(); this.activeCommands = new ConcurrentDictionary(); + + this.maxActiveEnumerations = maxActiveEnumerations; } protected override string EtwArea => ClassName; + public override void AddHeartbeatMetadata(EventMetadata metadata) + { + // Always emitted, even when eviction is disabled, so the size of the activeEnumerations + // collection (and thus whether the never-ended-enumeration leak is a real memory driver) + // can be observed in the field before enabling eviction anywhere. + metadata.Add("ActiveEnumerationCount", this.activeEnumerations.Count); + metadata.Add("ActiveCommandCount", this.activeCommands.Count); + } + + /// + /// Reads the gvfs.max-active-enumerations git config value (0 = eviction disabled, the + /// default). Uses the in-process libgit2 repo (no git.exe spawn). Never throws: any failure + /// reading git config leaves eviction disabled. + /// + private static int ReadMaxActiveEnumerationsFromConfig(GVFSContext context) + { + try + { + if (context?.Repository != null && + context.Repository.TryGetConfigValue(GVFSConstants.GitConfig.MaxActiveEnumerationsConfig, out string rawValue)) + { + if (string.IsNullOrWhiteSpace(rawValue)) + { + // Setting is unset: eviction disabled. + return 0; + } + + if (int.TryParse(rawValue.Trim(), out int value) && value > 0) + { + return value; + } + + if (context.Tracer != null) + { + EventMetadata metadata = new EventMetadata(); + metadata.Add("Area", ClassName); + metadata.Add("configValue", rawValue); + context.Tracer.RelatedWarning(metadata, nameof(ReadMaxActiveEnumerationsFromConfig) + ": could not parse " + GVFSConstants.GitConfig.MaxActiveEnumerationsConfig + " as a positive int, leaving enumeration eviction disabled"); + } + } + } + catch (Exception e) + { + if (context?.Tracer != null) + { + EventMetadata metadata = new EventMetadata(); + metadata.Add("Area", ClassName); + metadata.Add("Exception", e.ToString()); + context.Tracer.RelatedWarning(metadata, nameof(ReadMaxActiveEnumerationsFromConfig) + ": exception reading git config, leaving enumeration eviction disabled"); + } + } + + return 0; + } + + private void MaybeEvictStaleEnumerations() + { + if (this.maxActiveEnumerations <= 0) + { + // Eviction disabled (gvfs.max-active-enumerations unset or non-positive). + return; + } + + long now = Environment.TickCount64; + long last = Interlocked.Read(ref this.lastEnumerationEvictionSweepTickCount); + if (now - last < (long)ActiveEnumerationEvictionSweepInterval.TotalMilliseconds) + { + return; + } + + if (Interlocked.CompareExchange(ref this.lastEnumerationEvictionSweepTickCount, now, last) != last) + { + // Another thread just claimed this sweep interval. + return; + } + + this.EvictStaleEnumerations(); + } + + private void EvictStaleEnumerations() + { + if (this.activeEnumerations.Count <= this.maxActiveEnumerations) + { + return; + } + + long cutoff = Environment.TickCount64 - (long)this.activeEnumerationStaleTimeout.TotalMilliseconds; + int evictedCount = 0; + foreach (KeyValuePair entry in this.activeEnumerations) + { + if (entry.Value.LastActivityTickCount < cutoff && + this.activeEnumerations.TryRemove(entry.Key, out _)) + { + evictedCount++; + } + } + + if (evictedCount > 0) + { + EventMetadata metadata = this.CreateEventMetadata(); + metadata.Add("evictedCount", evictedCount); + metadata.Add("remainingCount", this.activeEnumerations.Count); + metadata.Add("maxActiveEnumerations", this.maxActiveEnumerations); + metadata.Add("staleTimeoutMinutes", this.activeEnumerationStaleTimeout.TotalMinutes); + metadata.Add("gcTotalMemoryBytes", GC.GetTotalMemory(forceFullCollection: false)); + metadata.Add( + TracingConstants.MessageKey.WarningMessage, + nameof(this.EvictStaleEnumerations) + ": evicted stale directory enumerations that ProjFS never ended, to bound memory usage"); + this.Context.Tracer.RelatedEvent(EventLevel.Warning, nameof(this.EvictStaleEnumerations), metadata, Keywords.Telemetry); + } + } + + internal TimeSpan ActiveEnumerationStaleTimeoutForTest + { + set { this.activeEnumerationStaleTimeout = value; } + } + + internal int MaxActiveEnumerationsForTest + { + set { this.maxActiveEnumerations = value; } + } + + /// + /// Test-only: resets the sweep throttle and runs the same eviction path the enumeration hot + /// callback runs, so eviction behavior can be exercised deterministically. + /// + internal void ForceEnumerationEvictionSweepForTest() + { + Interlocked.Exchange( + ref this.lastEnumerationEvictionSweepTickCount, + Environment.TickCount64 - (long)ActiveEnumerationEvictionSweepInterval.TotalMilliseconds - 1); + this.MaybeEvictStaleEnumerations(); + } + /// /// Public for unit testing /// @@ -119,14 +283,21 @@ public override void Stop() public override FileSystemResult ClearNegativePathCache(out uint totalEntryCount) { - HResult result = this.virtualizationInstance.ClearNegativePathCache(out totalEntryCount); + uint entryCount = 0; + HResult result = this.TryInvokeProjFS( + () => this.virtualizationInstance.ClearNegativePathCache(out entryCount), + nameof(this.ClearNegativePathCache)); + totalEntryCount = entryCount; return new FileSystemResult(HResultToFSResult(result), unchecked((int)result)); } public override FileSystemResult DeleteFile(string relativePath, UpdatePlaceholderType updateFlags, out UpdateFailureReason failureReason) { UpdateFailureCause failureCause = UpdateFailureCause.NoFailure; - HResult result = this.virtualizationInstance.DeleteFile(relativePath, (UpdateType)updateFlags, out failureCause); + HResult result = this.TryInvokeProjFS( + () => this.virtualizationInstance.DeleteFile(relativePath, (UpdateType)updateFlags, out failureCause), + nameof(this.DeleteFile), + relativePath); failureReason = (UpdateFailureReason)failureCause; return new FileSystemResult(HResultToFSResult(result), unchecked((int)result)); } @@ -137,17 +308,20 @@ public override FileSystemResult WritePlaceholderFile( string sha) { FileProperties properties = this.FileSystemCallbacks.GetLogsHeadFileProperties(); - HResult result = this.virtualizationInstance.WritePlaceholderInfo( - relativePath, - properties.CreationTimeUTC, - properties.LastAccessTimeUTC, - properties.LastWriteTimeUTC, - changeTime: properties.LastWriteTimeUTC, - fileAttributes: FileAttributes.Archive, - endOfFile: endOfFile, - isDirectory: false, - contentId: FileSystemVirtualizer.ConvertShaToContentId(sha), - providerId: PlaceholderVersionId); + HResult result = this.TryInvokeProjFS( + () => this.virtualizationInstance.WritePlaceholderInfo( + relativePath, + properties.CreationTimeUTC, + properties.LastAccessTimeUTC, + properties.LastWriteTimeUTC, + changeTime: properties.LastWriteTimeUTC, + fileAttributes: FileAttributes.Archive, + endOfFile: endOfFile, + isDirectory: false, + contentId: FileSystemVirtualizer.ConvertShaToContentId(sha), + providerId: PlaceholderVersionId), + nameof(this.WritePlaceholderFile), + relativePath); return new FileSystemResult(HResultToFSResult(result), unchecked((int)result)); } @@ -155,17 +329,20 @@ public override FileSystemResult WritePlaceholderFile( public override FileSystemResult WritePlaceholderDirectory(string relativePath) { FileProperties properties = this.FileSystemCallbacks.GetLogsHeadFileProperties(); - HResult result = this.virtualizationInstance.WritePlaceholderInfo( - relativePath, - properties.CreationTimeUTC, - properties.LastAccessTimeUTC, - properties.LastWriteTimeUTC, - changeTime: properties.LastWriteTimeUTC, - fileAttributes: FileAttributes.Directory, - endOfFile: 0, - isDirectory: true, - contentId: FolderContentId, - providerId: PlaceholderVersionId); + HResult result = this.TryInvokeProjFS( + () => this.virtualizationInstance.WritePlaceholderInfo( + relativePath, + properties.CreationTimeUTC, + properties.LastAccessTimeUTC, + properties.LastWriteTimeUTC, + changeTime: properties.LastWriteTimeUTC, + fileAttributes: FileAttributes.Directory, + endOfFile: 0, + isDirectory: true, + contentId: FolderContentId, + providerId: PlaceholderVersionId), + nameof(this.WritePlaceholderDirectory), + relativePath); return new FileSystemResult(HResultToFSResult(result), unchecked((int)result)); } @@ -183,18 +360,21 @@ public override FileSystemResult UpdatePlaceholderIfNeeded( out UpdateFailureReason failureReason) { UpdateFailureCause failureCause = UpdateFailureCause.NoFailure; - HResult result = this.virtualizationInstance.UpdateFileIfNeeded( - relativePath, - creationTime, - lastAccessTime, - lastWriteTime, - changeTime, - fileAttributes, - endOfFile, - ConvertShaToContentId(shaContentId), - PlaceholderVersionId, - (UpdateType)updateFlags, - out failureCause); + HResult result = this.TryInvokeProjFS( + () => this.virtualizationInstance.UpdateFileIfNeeded( + relativePath, + creationTime, + lastAccessTime, + lastWriteTime, + changeTime, + fileAttributes, + endOfFile, + ConvertShaToContentId(shaContentId), + PlaceholderVersionId, + (UpdateType)updateFlags, + out failureCause), + nameof(this.UpdatePlaceholderIfNeeded), + relativePath); failureReason = (UpdateFailureReason)failureCause; return new FileSystemResult(HResultToFSResult(result), unchecked((int)result)); } @@ -211,6 +391,8 @@ public HResult StartDirectoryEnumerationCallback(int commandId, Guid enumeration { try { + this.MaybeEvictStaleEnumerations(); + List projectedItems; if (this.FileSystemCallbacks.GitIndexProjection.TryGetProjectedItemsFromMemory(virtualPath, out projectedItems)) { @@ -287,6 +469,8 @@ public HResult GetDirectoryEnumerationCallback( return HResult.InternalError; } + activeEnumeration.RecordActivity(); + if (restartScan) { activeEnumeration.RestartEnumeration(filterFileName); @@ -652,13 +836,101 @@ private bool TryCompleteCommand(int commandId, HResult result) CancellationTokenSource cancellationSource; if (this.activeCommands.TryRemove(commandId, out cancellationSource)) { - this.virtualizationInstance.CompleteCommand(commandId, result); - return true; + // Track whether the native completion actually ran. If CompleteCommand faults under + // memory pressure, TryInvokeProjFS swallows the exception so the mount survives, but + // ProjFS never accepted the completion and so will not drive the matching + // EndDirectoryEnumeration callback. In that case we must report the command as not + // completed so callers clean up any state they registered (for example the active + // enumeration) instead of leaking it. + bool completionHandedOffToProjFS = false; + this.TryInvokeProjFS( + () => + { + HResult completionResult = this.virtualizationInstance.CompleteCommand(commandId, result); + completionHandedOffToProjFS = true; + return completionResult; + }, + nameof(this.TryCompleteCommand), + addDetails: metadata => + { + metadata.Add("commandId", commandId); + metadata.Add("completionResult", result.ToString("X") + "(" + result.ToString("G") + ")"); + }); + + return completionHandedOffToProjFS; } return false; } + private void AddMemoryPressureData(EventMetadata metadata) + { + metadata.Add("activeEnumerations", this.activeEnumerations.Count); + metadata.Add("activeCommands", this.activeCommands.Count); + metadata.Add("gcTotalMemoryBytes", GC.GetTotalMemory(forceFullCollection: false)); + } + + /// + /// Invokes a GVFS-initiated native ProjFS operation, converting a native failure into a + /// failed instead of a process crash. + /// + /// + /// A native ProjFS call can fault under memory pressure (for example a NullReferenceException + /// surfaced from projectedfslib.dll). If that exception escaped, it would tear down + /// GVFS.Mount, orphaning the virtualization root and causing every subsequent placeholder + /// access to fail with STATUS_FILE_SYSTEM_VIRTUALIZATION_UNAVAILABLE (0xC000CE01). Callers + /// map the returned non-Ok HResult to a failure result and continue serving the mount. + /// + /// This is used for outbound operations that are safe to fail individually. Operations that + /// mutate projection state use instead, which logs the same + /// telemetry but rethrows. Callback completions funnel through + /// (which uses this guard); the file-stream handler already fails a single hydration on any + /// exception. + /// + private HResult TryInvokeProjFS(Func nativeCall, string operationName, string relativePath = null, Action addDetails = null) + { + try + { + return nativeCall(); + } + catch (Exception e) + { + this.LogProjFSNativeFailure(operationName, e, relativePath, addDetails); + return HResult.InternalError; + } + } + + /// + /// Invokes a GVFS-initiated native ProjFS operation for which swallowing a failure is unsafe + /// (for example because it mutates projection state). Emits the same high-signal + /// *_NativeFailure telemetry as for the memory-pressure + /// diagnostics, then rethrows so the mount fails fast rather than continuing in an + /// inconsistent state. + /// + private HResult InvokeProjFSOrThrow(Func nativeCall, string operationName, string relativePath = null, Action addDetails = null) + { + try + { + return nativeCall(); + } + catch (Exception e) + { + this.LogProjFSNativeFailure(operationName, e, relativePath, addDetails); + throw; + } + } + + private void LogProjFSNativeFailure(string operationName, Exception e, string relativePath, Action addDetails) + { + EventMetadata metadata = this.CreateEventMetadata(relativePath, e); + addDetails?.Invoke(metadata); + this.AddMemoryPressureData(metadata); + metadata.Add( + TracingConstants.MessageKey.ErrorMessage, + operationName + ": native ProjFS call threw under suspected memory pressure"); + this.Context.Tracer.RelatedError(metadata, operationName + "_NativeFailure", Keywords.Telemetry); + } + private void StartDirectoryEnumerationAsyncHandler( CancellationToken cancellationToken, BlobSizes.BlobSizesConnection blobSizesConnection, @@ -720,10 +992,12 @@ private void StartDirectoryEnumerationAsyncHandler( if (!this.TryCompleteCommand(commandId, result)) { - // Command has already been canceled, and no EndDirectoryEnumeration callback will be received + // Either the command was already canceled or the native completion failed under + // memory pressure; in both cases no EndDirectoryEnumeration callback will arrive, so + // remove the enumeration we just registered rather than leaking it. EventMetadata metadata = this.CreateEventMetadata(virtualPath); - metadata.Add(TracingConstants.MessageKey.InfoMessage, $"{nameof(this.StartDirectoryEnumerationAsyncHandler)}: TryCompleteCommand returned false, command already canceled"); + metadata.Add(TracingConstants.MessageKey.InfoMessage, $"{nameof(this.StartDirectoryEnumerationAsyncHandler)}: TryCompleteCommand returned false; command canceled or native completion failed"); metadata.Add("commandId", commandId); metadata.Add("enumerationId", enumerationId); metadata.Add(nameof(result), result.ToString("X") + "(" + result.ToString("G") + ")"); @@ -731,7 +1005,7 @@ private void StartDirectoryEnumerationAsyncHandler( ActiveEnumeration activeEnumeration; bool activeEnumerationsUpdated = this.activeEnumerations.TryRemove(enumerationId, out activeEnumeration); metadata.Add("activeEnumerationsUpdated", activeEnumerationsUpdated); - this.Context.Tracer.RelatedEvent(EventLevel.Informational, $"{nameof(this.StartDirectoryEnumerationAsyncHandler)}_CommandAlreadyCanceled", metadata); + this.Context.Tracer.RelatedEvent(EventLevel.Informational, $"{nameof(this.StartDirectoryEnumerationAsyncHandler)}_CommandNotCompleted", metadata); } } @@ -1070,10 +1344,13 @@ private void MarkDirectoryAsPlaceholder( string triggeringProcessImageFileName) { string directoryPath = Path.Combine(this.Context.Enlistment.WorkingDirectoryRoot, virtualPath); - HResult hr = this.virtualizationInstance.MarkDirectoryAsPlaceholder( - directoryPath, - FolderContentId, - PlaceholderVersionId); + HResult hr = this.InvokeProjFSOrThrow( + () => this.virtualizationInstance.MarkDirectoryAsPlaceholder( + directoryPath, + FolderContentId, + PlaceholderVersionId), + nameof(this.MarkDirectoryAsPlaceholder), + virtualPath); if (hr == HResult.Ok) { diff --git a/GVFS/GVFS.Platform.Windows/WindowsPlatform.Shared.cs b/GVFS/GVFS.Platform.Windows/WindowsPlatform.Shared.cs index 59088bc88e..766c059571 100644 --- a/GVFS/GVFS.Platform.Windows/WindowsPlatform.Shared.cs +++ b/GVFS/GVFS.Platform.Windows/WindowsPlatform.Shared.cs @@ -3,6 +3,7 @@ using System; using System.Diagnostics; using System.IO; +using System.Runtime.InteropServices; using System.Security.Principal; namespace GVFS.Platform.Windows @@ -14,6 +15,9 @@ public partial class WindowsPlatform private const int StillActive = 259; /* from Win32 STILL_ACTIVE */ + private const int ErrorAccessDenied = 5; /* ERROR_ACCESS_DENIED */ + private const int ErrorInvalidParameter = 87; /* ERROR_INVALID_PARAMETER */ + public static bool IsElevatedImplementation() { using (WindowsIdentity id = WindowsIdentity.GetCurrent()) @@ -54,15 +58,14 @@ public static bool IsProcessActiveImplementation(int processId, bool tryGetProce } /// - /// Returns true if a process with the given PID is currently active AND writes its - /// creation timestamp (raw FILETIME, 100-ns ticks since 1601) to . - /// The returned value is intended for identity comparison only -- two calls for the - /// same underlying process (no PID reuse) always yield equal values; if the OS recycles - /// a PID to a new process the value will differ. Returns false (and startTime = 0) - /// if the process is gone, has terminated (but its kernel object lingers due to an - /// outstanding handle elsewhere), or cannot be opened for QueryLimitedInformation. + /// Attempts to read a process's creation timestamp (raw FILETIME, 100-ns ticks since 1601) + /// for identity comparison. The value written to is only valid + /// when the return value is ; two calls for the + /// same underlying process yield equal values, and a recycled PID yields a different value. + /// The failure values are deliberately distinct so that the orphan-lock detector can release + /// a lock only on positive evidence (see ). /// - public static bool TryGetActiveProcessStartTimeImplementation(int processId, out long startTime) + public static ProcessStartTimeResult TryGetActiveProcessStartTimeImplementation(int processId, out long startTime) { startTime = 0; @@ -70,24 +73,52 @@ public static bool TryGetActiveProcessStartTimeImplementation(int processId, out { if (process.IsInvalid) { - return false; + // Classify the failure by the Win32 error so callers can distinguish "the holder + // is gone" from "we could not tell". OpenProcess sets last error on failure and + // the P/Invoke is declared SetLastError=true, so the value is preserved here. + int error = Marshal.GetLastWin32Error(); + switch (error) + { + case ErrorInvalidParameter: + // No process exists with this PID. + return ProcessStartTimeResult.ProcessNotFound; + + case ErrorAccessDenied: + // A process exists but we cannot open it. Because we only reach the + // identity check for holders we successfully opened at acquire time, + // and OpenProcess(QueryLimitedInformation) access is stable for a + // process's lifetime, this means the PID now refers to a different + // process than the original holder. + return ProcessStartTimeResult.Inaccessible; + + default: + // Transient/unclassified (e.g. ERROR_NOT_ENOUGH_MEMORY). We do not know + // whether the original holder is alive. + return ProcessStartTimeResult.Indeterminate; + } } // GetProcessTimes succeeds for terminated processes whose kernel object still // exists (e.g., an outstanding handle elsewhere). Confirm the process is still // running before trusting the creation time as an identity marker. - if (!NativeMethods.GetExitCodeProcess(process, out uint exitCode) || exitCode != StillActive) + if (!NativeMethods.GetExitCodeProcess(process, out uint exitCode)) + { + return ProcessStartTimeResult.Indeterminate; + } + + if (exitCode != StillActive) { - return false; + // The process was opened but has already exited. + return ProcessStartTimeResult.ProcessNotFound; } if (!NativeMethods.GetProcessTimes(process, out long creationTime, out _, out _, out _)) { - return false; + return ProcessStartTimeResult.Indeterminate; } startTime = creationTime; - return true; + return ProcessStartTimeResult.Success; } } diff --git a/GVFS/GVFS.Platform.Windows/WindowsPlatform.cs b/GVFS/GVFS.Platform.Windows/WindowsPlatform.cs index 37949e6d61..91da42f767 100644 --- a/GVFS/GVFS.Platform.Windows/WindowsPlatform.cs +++ b/GVFS/GVFS.Platform.Windows/WindowsPlatform.cs @@ -24,6 +24,10 @@ public partial class WindowsPlatform : GVFSPlatform private const string BuildLabRegistryValue = "BuildLab"; private const string BuildLabExRegistryValue = "BuildLabEx"; + private const string ProfileListRegistryKey = "SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion\\ProfileList"; + private const string ProfileImagePathRegistryValue = "ProfileImagePath"; + private const string AdminProtectionProfilePrefix = "ADMIN_"; + public WindowsPlatform() : base(underConstruction: new UnderConstructionFlags()) { } @@ -76,6 +80,46 @@ public static object GetValueFromRegistry(RegistryHive registryHive, string key, return value; } + /// + /// Detects whether the current process token belongs to a Windows "Administrator protection" (AP) + /// shadow admin account. + /// + /// Under AP, elevating produces a token whose user is a hidden, system-managed local account + /// (MACHINE\admin_<user>, SID S-1-5-21-...) with its own user profile named ADMIN_<user>. That + /// account's SID differs from the real (interactive) user's SID, so treating it as "the current user" + /// for file-ownership purposes is wrong. + /// + /// This inspects the running process's own token (its effective elevation identity) rather than the + /// TypeOfAdminApprovalMode policy value, so it is correct even across an AP policy change that has not + /// yet been rebooted (where the policy value is pending but elevation still yields a shadow admin). + /// + public static bool IsCurrentUserAdminProtectionShadowAccount() + { + using (WindowsIdentity currentUser = WindowsIdentity.GetCurrent()) + { + SecurityIdentifier userSid = currentUser.User; + + // AP shadow accounts are local machine accounts (S-1-5-21-...); the real interactive user is a + // domain or Entra ID account (e.g. S-1-12-1-...), which is not an "account SID" in this sense. + if (userSid == null || !userSid.IsAccountSid()) + { + return false; + } + + string profilePath = GetStringFromRegistry( + $"{ProfileListRegistryKey}\\{userSid.Value}", + ProfileImagePathRegistryValue); + if (string.IsNullOrEmpty(profilePath)) + { + return false; + } + + string profileFolderName = Path.GetFileName( + profilePath.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar)); + return profileFolderName.StartsWith(AdminProtectionProfilePrefix, StringComparison.OrdinalIgnoreCase); + } + } + public static bool TrySetDWordInRegistry(RegistryHive registryHive, string key, string valueName, uint value) { RegistryKey localKey = RegistryKey.OpenBaseKey(registryHive, RegistryView.Registry64); @@ -211,7 +255,7 @@ public override bool IsProcessActive(int processId) return WindowsPlatform.IsProcessActiveImplementation(processId, tryGetProcessById: true); } - public override bool TryGetActiveProcessStartTime(int processId, out long startTime) + public override ProcessStartTimeResult TryGetActiveProcessStartTime(int processId, out long startTime) { return WindowsPlatform.TryGetActiveProcessStartTimeImplementation(processId, out startTime); } diff --git a/GVFS/GVFS.Tests/NUnitRunner.cs b/GVFS/GVFS.Tests/NUnitRunner.cs index 701aac4d4e..34d30f83bb 100644 --- a/GVFS/GVFS.Tests/NUnitRunner.cs +++ b/GVFS/GVFS.Tests/NUnitRunner.cs @@ -98,16 +98,29 @@ public void PrepareTestSlice(string filters, (uint, uint) testSlice) } // Now distribute the tests into the buckets. - // Tests from the same fixture class must stay in the same bucket - // when the fixture shares a single enlistment across tests (both - // EnlistmentPerFixture classes, MultiEnlistmentTests with [Order], - // and GitCommands fixture classes like GitCommandsTests, CheckoutTests, - // etc. use a shared enlistment). - // The regex captures "everything up to and including the class name" - // so that SomeClass.TestA and SomeClass.TestB share a prefix. + // + // Some test classes share state across test methods and must land in the + // same slice. The regex captures a per-class prefix so that all methods of + // those classes are grouped together. + // + // Classes that MUST stay together: + // EnlistmentPerFixture.* — share a single mounted enlistment for the + // whole fixture; tests may be order-dependent. + // MultiEnlistmentTests.ConfigVerbTests — uses [Order(1..8)]; each test + // reads/writes GVFS config set by the previous. + // + // Classes that are safe to split (do NOT add to this regex): + // GitCommands.* — each test resets state via git-checkout in [SetUp] + // or gets a fresh per-test enlistment. + // SharedCacheTests — [SetUp] generates a new Guid-based cache path per + // test; no shared state across test methods. + // ServiceVerbTests — [NonParallelizable] prevents within-process + // concurrency; different slices run on different + // machines so cross-slice isolation is guaranteed. Regex fixtureRegex = new Regex( - @"^.*\.(?:EnlistmentPerFixture|MultiEnlistmentTests|GitCommands)\..+\.", + @"^.*\.(?:EnlistmentPerFixture\..+|MultiEnlistmentTests\.ConfigVerbTests)\.", RegexOptions.CultureInvariant | RegexOptions.IgnoreCase); + for (uint i = 0; i < list.Length; i++) { var test = list[i].Trim(); diff --git a/GVFS/GVFS.UnitTests/Common/AzDevOpsOrgFromNuGetFeedTests.cs b/GVFS/GVFS.UnitTests/Common/AzDevOpsOrgFromNuGetFeedTests.cs deleted file mode 100644 index 74dbdbb667..0000000000 --- a/GVFS/GVFS.UnitTests/Common/AzDevOpsOrgFromNuGetFeedTests.cs +++ /dev/null @@ -1,35 +0,0 @@ -using GVFS.Common; -using GVFS.Tests.Should; -using NUnit.Framework; - -namespace GVFS.UnitTests.Common -{ - [TestFixture] - public class NuGetUpgraderTests - { - [TestCase("https://pkgs.dev.azure.com/test-pat/_packaging/Test-GVFS-Installers-Custom/nuget/v3/index.json", "https://test-pat.visualstudio.com")] - [TestCase("https://PKGS.DEV.azure.com/test-pat/_packaging/Test-GVFS-Installers-Custom/nuget/v3/index.json", "https://test-pat.visualstudio.com")] - [TestCase("https://dev.azure.com/test-pat/_packaging/Test-GVFS-Installers-Custom/nuget/v3/index.json", null)] - [TestCase("http://pkgs.dev.azure.com/test-pat/_packaging/Test-GVFS-Installers-Custom/nuget/v3/index.json", null)] - public void CanConstructAzureDevOpsUrlFromPackageFeedUrl(string packageFeedUrl, string expectedAzureDevOpsUrl) - { - bool success = AzDevOpsOrgFromNuGetFeed.TryCreateCredentialQueryUrl( - packageFeedUrl, - out string azureDevOpsUrl, - out string error); - - if (expectedAzureDevOpsUrl != null) - { - success.ShouldBeTrue(); - azureDevOpsUrl.ShouldEqual(expectedAzureDevOpsUrl); - error.ShouldBeNull(); - } - else - { - success.ShouldBeFalse(); - azureDevOpsUrl.ShouldBeNull(); - error.ShouldNotBeNull(); - } - } - } -} diff --git a/GVFS/GVFS.UnitTests/Common/GVFSLockTests.cs b/GVFS/GVFS.UnitTests/Common/GVFSLockTests.cs index 91173fd7b6..3d18fd15c7 100644 --- a/GVFS/GVFS.UnitTests/Common/GVFSLockTests.cs +++ b/GVFS/GVFS.UnitTests/Common/GVFSLockTests.cs @@ -260,6 +260,125 @@ public void TryAcquireLockForExternalRequestor_WhenHolderStartTimeMatches() mockTracer.VerifyAll(); } + /// + /// When the start-time read returns positive evidence that the holder is gone + /// (ProcessNotFound), the orphaned lock must be released. + /// + [TestCase] + public void TryAcquireLockForExternalRequestor_WhenHolderProcessNotFound() + { + Mock mockTracer = new Mock(MockBehavior.Strict); + mockTracer.Setup(x => x.RelatedEvent(EventLevel.Informational, "TryAcquireLockExternal", It.IsAny())); + mockTracer.Setup(x => x.RelatedEvent(EventLevel.Informational, "ExternalLockHolderExited", It.IsAny(), Keywords.Telemetry)); + mockTracer.Setup(x => x.SetGitCommandSessionId(string.Empty)); + MockPlatform mockPlatform = (MockPlatform)GVFSPlatform.Instance; + + mockPlatform.ActiveProcesses.Add(DefaultLockData.PID); + mockPlatform.ProcessStartTimes[DefaultLockData.PID] = 1000; + GVFSLock gvfsLock = new GVFSLock(mockTracer.Object); + NamedPipeMessages.LockData existingExternalHolder; + gvfsLock.TryAcquireLockForExternalRequestor(DefaultLockData, out existingExternalHolder).ShouldBeTrue(); + this.ValidateLockHeld(gvfsLock, DefaultLockData); + + // Positive evidence the holder is gone. + mockPlatform.ProcessStartTimeResults[DefaultLockData.PID] = ProcessStartTimeResult.ProcessNotFound; + + NamedPipeMessages.LockData newLockData = new NamedPipeMessages.LockData(4321, false, false, "git new", "456"); + mockPlatform.ActiveProcesses.Add(newLockData.PID); + mockPlatform.ProcessStartTimes[newLockData.PID] = 3000; + gvfsLock.TryAcquireLockForExternalRequestor(newLockData, out existingExternalHolder).ShouldBeTrue(); + existingExternalHolder.ShouldBeNull(); + this.ValidateLockHeld(gvfsLock, newLockData); + + mockPlatform.ActiveProcesses.Remove(DefaultLockData.PID); + mockPlatform.ActiveProcesses.Remove(newLockData.PID); + mockPlatform.ProcessStartTimes.Remove(DefaultLockData.PID); + mockPlatform.ProcessStartTimes.Remove(newLockData.PID); + mockPlatform.ProcessStartTimeResults.Remove(DefaultLockData.PID); + mockTracer.VerifyAll(); + } + + /// + /// When the holder's PID is now occupied by a process we cannot open (Inaccessible), the + /// original holder must be gone: we successfully opened it at acquire time, and that access + /// is stable for a process's lifetime, so an access-denied result means the PID was recycled + /// to a different process. The orphaned lock must be released. + /// + [TestCase] + public void TryAcquireLockForExternalRequestor_WhenHolderInaccessible() + { + Mock mockTracer = new Mock(MockBehavior.Strict); + mockTracer.Setup(x => x.RelatedEvent(EventLevel.Informational, "TryAcquireLockExternal", It.IsAny())); + mockTracer.Setup(x => x.RelatedEvent(EventLevel.Informational, "ExternalLockHolderExited", It.IsAny(), Keywords.Telemetry)); + mockTracer.Setup(x => x.SetGitCommandSessionId(string.Empty)); + MockPlatform mockPlatform = (MockPlatform)GVFSPlatform.Instance; + + mockPlatform.ActiveProcesses.Add(DefaultLockData.PID); + mockPlatform.ProcessStartTimes[DefaultLockData.PID] = 1000; + GVFSLock gvfsLock = new GVFSLock(mockTracer.Object); + NamedPipeMessages.LockData existingExternalHolder; + gvfsLock.TryAcquireLockForExternalRequestor(DefaultLockData, out existingExternalHolder).ShouldBeTrue(); + this.ValidateLockHeld(gvfsLock, DefaultLockData); + + // The PID is now occupied by a process we cannot open. + mockPlatform.ProcessStartTimeResults[DefaultLockData.PID] = ProcessStartTimeResult.Inaccessible; + + NamedPipeMessages.LockData newLockData = new NamedPipeMessages.LockData(4321, false, false, "git new", "456"); + mockPlatform.ActiveProcesses.Add(newLockData.PID); + mockPlatform.ProcessStartTimes[newLockData.PID] = 3000; + gvfsLock.TryAcquireLockForExternalRequestor(newLockData, out existingExternalHolder).ShouldBeTrue(); + existingExternalHolder.ShouldBeNull(); + this.ValidateLockHeld(gvfsLock, newLockData); + + mockPlatform.ActiveProcesses.Remove(DefaultLockData.PID); + mockPlatform.ActiveProcesses.Remove(newLockData.PID); + mockPlatform.ProcessStartTimes.Remove(DefaultLockData.PID); + mockPlatform.ProcessStartTimes.Remove(newLockData.PID); + mockPlatform.ProcessStartTimeResults.Remove(DefaultLockData.PID); + mockTracer.VerifyAll(); + } + + /// + /// Anti-false-release guard: when the start-time read is Indeterminate (e.g. a transient + /// resource failure), we do NOT know whether the holder is alive, so the lock must be KEPT. + /// Releasing here could hand the lock to a second writer while the original git process is + /// still running -- the worst-case regression this hardening prevents. + /// + [TestCase] + public void TryAcquireLockForExternalRequestor_WhenHolderIndeterminateKeepsLock() + { + Mock mockTracer = new Mock(MockBehavior.Strict); + mockTracer.Setup(x => x.RelatedEvent(EventLevel.Informational, "TryAcquireLockExternal", It.IsAny())); + mockTracer.Setup(x => x.RelatedEvent(EventLevel.Verbose, "TryAcquireLockExternal", It.IsAny())); + mockTracer.Setup(x => x.RelatedEvent(EventLevel.Verbose, "ExternalHolderLivenessIndeterminate", It.IsAny())); + MockPlatform mockPlatform = (MockPlatform)GVFSPlatform.Instance; + + mockPlatform.ActiveProcesses.Add(DefaultLockData.PID); + mockPlatform.ProcessStartTimes[DefaultLockData.PID] = 1000; + GVFSLock gvfsLock = new GVFSLock(mockTracer.Object); + NamedPipeMessages.LockData existingExternalHolder; + gvfsLock.TryAcquireLockForExternalRequestor(DefaultLockData, out existingExternalHolder).ShouldBeTrue(); + this.ValidateLockHeld(gvfsLock, DefaultLockData); + + // We cannot determine whether the holder is alive. + mockPlatform.ProcessStartTimeResults[DefaultLockData.PID] = ProcessStartTimeResult.Indeterminate; + + // A new requestor must be denied -- the lock is still held by the (possibly-alive) holder. + NamedPipeMessages.LockData newLockData = new NamedPipeMessages.LockData(4321, false, false, "git new", "456"); + mockPlatform.ActiveProcesses.Add(newLockData.PID); + mockPlatform.ProcessStartTimes[newLockData.PID] = 3000; + gvfsLock.TryAcquireLockForExternalRequestor(newLockData, out existingExternalHolder).ShouldBeFalse(); + this.ValidateExistingExternalHolder(DefaultLockData, existingExternalHolder); + this.ValidateLockHeld(gvfsLock, DefaultLockData); + + mockPlatform.ActiveProcesses.Remove(DefaultLockData.PID); + mockPlatform.ActiveProcesses.Remove(newLockData.PID); + mockPlatform.ProcessStartTimes.Remove(DefaultLockData.PID); + mockPlatform.ProcessStartTimes.Remove(newLockData.PID); + mockPlatform.ProcessStartTimeResults.Remove(DefaultLockData.PID); + mockTracer.VerifyAll(); + } + private GVFSLock AcquireDefaultLock(MockPlatform mockPlatform, ITracer mockTracer) { GVFSLock gvfsLock = new GVFSLock(mockTracer); diff --git a/GVFS/GVFS.UnitTests/Common/OrgInfoApiClientTests.cs b/GVFS/GVFS.UnitTests/Common/OrgInfoApiClientTests.cs deleted file mode 100644 index ced645d081..0000000000 --- a/GVFS/GVFS.UnitTests/Common/OrgInfoApiClientTests.cs +++ /dev/null @@ -1,110 +0,0 @@ -using GVFS.Common; -using GVFS.Tests.Should; -using Moq; -using Moq.Protected; -using NUnit.Framework; -using System; -using System.Collections.Generic; -using System.Net; -using System.Net.Http; -using System.Threading; -using System.Threading.Tasks; - -namespace GVFS.UnitTests.Common -{ - [TestFixture] - public class OrgInfoServerTests - { - public static List TestOrgInfo = new List() - { - new OrgInfo() { OrgName = "org1", Platform = "windows", Ring = "fast", Version = "1.2.3.1" }, - new OrgInfo() { OrgName = "org1", Platform = "windows", Ring = "slow", Version = "1.2.3.2" }, - new OrgInfo() { OrgName = "org1", Platform = "macOS", Ring = "fast", Version = "1.2.3.3" }, - new OrgInfo() { OrgName = "org1", Platform = "macOS", Ring = "slow", Version = "1.2.3.4" }, - new OrgInfo() { OrgName = "org2", Platform = "windows", Ring = "fast", Version = "1.2.3.5" }, - new OrgInfo() { OrgName = "org2", Platform = "windows", Ring = "slow", Version = "1.2.3.6" }, - new OrgInfo() { OrgName = "org2", Platform = "macOS", Ring = "fast", Version = "1.2.3.7" }, - new OrgInfo() { OrgName = "org2", Platform = "macOS", Ring = "slow", Version = "1.2.3.8" }, - }; - - private string baseUrl = "https://www.contoso.com"; - - private interface IHttpMessageHandlerProtectedMembers - { - Task SendAsync(HttpRequestMessage message, CancellationToken token); - } - - [TestCaseSource("TestOrgInfo")] - public void QueryNewestVersionWithParams(OrgInfo orgInfo) - { - Mock handlerMock = new Mock(MockBehavior.Strict); - - handlerMock.Protected().As() - .Setup(m => m.SendAsync(It.Is(request => this.UriMatches(request.RequestUri, this.baseUrl, orgInfo.OrgName, orgInfo.Platform, orgInfo.Ring)), It.IsAny())) - .ReturnsAsync(new HttpResponseMessage() - { - StatusCode = HttpStatusCode.OK, - Content = new StringContent(this.ConstructResponseContent(orgInfo.Version)) - }); - - HttpClient httpClient = new HttpClient(handlerMock.Object); - - OrgInfoApiClient upgradeChecker = new OrgInfoApiClient(httpClient, this.baseUrl); - Version version = upgradeChecker.QueryNewestVersion(orgInfo.OrgName, orgInfo.Platform, orgInfo.Ring); - - version.ShouldEqual(new Version(orgInfo.Version)); - - handlerMock.VerifyAll(); - } - - private bool UriMatches(Uri uri, string baseUrl, string expectedOrgName, string expectedPlatform, string expectedRing) - { - bool hostMatches = uri.Host.Equals(baseUrl); - - Dictionary queryParams = new Dictionary(StringComparer.OrdinalIgnoreCase); - foreach (string param in uri.Query.Substring(1).Split('&')) - { - string[] fields = param.Split('='); - string key = fields[0]; - string value = fields[1]; - - queryParams.Add(key, value); - } - - if (queryParams.Count != 3) - { - return false; - } - - if (!queryParams.TryGetValue("Organization", out string orgName) || !string.Equals(orgName, expectedOrgName, StringComparison.OrdinalIgnoreCase)) - { - return false; - } - - if (!queryParams.TryGetValue("platform", out string platform) || !string.Equals(platform, expectedPlatform, StringComparison.OrdinalIgnoreCase)) - { - return false; - } - - if (!queryParams.TryGetValue("ring", out string ring) || !string.Equals(ring, expectedRing, StringComparison.OrdinalIgnoreCase)) - { - return false; - } - - return true; - } - - private string ConstructResponseContent(string version) - { - return $"{{\"version\" : \"{version}\"}} "; - } - - public class OrgInfo - { - public string OrgName { get; set; } - public string Ring { get; set; } - public string Platform { get; set; } - public string Version { get; set; } - } - } -} diff --git a/GVFS/GVFS.UnitTests/Git/GitAuthenticationTests.cs b/GVFS/GVFS.UnitTests/Git/GitAuthenticationTests.cs index c083ac5862..25aa675b7e 100644 --- a/GVFS/GVFS.UnitTests/Git/GitAuthenticationTests.cs +++ b/GVFS/GVFS.UnitTests/Git/GitAuthenticationTests.cs @@ -1,5 +1,7 @@ using System; using System.Linq; +using System.Threading; +using System.Threading.Tasks; using GVFS.Common.Git; using GVFS.Tests; using GVFS.Tests.Should; @@ -273,6 +275,73 @@ public void RejectionShouldNotBeSentIfUnderlyingTokenHasChanged() gitProcess.CredentialRejections.ShouldBeEmpty(); } + [TestCase] + public void TryGetCredentialsBeforeInitializationDoesNotThrow() + { + // Regression test for a mount crash: when a cache server is configured, + // mount starts virtualization before the background auth/config query + // finishes initializing. A directory enumeration then calls + // TryGetCredentials while IsAnonymous has already been set false but + // initialization has not completed. The previous behavior threw an + // InvalidOperationException straight into the ProjFS callback, crashing + // the mount process. It must instead fail gracefully (retryable). + MockTracer tracer = new MockTracer(); + MockGitProcess gitProcess = this.GetGitProcess(); + + GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); + + // Keep the wait short so an uninitialized instance reports failure + // quickly instead of blocking for the full background timeout. + dut.InitializationWaitTimeoutMs = 10; + + string authString = null; + string error = null; + bool result = true; + + Assert.DoesNotThrow(() => result = dut.TryGetCredentials(tracer, out authString, out error)); + + result.ShouldBeFalse("TryGetCredentials should fail (not throw) before initialization completes"); + error.ShouldNotBeNull("A retryable error message should be returned"); + } + + [TestCase] + public void TryGetCredentialsWaitsForBackgroundInitializationThenSucceeds() + { + // A caller that arrives before initialization completes should block + // until initialization finishes, then return the fetched credentials - + // this is the correct behavior for the background cache-server auth path. + MockTracer tracer = new MockTracer(); + MockGitProcess gitProcess = this.GetGitProcess(); + + GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); + dut.InitializationWaitTimeoutMs = 30_000; + + string authString = null; + string error = null; + bool result = false; + + using (ManualResetEventSlim consumerStarted = new ManualResetEventSlim(false)) + { + Task consumer = Task.Run(() => + { + consumerStarted.Set(); + result = dut.TryGetCredentials(tracer, out authString, out error); + }); + + // Ensure the consumer has begun waiting on initialization before we + // complete it, so we exercise the wait path rather than a no-op. + consumerStarted.Wait(); + Thread.Sleep(50); + + dut.TryInitializeAndRequireAuth(tracer, out _); + + consumer.Wait(TimeSpan.FromSeconds(10)).ShouldBeTrue("Consumer should unblock once initialization completes"); + } + + result.ShouldBeTrue("TryGetCredentials should succeed after initialization: " + error); + authString.ShouldNotBeNull("A credential string should be returned"); + } + private MockGitProcess GetGitProcess() { MockGitProcess gitProcess = new MockGitProcess(); diff --git a/GVFS/GVFS.UnitTests/Git/GitProcessTests.cs b/GVFS/GVFS.UnitTests/Git/GitProcessTests.cs index 74ddde8ee8..8182f8dfb5 100644 --- a/GVFS/GVFS.UnitTests/Git/GitProcessTests.cs +++ b/GVFS/GVFS.UnitTests/Git/GitProcessTests.cs @@ -9,6 +9,88 @@ namespace GVFS.UnitTests.Git [TestFixture] public class GitProcessTests { + [TestCase] + public void BoundedGitOutputBuffer_KeepsShortOutput() + { + GitProcess.BoundedGitOutputBuffer buffer = new GitProcess.BoundedGitOutputBuffer(1000); + buffer.AppendLine("line one"); + buffer.AppendLine("line two"); + + buffer.Truncated.ShouldBeFalse(); + buffer.ToString().ShouldEqual("line one\nline two\n"); + } + + [TestCase] + public void BoundedGitOutputBuffer_TruncatesBeyondCapWithoutUnboundedGrowth() + { + // Simulate a flood of output (e.g. 'multi-pack-index write' spewing "could not load pack" + // against a corrupt packfile). Before the cap this grew an unbounded StringBuilder until + // GVFS.Mount hit OutOfMemoryException. + const int Cap = 4096; + GitProcess.BoundedGitOutputBuffer buffer = new GitProcess.BoundedGitOutputBuffer(Cap); + + const string NoisyLine = "error: could not load pack 2"; + for (int i = 0; i < 1_000_000; i++) + { + buffer.AppendLine(NoisyLine); + } + + buffer.Truncated.ShouldBeTrue(); + + string result = buffer.ToString(); + + // We appended ~28 million characters; the buffer must stay bounded near the cap + // (cap + a single one-time truncation marker), nowhere near the raw input size. + (result.Length < Cap + 200).ShouldBeTrue("Buffer grew beyond its cap: " + result.Length); + result.Contains("truncated").ShouldBeTrue(); + } + + [TestCase] + public void BoundedGitOutputBuffer_KeepsPartialLineThatCrossesCap() + { + GitProcess.BoundedGitOutputBuffer buffer = new GitProcess.BoundedGitOutputBuffer(10); + buffer.AppendLine("123456789012345"); + + buffer.Truncated.ShouldBeTrue(); + string result = buffer.ToString(); + + // The portion of the line that fit under the cap is retained, then the marker. + result.StartsWith("1234567890").ShouldBeTrue(); + result.Contains("truncated").ShouldBeTrue(); + } + + [TestCase] + public void BoundedGitOutputBuffer_ExactlyAtCapIsNotTruncated() + { + // "abcd" + "\n" == 5 chars == cap; must fit without tripping truncation. + GitProcess.BoundedGitOutputBuffer buffer = new GitProcess.BoundedGitOutputBuffer(5); + buffer.AppendLine("abcd"); + + buffer.Truncated.ShouldBeFalse(); + buffer.ToString().ShouldEqual("abcd\n"); + } + + [TestCase] + public void Result_TruncationFlagsDefaultToFalse() + { + GitProcess.Result result = new GitProcess.Result("out", "err", 0); + + result.OutputTruncated.ShouldBeFalse(); + result.ErrorsTruncated.ShouldBeFalse(); + } + + [TestCase] + public void Result_TruncationFlagsRoundTripThroughConstructor() + { + GitProcess.Result outputTruncated = new GitProcess.Result("out", "err", 0, outputTruncated: true, errorsTruncated: false); + outputTruncated.OutputTruncated.ShouldBeTrue(); + outputTruncated.ErrorsTruncated.ShouldBeFalse(); + + GitProcess.Result errorsTruncated = new GitProcess.Result("out", "err", 0, outputTruncated: false, errorsTruncated: true); + errorsTruncated.OutputTruncated.ShouldBeFalse(); + errorsTruncated.ErrorsTruncated.ShouldBeTrue(); + } + [TestCase] public void TryKillRunningProcess_NeverRan() { diff --git a/GVFS/GVFS.UnitTests/Mock/Common/MockLocalGVFSConfigBuilder.cs b/GVFS/GVFS.UnitTests/Mock/Common/MockLocalGVFSConfigBuilder.cs deleted file mode 100644 index 469067513b..0000000000 --- a/GVFS/GVFS.UnitTests/Mock/Common/MockLocalGVFSConfigBuilder.cs +++ /dev/null @@ -1,91 +0,0 @@ -using GVFS.Common; -using System.Collections.Generic; - -namespace GVFS.UnitTests.Mock.Common -{ - public class MockLocalGVFSConfigBuilder - { - private string defaultRing; - private string defaultUpgradeFeedUrl; - private string defaultUpgradeFeedPackageName; - private string defaultOrgServerUrl; - - private Dictionary entries; - - public MockLocalGVFSConfigBuilder( - string defaultRing, - string defaultUpgradeFeedUrl, - string defaultUpgradeFeedPackageName, - string defaultOrgServerUrl) - { - this.defaultRing = defaultRing; - this.defaultUpgradeFeedUrl = defaultUpgradeFeedUrl; - this.defaultUpgradeFeedPackageName = defaultUpgradeFeedPackageName; - this.defaultOrgServerUrl = defaultOrgServerUrl; - this.entries = new Dictionary(); - } - - public MockLocalGVFSConfigBuilder WithUpgradeRing(string value = null) - { - return this.With(GVFSConstants.LocalGVFSConfig.UpgradeRing, value ?? this.defaultRing); - } - - public MockLocalGVFSConfigBuilder WithNoUpgradeRing() - { - return this.WithNo(GVFSConstants.LocalGVFSConfig.UpgradeRing); - } - - public MockLocalGVFSConfigBuilder WithUpgradeFeedPackageName(string value = null) - { - return this.With(GVFSConstants.LocalGVFSConfig.UpgradeFeedPackageName, value ?? this.defaultUpgradeFeedPackageName); - } - - public MockLocalGVFSConfigBuilder WithNoUpgradeFeedPackageName() - { - return this.WithNo(GVFSConstants.LocalGVFSConfig.UpgradeFeedPackageName); - } - - public MockLocalGVFSConfigBuilder WithUpgradeFeedUrl(string value = null) - { - return this.With(GVFSConstants.LocalGVFSConfig.UpgradeFeedUrl, value ?? this.defaultUpgradeFeedUrl); - } - - public MockLocalGVFSConfigBuilder WithNoUpgradeFeedUrl() - { - return this.WithNo(GVFSConstants.LocalGVFSConfig.UpgradeFeedUrl); - } - - public MockLocalGVFSConfigBuilder WithOrgInfoServerUrl(string value = null) - { - return this.With(GVFSConstants.LocalGVFSConfig.OrgInfoServerUrl, value ?? this.defaultUpgradeFeedUrl); - } - - public MockLocalGVFSConfigBuilder WithNoOrgInfoServerUrl() - { - return this.WithNo(GVFSConstants.LocalGVFSConfig.OrgInfoServerUrl); - } - - public MockLocalGVFSConfig Build() - { - MockLocalGVFSConfig gvfsConfig = new MockLocalGVFSConfig(); - foreach (KeyValuePair kvp in this.entries) - { - gvfsConfig.TrySetConfig(kvp.Key, kvp.Value, out _); - } - - return gvfsConfig; - } - - private MockLocalGVFSConfigBuilder With(string key, string value) - { - this.entries.Add(key, value); - return this; - } - - private MockLocalGVFSConfigBuilder WithNo(string key) - { - this.entries.Remove(key); - return this; - } - } -} diff --git a/GVFS/GVFS.UnitTests/Mock/Common/MockPlatform.cs b/GVFS/GVFS.UnitTests/Mock/Common/MockPlatform.cs index e7dca80cc5..3ee7425de0 100644 --- a/GVFS/GVFS.UnitTests/Mock/Common/MockPlatform.cs +++ b/GVFS/GVFS.UnitTests/Mock/Common/MockPlatform.cs @@ -53,6 +53,13 @@ public override bool SupportsSystemInstallLog /// public Dictionary ProcessStartTimes { get; } = new Dictionary(); + /// + /// Optional per-PID result overrides for , used by + /// tests that exercise the non-Success outcomes (Inaccessible / Indeterminate). When a PID is + /// present here, the specified result is returned regardless of . + /// + public Dictionary ProcessStartTimeResults { get; } = new Dictionary(); + public override void ConfigureVisualStudio(string gitBinPath, ITracer tracer) { throw new NotSupportedException(); @@ -141,19 +148,31 @@ public override bool IsProcessActive(int processId) return this.ActiveProcesses.Contains(processId); } - public override bool TryGetActiveProcessStartTime(int processId, out long startTime) + public override ProcessStartTimeResult TryGetActiveProcessStartTime(int processId, out long startTime) { + startTime = 0; + + // Explicit result override wins, so tests can exercise Inaccessible / Indeterminate. + if (this.ProcessStartTimeResults.TryGetValue(processId, out ProcessStartTimeResult overrideResult)) + { + if (overrideResult == ProcessStartTimeResult.Success) + { + startTime = this.ProcessStartTimes.TryGetValue(processId, out long overrideTime) ? overrideTime : processId; + } + + return overrideResult; + } + if (this.ActiveProcesses.Contains(processId)) { // If the test populated an explicit start time use that, otherwise fall back // to a deterministic non-zero value so test scenarios that don't care about // PID identity (the existing majority of unit tests) keep working. startTime = this.ProcessStartTimes.TryGetValue(processId, out long stored) ? stored : processId; - return true; + return ProcessStartTimeResult.Success; } - startTime = 0; - return false; + return ProcessStartTimeResult.ProcessNotFound; } public override void IsServiceInstalledAndRunning(string name, out bool installed, out bool running) diff --git a/GVFS/GVFS.UnitTests/Virtualization/FileSystemCallbacksTests.cs b/GVFS/GVFS.UnitTests/Virtualization/FileSystemCallbacksTests.cs index 6ebd817fdb..3a515c3b51 100644 --- a/GVFS/GVFS.UnitTests/Virtualization/FileSystemCallbacksTests.cs +++ b/GVFS/GVFS.UnitTests/Virtualization/FileSystemCallbacksTests.cs @@ -34,7 +34,7 @@ public void IsPathInsideDotGitIsTrueForDotGitPath() { FileSystemCallbacks.IsPathInsideDotGit(@".git" + Path.DirectorySeparatorChar).ShouldEqual(true); FileSystemCallbacks.IsPathInsideDotGit(Path.Combine(".git", "test_file.txt")).ShouldEqual(true); - FileSystemCallbacks.IsPathInsideDotGit(Path.Combine(".git", "test_folder", "test_file.txt")).ShouldEqual(true); + FileSystemCallbacks.IsPathInsideDotGit(Path.Combine(".git", "test_folder", "test_file.txt")).ShouldEqual(true); } [TestCase] diff --git a/GVFS/GVFS.UnitTests/Windows/Mock/MockVirtualizationInstance.cs b/GVFS/GVFS.UnitTests/Windows/Mock/MockVirtualizationInstance.cs index 16659dddab..95147d9bbc 100644 --- a/GVFS/GVFS.UnitTests/Windows/Mock/MockVirtualizationInstance.cs +++ b/GVFS/GVFS.UnitTests/Windows/Mock/MockVirtualizationInstance.cs @@ -62,6 +62,18 @@ public HResult WriteFileReturnResult public HResult DeleteFileResult { get; set; } public UpdateFailureCause DeleteFileUpdateFailureCause { get; set; } + // When set, DeleteFile throws this to simulate a native ProjFS failure (e.g. a + // NullReferenceException surfaced from projectedfslib.dll under memory pressure). + public Exception DeleteFileException { get; set; } + + // When set, the other GVFS-initiated native operations (ClearNegativePathCache, + // WritePlaceholderInfo, UpdateFileIfNeeded) throw this to simulate a native ProjFS failure. + public Exception NativeCallException { get; set; } + + // When set, CompleteCommand throws this to simulate the native ProjFS completion faulting + // under memory pressure (e.g. a NullReferenceException surfaced from projectedfslib.dll). + public Exception CompleteCommandException { get; set; } + public HResult UpdateFileIfNeededResult { get; set; } public UpdateFailureCause UpdateFileIfNeededFailureCase { get; set; } @@ -82,6 +94,11 @@ public HResult DetachDriver() public HResult ClearNegativePathCache(out uint totalEntryNumber) { + if (this.NativeCallException != null) + { + throw this.NativeCallException; + } + totalEntryNumber = this.NegativePathCacheCount; this.NegativePathCacheCount = 0; return HResult.Ok; @@ -89,12 +106,22 @@ public HResult ClearNegativePathCache(out uint totalEntryNumber) public HResult DeleteFile(string relativePath, UpdateType updateFlags, out UpdateFailureCause failureReason) { + if (this.DeleteFileException != null) + { + throw this.DeleteFileException; + } + failureReason = this.DeleteFileUpdateFailureCause; return this.DeleteFileResult; } public HResult UpdateFileIfNeeded(string relativePath, DateTime creationTime, DateTime lastAccessTime, DateTime lastWriteTime, DateTime changeTime, FileAttributes fileAttributes, long endOfFile, byte[] contentId, byte[] providerId, UpdateType updateFlags, out UpdateFailureCause failureReason) { + if (this.NativeCallException != null) + { + throw this.NativeCallException; + } + failureReason = this.UpdateFileIfNeededFailureCase; return this.UpdateFileIfNeededResult; } @@ -121,6 +148,11 @@ public HResult WritePlaceholderInfo( byte[] contentId, byte[] epochId) { + if (this.NativeCallException != null) + { + throw this.NativeCallException; + } + this.CreatedPlaceholders.Add(relativePath); this.placeholderCreated.Set(); return HResult.Ok; @@ -169,6 +201,11 @@ public HResult CompleteCommand(int commandId, IDirectoryEnumerationResults resul public HResult CompleteCommand(int commandId, HResult completionResult) { + if (this.CompleteCommandException != null) + { + throw this.CompleteCommandException; + } + this.completionResult = completionResult; this.commandCompleted.Set(); return HResult.Ok; diff --git a/GVFS/GVFS.UnitTests/Windows/Virtualization/ActiveEnumerationTests.cs b/GVFS/GVFS.UnitTests/Windows/Virtualization/ActiveEnumerationTests.cs index 6770d2e1a8..e914a00f84 100644 --- a/GVFS/GVFS.UnitTests/Windows/Virtualization/ActiveEnumerationTests.cs +++ b/GVFS/GVFS.UnitTests/Windows/Virtualization/ActiveEnumerationTests.cs @@ -45,6 +45,20 @@ public void EnumerationHandlesEmptyList() activeEnumeration.Current.ShouldEqual(null); } + [TestCase] + public void RecordActivityUpdatesLastActivityTime() + { + long beforeCreate = Environment.TickCount64; + ActiveEnumeration activeEnumeration = CreateActiveEnumeration(new List()); + + // The constructor records initial activity. + (activeEnumeration.LastActivityTickCount >= beforeCreate).ShouldBeTrue(); + + long beforeRecord = Environment.TickCount64; + activeEnumeration.RecordActivity(); + (activeEnumeration.LastActivityTickCount >= beforeRecord).ShouldBeTrue(); + } + [TestCase] public void EnumerateSingleEntryList() { diff --git a/GVFS/GVFS.UnitTests/Windows/Virtualization/WindowsFileSystemVirtualizerTests.cs b/GVFS/GVFS.UnitTests/Windows/Virtualization/WindowsFileSystemVirtualizerTests.cs index 5ca3f41159..c2d96a446b 100644 --- a/GVFS/GVFS.UnitTests/Windows/Virtualization/WindowsFileSystemVirtualizerTests.cs +++ b/GVFS/GVFS.UnitTests/Windows/Virtualization/WindowsFileSystemVirtualizerTests.cs @@ -11,7 +11,9 @@ using System; using System.Collections.Generic; using System.IO; +using System.Threading; using System.Threading.Tasks; +using GVFS.Common.Tracing; namespace GVFS.UnitTests.Windows.Virtualization { @@ -95,6 +97,75 @@ public void DeleteFile() } } + [TestCase] + public void DeleteFileReturnsIOErrorWhenVirtualizationInstanceThrows() + { + using (MockVirtualizationInstance mockVirtualization = new MockVirtualizationInstance()) + using (WindowsFileSystemVirtualizer virtualizer = new WindowsFileSystemVirtualizer(this.Repo.Context, this.Repo.GitObjects, mockVirtualization, numWorkThreads)) + { + // A native ProjFS failure (for example a NullReferenceException surfaced from + // projectedfslib.dll under memory pressure) must fail this single delete rather than + // propagate out and crash the mount. + mockVirtualization.DeleteFileException = new NullReferenceException("simulated native ProjFS failure"); + + UpdateFailureReason failureReason = UpdateFailureReason.DirtyData; + virtualizer + .DeleteFile("test.txt", UpdatePlaceholderType.AllowReadOnly, out failureReason) + .ShouldEqual(new FileSystemResult(FSResult.IOError, (int)HResult.InternalError)); + failureReason.ShouldEqual(UpdateFailureReason.NoFailure); + } + } + + [TestCase] + public void OutboundProjFSOperationsReturnFailureWhenVirtualizationInstanceThrows() + { + using (MockVirtualizationInstance mockVirtualization = new MockVirtualizationInstance()) + using (WindowsFileSystemVirtualizer virtualizer = new WindowsFileSystemVirtualizer(this.Repo.Context, this.Repo.GitObjects, mockVirtualization, numWorkThreads)) + { + // Every GVFS-initiated native ProjFS operation must fail the single call rather than + // propagate a native exception (e.g. a NullReferenceException from projectedfslib.dll + // under memory pressure) and crash the mount. + mockVirtualization.NativeCallException = new NullReferenceException("simulated native ProjFS failure"); + + virtualizer + .ClearNegativePathCache(out uint _) + .ShouldEqual(new FileSystemResult(FSResult.IOError, (int)HResult.InternalError)); + + UpdateFailureReason failureReason = UpdateFailureReason.DirtyData; + virtualizer + .UpdatePlaceholderIfNeeded( + "test.txt", + DateTime.Now, + DateTime.Now, + DateTime.Now, + DateTime.Now, + 0, + 15, + string.Empty, + UpdatePlaceholderType.AllowReadOnly, + out failureReason) + .ShouldEqual(new FileSystemResult(FSResult.IOError, (int)HResult.InternalError)); + failureReason.ShouldEqual(UpdateFailureReason.NoFailure); + } + } + + [TestCase] + public void WritePlaceholderOperationsReturnFailureWhenVirtualizationInstanceThrows() + { + using (WindowsFileSystemVirtualizerTester tester = new WindowsFileSystemVirtualizerTester(this.Repo)) + { + tester.MockVirtualization.NativeCallException = new NullReferenceException("simulated native ProjFS failure"); + + tester.WindowsVirtualizer + .WritePlaceholderFile("test.txt", endOfFile: 15, sha: new string('0', 40)) + .ShouldEqual(new FileSystemResult(FSResult.IOError, (int)HResult.InternalError)); + + tester.WindowsVirtualizer + .WritePlaceholderDirectory("testDir") + .ShouldEqual(new FileSystemResult(FSResult.IOError, (int)HResult.InternalError)); + } + } + [TestCase] public void UpdatePlaceholderIfNeeded() { @@ -181,6 +252,80 @@ public void OnStartDirectoryEnumerationReturnsSuccessWhenResultsInMemory() } } + [TestCase] + public void HeartbeatMetadataReportsActiveEnumerationCount() + { + using (WindowsFileSystemVirtualizerTester tester = new WindowsFileSystemVirtualizerTester(this.Repo, new[] { "test" })) + { + tester.GitIndexProjection.EnumerationInMemory = true; + + tester.MockVirtualization.RequiredCallbacks.StartDirectoryEnumerationCallback(1, Guid.NewGuid(), "test", TriggeringProcessId, TriggeringProcessImageFileName).ShouldEqual(HResult.Ok); + tester.MockVirtualization.RequiredCallbacks.StartDirectoryEnumerationCallback(2, Guid.NewGuid(), "test", TriggeringProcessId, TriggeringProcessImageFileName).ShouldEqual(HResult.Ok); + + EventMetadata metadata = new EventMetadata(); + tester.WindowsVirtualizer.AddHeartbeatMetadata(metadata); + + metadata.ContainsKey("ActiveEnumerationCount").ShouldBeTrue(); + ((int)metadata["ActiveEnumerationCount"]).ShouldEqual(2); + metadata.ContainsKey("ActiveCommandCount").ShouldBeTrue(); + } + } + + [TestCase] + public void StaleEnumerationsAreNotEvictedWhenDisabled() + { + using (WindowsFileSystemVirtualizerTester tester = new WindowsFileSystemVirtualizerTester(this.Repo, new[] { "test" })) + { + tester.GitIndexProjection.EnumerationInMemory = true; + + // Eviction is disabled by default (gvfs.max-active-enumerations unset => 0). Even with + // a zero stale timeout - which would make every enumeration eligible - nothing is evicted. + tester.WindowsVirtualizer.MaxActiveEnumerationsForTest = 0; + tester.WindowsVirtualizer.ActiveEnumerationStaleTimeoutForTest = TimeSpan.Zero; + + Guid firstId = Guid.NewGuid(); + Guid secondId = Guid.NewGuid(); + tester.MockVirtualization.RequiredCallbacks.StartDirectoryEnumerationCallback(1, firstId, "test", TriggeringProcessId, TriggeringProcessImageFileName).ShouldEqual(HResult.Ok); + Thread.Sleep(20); + tester.MockVirtualization.RequiredCallbacks.StartDirectoryEnumerationCallback(2, secondId, "test", TriggeringProcessId, TriggeringProcessImageFileName).ShouldEqual(HResult.Ok); + + tester.WindowsVirtualizer.ForceEnumerationEvictionSweepForTest(); + + // Both enumerations are still present (a missing id would return InternalError). + tester.MockVirtualization.RequiredCallbacks.EndDirectoryEnumerationCallback(firstId).ShouldEqual(HResult.Ok); + tester.MockVirtualization.RequiredCallbacks.EndDirectoryEnumerationCallback(secondId).ShouldEqual(HResult.Ok); + } + } + + [TestCase] + public void StaleEnumerationsAreEvictedWhenEnabledButLiveOnesAreKept() + { + using (WindowsFileSystemVirtualizerTester tester = new WindowsFileSystemVirtualizerTester(this.Repo, new[] { "test" })) + { + tester.GitIndexProjection.EnumerationInMemory = true; + + // Enable eviction above 1 live enumeration, treating anything idle longer than 20ms as stale. + tester.WindowsVirtualizer.MaxActiveEnumerationsForTest = 1; + tester.WindowsVirtualizer.ActiveEnumerationStaleTimeoutForTest = TimeSpan.FromMilliseconds(20); + + Guid staleId = Guid.NewGuid(); + tester.MockVirtualization.RequiredCallbacks.StartDirectoryEnumerationCallback(1, staleId, "test", TriggeringProcessId, TriggeringProcessImageFileName).ShouldEqual(HResult.Ok); + + // Let the first enumeration age well past the stale timeout before adding a fresh one. + Thread.Sleep(200); + + Guid freshId = Guid.NewGuid(); + tester.MockVirtualization.RequiredCallbacks.StartDirectoryEnumerationCallback(2, freshId, "test", TriggeringProcessId, TriggeringProcessImageFileName).ShouldEqual(HResult.Ok); + + tester.WindowsVirtualizer.ForceEnumerationEvictionSweepForTest(); + + // The stale enumeration was evicted: its End now fails to find it (fails loudly, does + // not silently return partial results). The fresh enumeration is retained. + tester.MockVirtualization.RequiredCallbacks.EndDirectoryEnumerationCallback(staleId).ShouldEqual(HResult.InternalError); + tester.MockVirtualization.RequiredCallbacks.EndDirectoryEnumerationCallback(freshId).ShouldEqual(HResult.Ok); + } + } + [TestCase] public void GetPlaceholderInformationHandlerPathNotProjected() { @@ -473,5 +618,33 @@ public void OnGetFileStreamHandlesHResultHandleResult() mockTracker.RelatedErrorEvents.ShouldBeEmpty(); } } + + [TestCase] + public void OnStartDirectoryEnumerationCleansUpEnumerationWhenNativeCompletionFails() + { + using (WindowsFileSystemVirtualizerTester tester = new WindowsFileSystemVirtualizerTester(this.Repo)) + { + MockTracer mockTracker = this.Repo.Context.Tracer as MockTracer; + mockTracker.WaitRelatedEventName = "StartDirectoryEnumerationAsyncHandler_CommandNotCompleted"; + + // Simulate the native ProjFS completion faulting under memory pressure. TryInvokeProjFS + // must swallow the exception so the mount survives, and because ProjFS never accepted the + // completion no EndDirectoryEnumeration callback will arrive. The enumeration registered by + // the async handler must therefore be removed here rather than leaked. + tester.MockVirtualization.CompleteCommandException = new NullReferenceException("simulated native ProjFS failure"); + + Guid enumerationGuid = Guid.NewGuid(); + tester.GitIndexProjection.EnumerationInMemory = false; + tester.MockVirtualization.RequiredCallbacks.StartDirectoryEnumerationCallback(1, enumerationGuid, "test", TriggeringProcessId, TriggeringProcessImageFileName).ShouldEqual(HResult.Pending); + + // Deterministically wait for the async handler to reach its cleanup path (the enumeration is + // removed immediately before this event is emitted). If the native failure were reported as a + // successful completion, this event would never fire. + mockTracker.WaitForRelatedEvent(); + + // The enumeration must already have been removed, so End cannot find it. + tester.MockVirtualization.RequiredCallbacks.EndDirectoryEnumerationCallback(enumerationGuid).ShouldEqual(HResult.InternalError); + } + } } } diff --git a/GVFS/GVFS.Virtualization/FileSystem/FileSystemVirtualizer.cs b/GVFS/GVFS.Virtualization/FileSystem/FileSystemVirtualizer.cs index 41e596401f..b3d0c0d3d0 100644 --- a/GVFS/GVFS.Virtualization/FileSystem/FileSystemVirtualizer.cs +++ b/GVFS/GVFS.Virtualization/FileSystem/FileSystemVirtualizer.cs @@ -111,6 +111,14 @@ public abstract FileSystemResult UpdatePlaceholderIfNeeded( public abstract FileSystemResult DehydrateFolder(string relativePath); + /// + /// Adds platform-specific virtualization diagnostics (for example live directory-enumeration + /// counts) to the periodic Heartbeat telemetry event. The default implementation adds nothing. + /// + public virtual void AddHeartbeatMetadata(EventMetadata metadata) + { + } + public void Dispose() { if (this.fileAndNetworkRequests != null) diff --git a/GVFS/GVFS.Virtualization/FileSystemCallbacks.cs b/GVFS/GVFS.Virtualization/FileSystemCallbacks.cs index ab2ff36d7a..91e627dd23 100644 --- a/GVFS/GVFS.Virtualization/FileSystemCallbacks.cs +++ b/GVFS/GVFS.Virtualization/FileSystemCallbacks.cs @@ -278,6 +278,8 @@ public EventMetadata GetAndResetHeartBeatMetadata(out bool logToFile) metadata.Add("FilePlaceholderCount", this.placeholderDatabase.GetFilePlaceholdersCount()); metadata.Add("FolderPlaceholderCount", this.placeholderDatabase.GetFolderPlaceholdersCount()); + this.fileSystemVirtualizer?.AddHeartbeatMetadata(metadata); + if (this.gitStatusCache.WriteTelemetryandReset(metadata)) { logToFile = true; @@ -436,6 +438,20 @@ public bool AddStagedFilesToModifiedPaths(string messageBody, out int addedCount // Query all staged files in one call using --name-status -z. // Output format: "A\0path1\0M\0path2\0D\0path3\0" GitProcess.Result result = gitProcess.DiffCachedNameStatus(pathspecs, pathspecFromFile, pathspecFileNul); + + if (result.OutputTruncated) + { + // The staged-file list exceeded the capture buffer. Acting on a partial list would leave + // some staged files out of ModifiedPaths (skip-worktree not cleared, stale placeholders), + // which is worse than failing. Fail safe and let the caller retry. + EventMetadata metadata = new EventMetadata(); + metadata.Add("ExitCode", result.ExitCode); + this.context.Tracer.RelatedError( + metadata, + nameof(this.AddStagedFilesToModifiedPaths) + ": git diff --cached output was truncated; refusing to update ModifiedPaths from a partial staged-file list"); + return false; + } + if (result.ExitCodeIsSuccess && !string.IsNullOrEmpty(result.Output)) { string[] parts = result.Output.Split(new[] { '\0' }, StringSplitOptions.RemoveEmptyEntries); diff --git a/GVFS/GVFS/CommandLine/PrefetchVerb.cs b/GVFS/GVFS/CommandLine/PrefetchVerb.cs index ef6d3ddf94..945984d62c 100644 --- a/GVFS/GVFS/CommandLine/PrefetchVerb.cs +++ b/GVFS/GVFS/CommandLine/PrefetchVerb.cs @@ -157,6 +157,8 @@ protected override void Execute(GVFSEnlistment enlistment) metadata.Add("FilesFromStdIn", this.FilesFromStdIn); metadata.Add("FoldersFromStdIn", this.FoldersFromStdIn); metadata.Add("HydrateFiles", this.HydrateFiles); + bool prefetchOffloadEnabled = this.IsPrefetchOffloadEnabled(tracer, enlistment); + metadata.Add("PrefetchOffloadEnabled", prefetchOffloadEnabled); tracer.RelatedEvent(EventLevel.Informational, "PerformPrefetch", metadata); if (this.Commits) @@ -181,7 +183,7 @@ protected override void Execute(GVFSEnlistment enlistment) // has its own spinner. We don't wrap this in ShowStatusWhileRunning // because a false return (mount unavailable) would print "Failed" // to the console, which is misleading for an expected fallback. - bool offloadSucceeded = this.TryPrefetchCommitsViaMountProcess(tracer, enlistment); + bool offloadSucceeded = prefetchOffloadEnabled && this.TryPrefetchCommitsViaMountProcess(tracer, enlistment); if (!offloadSucceeded) { @@ -220,9 +222,12 @@ protected override void Execute(GVFSEnlistment enlistment) // (without hydration), then hydrate locally in the verb process. // This avoids the mount process writing to ProjFS-virtualized files // (self-callback risk) while still benefiting from warm auth. - if (!this.TryPrefetchBlobsViaMountProcess(tracer, enlistment, filesList, foldersList, headCommitId)) + bool offloadSucceeded = prefetchOffloadEnabled && + this.TryPrefetchBlobsViaMountProcess(tracer, enlistment, filesList, foldersList, headCommitId); + + if (!offloadSucceeded) { - // Mount unavailable — fall back to direct auth for download + // Mount unavailable/offload disabled — fall back to direct auth for download GitObjectsHttpRequestor objectRequestor; CacheServerInfo resolvedCacheServer; this.InitializeServerConnection( @@ -242,7 +247,8 @@ protected override void Execute(GVFSEnlistment enlistment) BlobPrefetcher.UpdateNoopCache(prefetchCache, prefetchCacheSize, headCommitId, filesList, foldersList, this.HydrateFiles); } } - else if (!this.TryPrefetchBlobsViaMountProcess(tracer, enlistment, filesList, foldersList, headCommitId)) + else if (!prefetchOffloadEnabled || + !this.TryPrefetchBlobsViaMountProcess(tracer, enlistment, filesList, foldersList, headCommitId)) { GitObjectsHttpRequestor objectRequestor; CacheServerInfo resolvedCacheServer; @@ -692,6 +698,23 @@ private string GetCacheServerDisplay(CacheServerInfo cacheServer, string repoUrl return "from origin (no cache server)"; } + private bool IsPrefetchOffloadEnabled(ITracer tracer, GVFSEnlistment enlistment) + { + try + { + using (LibGit2Repo repo = new LibGit2Repo(tracer, enlistment.WorkingDirectoryBackingRoot)) + { + bool? enabled = repo.GetConfigBool(GVFSConstants.GitConfig.PrefetchOffload); + return enabled ?? GVFSConstants.GitConfig.PrefetchOffloadDefault; + } + } + catch (Exception ex) + { + tracer.RelatedWarning($"Failed to read '{GVFSConstants.GitConfig.PrefetchOffload}' config; defaulting to {GVFSConstants.GitConfig.PrefetchOffloadDefault}: {ex.GetType().Name}: {ex.Message}"); + return GVFSConstants.GitConfig.PrefetchOffloadDefault; + } + } + /// /// Hydrates files matching the file/folder filters by reading 1 byte from each. /// Runs in the verb process (not the mount) to avoid ProjFS self-callbacks. diff --git a/GVFS/GVFS/CommandLine/ServiceVerb.cs b/GVFS/GVFS/CommandLine/ServiceVerb.cs index 842521fa7a..0741aa40e1 100644 --- a/GVFS/GVFS/CommandLine/ServiceVerb.cs +++ b/GVFS/GVFS/CommandLine/ServiceVerb.cs @@ -73,7 +73,7 @@ public override void Execute() { foreach (string repoRoot in repoList) { - if (this.IsRepoMounted(repoRoot)) + if (this.IsRepoReady(repoRoot)) { this.Output.WriteLine(repoRoot); } @@ -93,7 +93,7 @@ public override void Execute() foreach (string repoRoot in repoList) { - if (!this.IsRepoMounted(repoRoot)) + if (this.IsRepoAvailableToMount(repoRoot)) { this.Output.WriteLine("\r\nMounting repo at " + repoRoot); ReturnCode result = this.Execute(repoRoot); @@ -118,7 +118,7 @@ public override void Execute() foreach (string repoRoot in repoList) { - if (this.IsRepoMounted(repoRoot)) + if (this.IsRepoAvailableToUnmount(repoRoot)) { this.Output.WriteLine("\r\nUnmounting repo at " + repoRoot); ReturnCode result = this.Execute( @@ -199,8 +199,35 @@ private bool TryGetRepoList(out List repoList, out string errorMessage) } } - private bool IsRepoMounted(string repoRoot) + private bool IsRepoReady(string repoRoot) { + // For --list-mounted: only a repo whose mount is fully Ready is reported as mounted. + return this.TryGetRepoMountStatus(repoRoot, out string mountStatus) + && mountStatus.Equals(NamedPipeMessages.GetStatus.Ready, StringComparison.Ordinal); + } + + private bool IsRepoAvailableToMount(string repoRoot) + { + // For --mount-all: a repo can be mounted only when no live mount process is + // answering for it (i.e. it is not already Mounting, Ready, Unmounting, or MountFailed). + return !this.TryGetRepoMountStatus(repoRoot, out _); + } + + private bool IsRepoAvailableToUnmount(string repoRoot) + { + // For --unmount-all: only unmount a repo whose mount is in a state that accepts an + // unmount request. A repo already Unmounting is reaching the desired state on its own, + // and one still Mounting rejects the request, so both are skipped to avoid spurious + // "Already unmounting" / "not mounted" failures during concurrent teardown. + return this.TryGetRepoMountStatus(repoRoot, out string mountStatus) + && (mountStatus.Equals(NamedPipeMessages.GetStatus.Ready, StringComparison.Ordinal) + || mountStatus.Equals(NamedPipeMessages.GetStatus.MountFailed, StringComparison.Ordinal)); + } + + private bool TryGetRepoMountStatus(string repoRoot, out string mountStatus) + { + mountStatus = null; + // Hide the output of status StringWriter statusOutput = new StringWriter(); ReturnCode result = this.Execute( @@ -210,9 +237,20 @@ private bool IsRepoMounted(string repoRoot) verb.Output = statusOutput; }); - if (result == ReturnCode.Success) + if (result != ReturnCode.Success) + { + // No live mount process is answering for this repo. + return false; + } + + foreach (string line in statusOutput.ToString().Split('\n')) { - return true; + string trimmedLine = line.Trim(); + if (trimmedLine.StartsWith(StatusVerb.MountStatusOutputPrefix, StringComparison.Ordinal)) + { + mountStatus = trimmedLine.Substring(StatusVerb.MountStatusOutputPrefix.Length).Trim(); + return true; + } } return false; diff --git a/GVFS/GVFS/CommandLine/SparseVerb.cs b/GVFS/GVFS/CommandLine/SparseVerb.cs index 1ea6e5aed4..334e4d1ca4 100644 --- a/GVFS/GVFS/CommandLine/SparseVerb.cs +++ b/GVFS/GVFS/CommandLine/SparseVerb.cs @@ -639,6 +639,17 @@ private void CheckGitStatus(ITracer tracer, GVFSEnlistment enlistment, HashSetUser.Sid))) -+ if ((*sid = sid_dup(info->User.Sid))) - error = 0; - -+ if (GetTokenInformation(token, TokenElevationType, &elevation_type, sizeof(elevation_type), &size) && -+ elevation_type == TokenElevationTypeLimited) { -+ /* -+ * The current process is run by a member of the Administrators group -+ * but is not running elevated. -+ */ -+ if (!GetTokenInformation(token, TokenLinkedToken, linked_token, sizeof(HANDLE), &size)) { -+ linked_token = NULL; -+ } -+ } - done: - if (token) - CloseHandle(token); -@@ -1926,6 +1940,7 @@ int git_fs_path_owner_is( - git_fs_path_owner_t owner_type) - { - PSID owner_sid = NULL, user_sid = NULL; -+ static HANDLE linked_token; - BOOL is_admin, admin_owned; - int error; - -@@ -1938,7 +1953,7 @@ int git_fs_path_owner_is( - goto done; - - if ((owner_type & GIT_FS_PATH_OWNER_CURRENT_USER) != 0) { -- if ((error = current_user_sid(&user_sid)) < 0) -+ if ((error = current_user_sid(&user_sid, &linked_token)) < 0) - goto done; - - if (EqualSid(owner_sid, user_sid)) { -@@ -1959,7 +1974,8 @@ int git_fs_path_owner_is( - - if (admin_owned && - (owner_type & GIT_FS_PATH_USER_IS_ADMINISTRATOR) != 0 && -- CheckTokenMembership(NULL, owner_sid, &is_admin) && -+ (CheckTokenMembership(NULL, owner_sid, &is_admin) && -+ CheckTokenMembership(linked_token, owner_sid, &is_admin)) && - is_admin) { - *out = true; - goto done; +diff --git a/src/util/fs_path.c b/src/util/fs_path.c +index 5b00b1b..976a9f0 100644 +--- a/src/util/fs_path.c ++++ b/src/util/fs_path.c +@@ -1853,12 +1853,14 @@ static PSID *sid_dup(PSID sid) + return dup; + } + +-static int current_user_sid(PSID *out) ++static int current_user_sid(PSID *sid, HANDLE *linked_token) + { + TOKEN_USER *info = NULL; + HANDLE token = NULL; + DWORD len = 0; + int error = -1; ++ TOKEN_ELEVATION_TYPE elevation_type; ++ DWORD size; + + if (!OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &token)) { + git_error_set(GIT_ERROR_OS, "could not lookup process information"); +@@ -1879,7 +1881,19 @@ static int current_user_sid(PSID *out) + goto done; + } + +- if ((*out = sid_dup(info->User.Sid))) ++ if (GetTokenInformation(token, TokenElevationType, &elevation_type, sizeof(elevation_type), &size) && ++ elevation_type == TokenElevationTypeLimited) { ++ /* ++ * The current process is run by a member of the Administrators group ++ * but is not running elevated. ++ */ ++ if (!GetTokenInformation(token, TokenLinkedToken, linked_token, sizeof(HANDLE), &size)) { ++ CloseHandle(*linked_token); ++ *linked_token = NULL; ++ } ++ } ++ ++ if ((*sid = sid_dup(info->User.Sid))) + error = 0; + + done: +@@ -1926,6 +1940,7 @@ int git_fs_path_owner_is( + git_fs_path_owner_t owner_type) + { + PSID owner_sid = NULL, user_sid = NULL; ++ HANDLE linked_token = NULL; + BOOL is_admin, admin_owned; + int error; + +@@ -1934,17 +1949,14 @@ int git_fs_path_owner_is( + return 0; + } + +- if ((error = file_owner_sid(&owner_sid, path)) < 0) ++ if ((error = file_owner_sid(&owner_sid, path)) < 0 || ++ (error = current_user_sid(&user_sid, &linked_token)) < 0) + goto done; + +- if ((owner_type & GIT_FS_PATH_OWNER_CURRENT_USER) != 0) { +- if ((error = current_user_sid(&user_sid)) < 0) +- goto done; +- +- if (EqualSid(owner_sid, user_sid)) { +- *out = true; +- goto done; +- } ++ if ((owner_type & GIT_FS_PATH_OWNER_CURRENT_USER) != 0 && ++ EqualSid(owner_sid, user_sid)) { ++ *out = true; ++ goto done; + } + + admin_owned = +@@ -1960,6 +1972,7 @@ int git_fs_path_owner_is( + if (admin_owned && + (owner_type & GIT_FS_PATH_USER_IS_ADMINISTRATOR) != 0 && + CheckTokenMembership(NULL, owner_sid, &is_admin) && ++ CheckTokenMembership(linked_token, owner_sid, &is_admin) && + is_admin) { + *out = true; + goto done; +@@ -1968,6 +1981,9 @@ int git_fs_path_owner_is( + *out = false; + + done: ++ if (linked_token) ++ CloseHandle(linked_token); ++ + git__free(owner_sid); + git__free(user_sid); + return error; diff --git a/overlays/libgit2/portfile.cmake b/overlays/libgit2/portfile.cmake index ddf650e177..bc47e5f910 100644 --- a/overlays/libgit2/portfile.cmake +++ b/overlays/libgit2/portfile.cmake @@ -2,7 +2,7 @@ vcpkg_from_github( OUT_SOURCE_PATH SOURCE_PATH REPO libgit2/libgit2 REF "v${VERSION}" - SHA512 5359d07b85b691b92784d5ef52825be2c1f1616bab8e969e51177bf8f5e767edbaf267943296407f62df1f3d7fa08e48c802374289046b71da2cc1ac7d43a15f + SHA512 85036ade3aca33c5283605ae9de21a7948ec5952bc8cd468aa024ca873851a56ab0ebe62f95c0da109df9163875e9687a377d3df69728d434cc258b3f845ef0c HEAD_REF main PATCHES dependencies.diff diff --git a/overlays/libgit2/vcpkg.json b/overlays/libgit2/vcpkg.json index 700a092558..4d3a5d3b20 100644 --- a/overlays/libgit2/vcpkg.json +++ b/overlays/libgit2/vcpkg.json @@ -1,6 +1,6 @@ { "name": "libgit2", - "version-semver": "1.9.3", + "version-semver": "1.9.4", "description": "A C library implementing the Git core methods with a solid API", "homepage": "https://github.com/libgit2/libgit2", "license": null,