Skip to content

Commit 0208851

Browse files
tyrielvtyrielv
committed
Mount: prevent process crash on unhandled request handler exceptions
HandleRequest now catches exceptions from individual pipe request handlers instead of letting them propagate to OnNewConnection, which calls Environment.Exit on any unhandled exception. A single transient error (network timeout, disk I/O failure) in a download handler would crash the entire mount process, breaking all pipe connections. HandleDownloadObjectRequest is refactored to isolate the download logic in DownloadObject and wrap it in a try-catch that returns a DownloadFailed response on exception. The read-object hook then receives a proper failure response instead of ERROR_BROKEN_PIPE (109), and git handles the object-not-available error more gracefully. Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
1 parent 73e8e29 commit 0208851

1 file changed

Lines changed: 124 additions & 93 deletions

File tree

‎GVFS/GVFS.Mount/InProcessMount.cs‎

Lines changed: 124 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -474,60 +474,71 @@ private void HandleRequest(ITracer tracer, string request, NamedPipeServer.Conne
474474
{
475475
NamedPipeMessages.Message message = NamedPipeMessages.Message.FromString(request);
476476

477-
switch (message.Header)
477+
try
478478
{
479-
case NamedPipeMessages.GetStatus.Request:
480-
this.HandleGetStatusRequest(connection);
481-
break;
482-
483-
case NamedPipeMessages.Unmount.Request:
484-
this.HandleUnmountRequest(connection);
485-
break;
486-
487-
case NamedPipeMessages.AcquireLock.AcquireRequest:
488-
this.HandleLockRequest(message.Body, connection);
489-
break;
490-
491-
case NamedPipeMessages.ReleaseLock.Request:
492-
this.HandleReleaseLockRequest(message.Body, connection);
493-
break;
494-
495-
case NamedPipeMessages.DownloadObject.DownloadRequest:
496-
this.HandleDownloadObjectRequest(message, connection);
497-
break;
498-
499-
case NamedPipeMessages.ModifiedPaths.ListRequest:
500-
this.HandleModifiedPathsListRequest(message, connection);
501-
break;
502-
503-
case NamedPipeMessages.PostIndexChanged.NotificationRequest:
504-
this.HandlePostIndexChangedRequest(message, connection);
505-
break;
506-
507-
case NamedPipeMessages.PrepareForUnstage.Request:
508-
this.HandlePrepareForUnstageRequest(message, connection);
509-
break;
510-
511-
case NamedPipeMessages.RunPostFetchJob.PostFetchJob:
512-
this.HandlePostFetchJobRequest(message, connection);
513-
break;
514-
515-
case NamedPipeMessages.DehydrateFolders.Dehydrate:
516-
this.HandleDehydrateFolders(message, connection);
517-
break;
518-
519-
case NamedPipeMessages.HydrationStatus.Request:
520-
this.HandleGetHydrationStatusRequest(connection);
521-
break;
522-
523-
default:
524-
EventMetadata metadata = new EventMetadata();
525-
metadata.Add("Area", "Mount");
526-
metadata.Add("Header", message.Header);
527-
this.tracer.RelatedError(metadata, "HandleRequest: Unknown request");
528-
529-
connection.TrySendResponse(NamedPipeMessages.UnknownRequest);
530-
break;
479+
switch (message.Header)
480+
{
481+
case NamedPipeMessages.GetStatus.Request:
482+
this.HandleGetStatusRequest(connection);
483+
break;
484+
485+
case NamedPipeMessages.Unmount.Request:
486+
this.HandleUnmountRequest(connection);
487+
break;
488+
489+
case NamedPipeMessages.AcquireLock.AcquireRequest:
490+
this.HandleLockRequest(message.Body, connection);
491+
break;
492+
493+
case NamedPipeMessages.ReleaseLock.Request:
494+
this.HandleReleaseLockRequest(message.Body, connection);
495+
break;
496+
497+
case NamedPipeMessages.DownloadObject.DownloadRequest:
498+
this.HandleDownloadObjectRequest(message, connection);
499+
break;
500+
501+
case NamedPipeMessages.ModifiedPaths.ListRequest:
502+
this.HandleModifiedPathsListRequest(message, connection);
503+
break;
504+
505+
case NamedPipeMessages.PostIndexChanged.NotificationRequest:
506+
this.HandlePostIndexChangedRequest(message, connection);
507+
break;
508+
509+
case NamedPipeMessages.PrepareForUnstage.Request:
510+
this.HandlePrepareForUnstageRequest(message, connection);
511+
break;
512+
513+
case NamedPipeMessages.RunPostFetchJob.PostFetchJob:
514+
this.HandlePostFetchJobRequest(message, connection);
515+
break;
516+
517+
case NamedPipeMessages.DehydrateFolders.Dehydrate:
518+
this.HandleDehydrateFolders(message, connection);
519+
break;
520+
521+
case NamedPipeMessages.HydrationStatus.Request:
522+
this.HandleGetHydrationStatusRequest(connection);
523+
break;
524+
525+
default:
526+
EventMetadata metadata = new EventMetadata();
527+
metadata.Add("Area", "Mount");
528+
metadata.Add("Header", message.Header);
529+
this.tracer.RelatedError(metadata, "HandleRequest: Unknown request");
530+
531+
connection.TrySendResponse(NamedPipeMessages.UnknownRequest);
532+
break;
533+
}
534+
}
535+
catch (Exception e)
536+
{
537+
EventMetadata metadata = new EventMetadata();
538+
metadata.Add("Area", "Mount");
539+
metadata.Add("Header", message.Header);
540+
metadata.Add("Exception", e.ToString());
541+
this.tracer.RelatedError(metadata, "HandleRequest: Unhandled exception in request handler");
531542
}
532543
}
533544

@@ -872,56 +883,76 @@ private void HandleDownloadObjectRequest(NamedPipeMessages.Message message, Name
872883
}
873884
else
874885
{
875-
Stopwatch downloadTime = Stopwatch.StartNew();
876-
877-
/* If this is the root tree for a commit that was was just downloaded, assume that more
878-
* trees will be needed soon and download them as well by using the download commit API.
879-
*
880-
* Otherwise, or as a fallback if the commit download fails, download the object directly.
881-
*/
882-
if (this.ShouldDownloadCommitPack(objectSha, out string commitSha)
883-
&& this.gitObjects.TryDownloadCommit(commitSha))
884-
{
885-
this.DownloadedCommitPack(commitSha);
886-
response = new NamedPipeMessages.DownloadObject.Response(NamedPipeMessages.DownloadObject.SuccessResult);
887-
// FUTURE: Should the stats be updated to reflect all the trees in the pack?
888-
// FUTURE: Should we try to clean up duplicate trees or increase depth of the commit download?
889-
}
890-
else if (this.gitObjects.TryDownloadAndSaveObject(objectSha, GVFSGitObjects.RequestSource.NamedPipeMessage) == GitObjects.DownloadAndSaveObjectResult.Success)
886+
try
891887
{
892-
this.UpdateTreesForDownloadedCommits(objectSha);
893-
response = new NamedPipeMessages.DownloadObject.Response(NamedPipeMessages.DownloadObject.SuccessResult);
888+
response = this.DownloadObject(objectSha);
894889
}
895-
else
890+
catch (Exception e)
896891
{
892+
EventMetadata metadata = new EventMetadata();
893+
metadata.Add("Area", "Mount");
894+
metadata.Add("objectSha", objectSha);
895+
metadata.Add("Exception", e.ToString());
896+
this.tracer.RelatedWarning(metadata, nameof(this.HandleDownloadObjectRequest) + ": Exception downloading object");
897+
897898
response = new NamedPipeMessages.DownloadObject.Response(NamedPipeMessages.DownloadObject.DownloadFailed);
898899
}
900+
}
901+
}
899902

903+
connection.TrySendResponse(response.CreateMessage());
904+
}
900905

901-
Native.ObjectTypes? objectType;
902-
this.context.Repository.TryGetObjectType(objectSha, out objectType);
903-
this.context.Repository.GVFSLock.Stats.RecordObjectDownload(objectType == Native.ObjectTypes.Blob, downloadTime.ElapsedMilliseconds);
906+
private NamedPipeMessages.DownloadObject.Response DownloadObject(string objectSha)
907+
{
908+
NamedPipeMessages.DownloadObject.Response response;
909+
Stopwatch downloadTime = Stopwatch.StartNew();
904910

905-
if (objectType == Native.ObjectTypes.Commit
906-
&& !this.context.Repository.CommitAndRootTreeExists(objectSha, out var treeSha)
907-
&& !string.IsNullOrEmpty(treeSha))
908-
{
909-
/* If a commit is downloaded, it wasn't prefetched.
910-
* The trees for the commit may be needed soon depending on the context.
911-
* e.g. git log (without a pathspec) doesn't need trees, but git checkout does.
912-
*
913-
* If any prefetch has been done there is probably a similar commit/tree in the graph,
914-
* but in case there isn't (such as if the cache server repack maintenance job is failing)
915-
* we should still try to avoid downloading an excessive number of loose trees for a commit.
916-
*
917-
* Save the tree/commit so if more trees are requested we can download all the trees for the commit in a batch.
918-
*/
919-
this.missingTreeTracker.AddMissingRootTree(treeSha: treeSha, commitSha: objectSha);
920-
}
921-
}
911+
/* If this is the root tree for a commit that was was just downloaded, assume that more
912+
* trees will be needed soon and download them as well by using the download commit API.
913+
*
914+
* Otherwise, or as a fallback if the commit download fails, download the object directly.
915+
*/
916+
if (this.ShouldDownloadCommitPack(objectSha, out string commitSha)
917+
&& this.gitObjects.TryDownloadCommit(commitSha))
918+
{
919+
this.DownloadedCommitPack(commitSha);
920+
response = new NamedPipeMessages.DownloadObject.Response(NamedPipeMessages.DownloadObject.SuccessResult);
921+
// FUTURE: Should the stats be updated to reflect all the trees in the pack?
922+
// FUTURE: Should we try to clean up duplicate trees or increase depth of the commit download?
923+
}
924+
else if (this.gitObjects.TryDownloadAndSaveObject(objectSha, GVFSGitObjects.RequestSource.NamedPipeMessage) == GitObjects.DownloadAndSaveObjectResult.Success)
925+
{
926+
this.UpdateTreesForDownloadedCommits(objectSha);
927+
response = new NamedPipeMessages.DownloadObject.Response(NamedPipeMessages.DownloadObject.SuccessResult);
928+
}
929+
else
930+
{
931+
response = new NamedPipeMessages.DownloadObject.Response(NamedPipeMessages.DownloadObject.DownloadFailed);
922932
}
923933

