Skip to content

Commit c46bc36

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. Both catch sites use exception filters to exclude OutOfMemoryException, which indicates a corrupted heap state where continuing is unsafe. StackOverflowException and AccessViolationException are already uncatchable in .NET Core and need no explicit exclusion. 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 fa0c3c5 commit c46bc36

1 file changed

Lines changed: 119 additions & 87 deletions

File tree

‎GVFS/GVFS.Mount/InProcessMount.cs‎

Lines changed: 119 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -475,68 +475,79 @@ private void HandleRequest(ITracer tracer, string request, NamedPipeServer.Conne
475475
{
476476
NamedPipeMessages.Message message = NamedPipeMessages.Message.FromString(request);
477477

478-
switch (message.Header)
478+
try
479479
{
480-
case NamedPipeMessages.GetStatus.Request:
481-
this.HandleGetStatusRequest(connection);
482-
break;
480+
switch (message.Header)
481+
{
482+
case NamedPipeMessages.GetStatus.Request:
483+
this.HandleGetStatusRequest(connection);
484+
break;
483485

484-
case NamedPipeMessages.Unmount.Request:
485-
this.HandleUnmountRequest(connection);
486-
break;
486+
case NamedPipeMessages.Unmount.Request:
487+
this.HandleUnmountRequest(connection);
488+
break;
487489

488-
case NamedPipeMessages.AcquireLock.AcquireRequest:
489-
this.HandleLockRequest(message.Body, connection);
490-
break;
490+
case NamedPipeMessages.AcquireLock.AcquireRequest:
491+
this.HandleLockRequest(message.Body, connection);
492+
break;
491493

492-
case NamedPipeMessages.ReleaseLock.Request:
493-
this.HandleReleaseLockRequest(message.Body, connection);
494-
break;
494+
case NamedPipeMessages.ReleaseLock.Request:
495+
this.HandleReleaseLockRequest(message.Body, connection);
496+
break;
495497

496-
case NamedPipeMessages.DownloadObject.DownloadRequest:
497-
this.HandleDownloadObjectRequest(message, connection);
498-
break;
498+
case NamedPipeMessages.DownloadObject.DownloadRequest:
499+
this.HandleDownloadObjectRequest(message, connection);
500+
break;
499501

500-
case NamedPipeMessages.ModifiedPaths.ListRequest:
501-
this.HandleModifiedPathsListRequest(message, connection);
502-
break;
502+
case NamedPipeMessages.ModifiedPaths.ListRequest:
503+
this.HandleModifiedPathsListRequest(message, connection);
504+
break;
503505

504-
case NamedPipeMessages.PostIndexChanged.NotificationRequest:
505-
this.HandlePostIndexChangedRequest(message, connection);
506-
break;
506+
case NamedPipeMessages.PostIndexChanged.NotificationRequest:
507+
this.HandlePostIndexChangedRequest(message, connection);
508+
break;
507509

508-
case NamedPipeMessages.PrepareForUnstage.Request:
509-
this.HandlePrepareForUnstageRequest(message, connection);
510-
break;
510+
case NamedPipeMessages.PrepareForUnstage.Request:
511+
this.HandlePrepareForUnstageRequest(message, connection);
512+
break;
511513

512-
case NamedPipeMessages.RunPostFetchJob.PostFetchJob:
513-
this.HandlePostFetchJobRequest(message, connection);
514-
break;
514+
case NamedPipeMessages.RunPostFetchJob.PostFetchJob:
515+
this.HandlePostFetchJobRequest(message, connection);
516+
break;
515517

516-
case NamedPipeMessages.DehydrateFolders.Dehydrate:
517-
this.HandleDehydrateFolders(message, connection);
518-
break;
518+
case NamedPipeMessages.DehydrateFolders.Dehydrate:
519+
this.HandleDehydrateFolders(message, connection);
520+
break;
519521

520-
case NamedPipeMessages.PrefetchCommits.Request:
521-
this.HandlePrefetchCommitsRequest(connection);
522-
break;
522+
case NamedPipeMessages.PrefetchCommits.Request:
523+
this.HandlePrefetchCommitsRequest(connection);
524+
break;
523525

524-
case NamedPipeMessages.PrefetchBlobs.RequestHeader:
525-
this.HandlePrefetchBlobsRequest(message, connection);
526-
break;
526+
case NamedPipeMessages.PrefetchBlobs.RequestHeader:
527+
this.HandlePrefetchBlobsRequest(message, connection);
528+
break;
527529

528-
case NamedPipeMessages.HydrationStatus.Request:
529-
this.HandleGetHydrationStatusRequest(connection);
530-
break;
530+
case NamedPipeMessages.HydrationStatus.Request:
531+
this.HandleGetHydrationStatusRequest(connection);
532+
break;
531533

532-
default:
533-
EventMetadata metadata = new EventMetadata();
534-
metadata.Add("Area", "Mount");
535-
metadata.Add("Header", message.Header);
536-
this.tracer.RelatedError(metadata, "HandleRequest: Unknown request");
534+
default:
535+
EventMetadata metadata = new EventMetadata();
536+
metadata.Add("Area", "Mount");
537+
metadata.Add("Header", message.Header);
538+
this.tracer.RelatedError(metadata, "HandleRequest: Unknown request");
537539

538-
connection.TrySendResponse(NamedPipeMessages.UnknownRequest);
539-
break;
540+
connection.TrySendResponse(NamedPipeMessages.UnknownRequest);
541+
break;
542+
}
543+
}
544+
catch (Exception e) when (e is not OutOfMemoryException)
545+
{
546+
EventMetadata metadata = new EventMetadata();
547+
metadata.Add("Area", "Mount");
548+
metadata.Add("Header", message.Header);
549+
metadata.Add("Exception", e.ToString());
550+
this.tracer.RelatedError(metadata, "HandleRequest: Unhandled exception in request handler");
540551
}
541552
}
542553

@@ -881,56 +892,76 @@ private void HandleDownloadObjectRequest(NamedPipeMessages.Message message, Name
881892
}
882893
else
883894
{
884-
Stopwatch downloadTime = Stopwatch.StartNew();
885-
886-
/* If this is the root tree for a commit that was was just downloaded, assume that more
887-
* trees will be needed soon and download them as well by using the download commit API.
888-
*
889-
* Otherwise, or as a fallback if the commit download fails, download the object directly.
890-
*/
891-
if (this.ShouldDownloadCommitPack(objectSha, out string commitSha)
892-
&& this.gitObjects.TryDownloadCommit(commitSha))
893-
{
894-
this.DownloadedCommitPack(commitSha);
895-
response = new NamedPipeMessages.DownloadObject.Response(NamedPipeMessages.DownloadObject.SuccessResult);
896-
// FUTURE: Should the stats be updated to reflect all the trees in the pack?
897-
// FUTURE: Should we try to clean up duplicate trees or increase depth of the commit download?
898-
}
899-
else if (this.gitObjects.TryDownloadAndSaveObject(objectSha, GVFSGitObjects.RequestSource.NamedPipeMessage) == GitObjects.DownloadAndSaveObjectResult.Success)
895+
try
900896
{
901-
this.UpdateTreesForDownloadedCommits(objectSha);
902-
response = new NamedPipeMessages.DownloadObject.Response(NamedPipeMessages.DownloadObject.SuccessResult);
897+
response = this.DownloadObject(objectSha);
903898
}
904-
else
899+
catch (Exception e) when (e is not OutOfMemoryException)
905900
{
901+
EventMetadata metadata = new EventMetadata();
902+
metadata.Add("Area", "Mount");
903+
metadata.Add("objectSha", objectSha);
904+
metadata.Add("Exception", e.ToString());
905+
this.tracer.RelatedWarning(metadata, nameof(this.HandleDownloadObjectRequest) + ": Exception downloading object");
906+
906907
response = new NamedPipeMessages.DownloadObject.Response(NamedPipeMessages.DownloadObject.DownloadFailed);
907908
}
909+
}
910+
}
908911

912+
connection.TrySendResponse(response.CreateMessage());
913+
}
909914

910-
Native.ObjectTypes? objectType;
911-
this.context.Repository.TryGetObjectType(objectSha, out objectType);
912-
this.context.Repository.GVFSLock.Stats.RecordObjectDownload(objectType == Native.ObjectTypes.Blob, downloadTime.ElapsedMilliseconds);
915+
private NamedPipeMessages.DownloadObject.Response DownloadObject(string objectSha)
916+
{
917+
NamedPipeMessages.DownloadObject.Response response;
918+
Stopwatch downloadTime = Stopwatch.StartNew();
913919

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

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

936967
private bool ShouldDownloadCommitPack(string objectSha, out string commitSha)
@@ -1124,6 +1155,7 @@ private void HandlePrefetchBlobsRequest(NamedPipeMessages.Message message, Named
11241155
request.Files,
11251156
request.Folders,
11261157
lastPrefetchArgs,
1158+
maxCacheSize: BlobPrefetcher.DefaultPrefetchCacheSize,
11271159
chunkSize: 4000,
11281160
searchThreadCount: maxThreads,
11291161
downloadThreadCount: downloadThreads,

0 commit comments

Comments
 (0)