microsoft / microsoft/VFSForGit

handle and report placeholder update failures without killing provider

Open
#1,316 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

type: enhancement
Dominant language
C#
Stars
6.1k
Forks
474
Avg merge
2d 4h
Merged PRs (30d)
8

Description

While investigating a functional test failure on Linux (now resolved thanks to a tip from @wilbaker!), we noticed that a 404 response from the GVFS server, while handling a PostIndexChanged request from a Git command's post-indexchanged hook, caused the GVFS provider to exit and the Git command to report a broken pipe. On Linux in particular, this then leaves the mount in a non-responsive state (as the FUSE process, i.e., the GVFS provider process, has died), and the only option is to forcibly umount the dead mount.

In particular, the gvfs_mount_process_*.log file reports something like:

UpdatePlaceholders (Start) {"File placeholder count":419,"Folder placeholders count":253}
QueryFileSizes {"RequestId":289,"ObjectIdJson":"[\"D739E69ED775B7140C296F47BE03169F26D7179E\"]"}
NetworkResponse {"RequestId":289,"availableConnections":1,"CacheName":"","StatusCode":404,"connectionWaitTimeMS":"0.0004","responseWaitTimeMS":"107.8310"}
...
NetworkError {"Area":"FolderEntryData","SHA":"D739E69ED775B7140C296F47BE03169F26D7179E","ErrorMessage":"PopulateMissingSizesFromRemote: Failed to download size for child entry"}
UpdatePlaceholders (Stop) {"DurationMs":125763}
Error {"Area":"GitIndexProjection","Exception":"GVFS.Virtualization.Projection.SizesUnavailableException: Failed to download size for D739E69ED775B7140C296F47BE03169F26D7179E
  at GVFS.Virtualization.Projection.GitIndexProjection.FolderData.PopulateSizesFromRemote(ITracer tracer, GVFSGitObjects gitObjects, BlobSizesConnection blobSizesConnection, HashSet`1 missingShas, List`1 childrenMissingSizes, CancellationToken cancellationToken) in GVFS/GVFS.Virtualization/Projection/GitIndexProjection.FolderData.cs:line 152
  at GVFS.Virtualization.Projection.GitIndexProjection.FolderData.PopulateSizes(ITracer tracer, GVFSGitObjects gitObjects, BlobSizesConnection blobSizesConnection, Dictionary`2 availableSizes, CancellationToken cancellationToken) in GVFS/GVFS.Virtualization/Projection/GitIndexProjection.FolderData.cs:line 67
  at GVFS.Virtualization.Projection.GitIndexProjection.ReExpandFolder(BlobSizesConnection blobSizesConnection, String relativeFolderPath, ConcurrentDictionary`2 updatedPlaceholderList, HashSet`1 existingFolderPlaceholders) in GVFS/GVFS.Virtualization/Projection/GitIndexProjection.cs:line 1415
  at GVFS.Virtualization.Projection.GitIndexProjection.UpdatePlaceholders() in GVFS/GVFS.Virtualization/Projection/GitIndexProjection.cs:line 1209
  at GVFS.Virtualization.Projection.GitIndexProjection.ParseIndexThreadMain() in GVFS/GVFS.Virtualization/Projection/GitIndexProjection.cs:line 1053",
  "ErrorMessage":"ParseIndexThreadMain caught unhandled exception, exiting process"}

While I understand that this is an exceptional condition, and there is no obvious way to recover the repository to a known-good state that would let the user proceed, it would be ideal (especially on Linux) to at least avoid exiting the GVFS provider process entirely, and instead place it into a live-but-invalid mode. I also note that this is somewhat challenging because the Git hook response code does not directly execute the UpdatePlaceholders() method, but merely signals it and waits for a thread synchronization event on the projectionParseComplete ManualResetEventSlim.

As a possible alternative, however, suppose that UpdatePlaceholders() or the ParseIndexThreadMain() loop, rather than exiting on exceptions such as the SizesUnavailableException, instead set a non-reversible "invalid" flag within the GitIndexProjection class, or the provider as a whole, and then checked that flag before responding to any further Git hook requests, and if it was set, always immediately returned a FailureResult or equivalent for the given hook request. The hook which triggered the invalid state could also receive such a response (instead of a broken pipe) if it also checked the state flag after signalling the event loop, e.g., something like:

public bool ForceIndexProjectionUpdate(bool invalidateProjection, bool invalidateModifiedPaths)
        {
            this.InvalidateState(invalidateProjection, invalidateModifiedPaths);
            this.GitIndexProjection.WaitForProjectionUpdate();
            return this.GitIndexProjection.ValidState;
        }

(Of course, the projectionParseComplete synchronization event would still need to be Set() to make this happen; that would have to be part of the exception-handling in the event loop.)

The advantage of such an implementation, on Linux, would be that the working directory filesystem would remain mounted and nominally functional -- the user could continue to ls and cd and so forth, at least within the file hierarchy as it was hydrated at the time the provider entered its invalid state. Other actions, which would normally cause further hydration, would receive at least some form of errno(3) which they could handle (i.e., the GVFS provider would convert hydration errors into errno(3) values instead of performing hydration, and libprojfs would then deliver them up through FUSE.

However, any attempt to use Git commands could return a "GVFS provider is no longer in a valid state; please unmount" message -- including, importantly, the first one which triggered the entry into the invalid state -- thus advising the user of how to proceed.

/cc @kivikakk

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading GitIndexProjection.ParseIndexThreadMain(), UpdatePlaceholders(), and FolderData.PopulateSizesFromRemote(), then trace InProcessMount.cs and FileSystemCallbacks.cs for hook signaling and completion handling. Check how FailureResult is represented in NamedPipeMessages.cs. Done means a placeholder update failure reports a controlled failure to Git without exiting the provider, while the completion event is still released and the invalid state is communicated to later requests.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp, git, linux
Domain
devtools, operating-systems
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.