924-
connection.TrySendResponse(response.CreateMessage());
934+
Native.ObjectTypes? objectType;
935+
this.context.Repository.TryGetObjectType(objectSha, out objectType);
936+
this.context.Repository.GVFSLock.Stats.RecordObjectDownload(objectType == Native.ObjectTypes.Blob, downloadTime.ElapsedMilliseconds);
937+
938+
if (objectType == Native.ObjectTypes.Commit
939+
&& !this.context.Repository.CommitAndRootTreeExists(objectSha, out var treeSha)
940+
&& !string.IsNullOrEmpty(treeSha))
941+
{
942+
/* If a commit is downloaded, it wasn't prefetched.
943+
* The trees for the commit may be needed soon depending on the context.
944+
* e.g. git log (without a pathspec) doesn't need trees, but git checkout does.
945+
*
946+
* If any prefetch has been done there is probably a similar commit/tree in the graph,
947+
* but in case there isn't (such as if the cache server repack maintenance job is failing)
948+
* we should still try to avoid downloading an excessive number of loose trees for a commit.
949+
*
950+
* Save the tree/commit so if more trees are requested we can download all the trees for the commit in a batch.
951+
*/
952+
this.missingTreeTracker.AddMissingRootTree(treeSha: treeSha, commitSha: objectSha);
953+
}
954+
955+
return response;
925956
}
926957

927958
private bool ShouldDownloadCommitPack(string objectSha, out string commitSha)

0 commit comments

Comments
 (0)