Release M276.1#2019
Merged
Merged
Conversation
….vfs.0.2 Update default Microsoft Git version to v2.54.0.vfs.0.2
GVFS.Service uses CreateProcessAsUser to spawn 'GVFS.exe mount <repo>
--internal_use_only <json>' on each user logon. The JSON arg
({"ServiceName":null,"StartedByService":true,...}) was concatenated
straight into the command line with no escaping. Windows
CommandLineToArgvW strips the embedded " characters when re-parsing
argv in the child, so the receiving process sees
{ServiceName:null,StartedByService:true,...}.
Newtonsoft.Json was permissive enough to round-trip that corrupted
payload. After commit c83beab ("Migrate from Newtonsoft.Json to
System.Text.Json"), System.Text.Json (strict — quoted property names
required) throws JsonException, the verb hits ReportErrorAndExit
*before* AddLogFileEventListener runs, and the child exits without ever
creating a mount log file. The service then blocks for the full
60-second WaitUntilMounted timeout on each repo, fails automount, and
reports only "the GVFS.Mount process is not responding."
Fix:
* Add CommandLineEscaping.EscapeArgument implementing the
CommandLineToArgvW escaping rules (quote when needed, double
backslashes that precede a quote terminator, escape literal "
as \").
* Replace CurrentUser.RunAs(string, string) with TryRunAs(string,
string[], out int processId) so callers supply argv values and
the escaping happens centrally. Also surface the child PID so
callers can track its liveness.
* WindowsPlatform.StartBackgroundVFS4GProcess: route every arg
through the new escaper (previously only space-containing args
got naively quoted, embedded " was never escaped).
* GVFSEnlistment.WaitUntilMounted: add an overload that takes a
Func<MountProcessSnapshot>. When supplied, the wait loop polls
the snapshot between short (500 ms) connect attempts and fails
fast with the exit code if the child has already terminated,
instead of blocking for the full pipe timeout.
* GVFSMountProcess: pass the spawned child's PID into the new
WaitUntilMounted overload so logon automount detects an early
GVFS.exe exit within seconds and logs a useful diagnostic
("GVFS.Mount process (Id N) exited with code X before the named
pipe was ready") instead of a 60-second silent timeout.
Tests:
* CommandLineEscapingTests covers the canonical edge cases
(spaces, tabs, embedded quotes, trailing backslashes) and the
exact InternalVerbParameters JSON shape that triggered the
regression. A round-trip test feeds the escaped form back
through CommandLineToArgvW and verifies the original string
pops out unchanged.
* WaitUntilMountedProcessTrackingTests verifies the wait
short-circuits on an exit reported by the first snapshot poll,
and also catches a late exit reported on a later poll, both
well under the legacy 60-second timeout.
Assisted-by: Claude Opus 4.7
Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Address code-review feedback on the prior commit. The wait loop used
the spawned mount process's PID and re-opened it via
Process.GetProcessById on every poll, which has two problems:
1. The lookup races with the very failure mode the fast-fail path is
meant to catch. If the child exits between CreateProcessAsUser
returning and GVFSMountProcess looking it up, GetProcessById
throws ArgumentException, TryGetProcessById returns null, and
WaitUntilMounted silently falls back to the legacy single 60-second
blocking Connect — defeating the whole purpose.
2. Querying by PID is vulnerable to PID reuse: if Windows recycles
the PID quickly enough, a snapshot poll could observe an
unrelated process's exit state.
Both problems disappear if we keep the process handle that
CreateProcessAsUser already returned. The kernel keeps the process
object alive for as long as we hold the handle, so liveness and
exit-code queries remain valid even after the child has exited, and
PID reuse is irrelevant because we never look up by PID again.
Changes:
* CurrentUser.TryRunAs returns a SafeProcessHandle (owned) instead
of just a PID. Internally, the thread handle is still closed
immediately; only the process handle is retained.
* New ProcessHandleHelper exposes HasExited (non-blocking
WaitForSingleObject(handle, 0)) and TryGetExitCode
(GetExitCodeProcess) directly against the handle. This avoids
the STILL_ACTIVE (259) ambiguity in GetExitCodeProcess alone —
we only ask for the exit code once the wait says signaled.
* GVFSMountProcess.MountRepository owns the SafeProcessHandle in a
using block spanning the entire WaitUntilMounted call and uses
the handle for snapshot queries.
* Removed TryGetProcessById and the Process.GetProcessById /
Process.HasExited path entirely — no PID lookup remains in the
automount code path.
Tests:
* Build clean, 0 errors.
* All 842 unit tests still pass (24 new + 818 existing). The
existing WaitUntilMountedProcessTrackingTests don't need
changes because they exercise the snapshot delegate, not the
handle plumbing.
Assisted-by: Claude Opus 4.7
Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Service: Fix automount regression - escape JSON arg passed to child mount processes
KeithIsSleeping
approved these changes
Jun 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes: