Skip to content

Bound HTTP body read waits and report transfer progress #2141

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Tyrie Vella (tyrielv) wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Bound HTTP body read waits and report transfer progress #2141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8586f20
Bound HTTP body read waits and report transfer progress
tyrielv Oct 19, 2026
847ce53
Preserve cancellation and response ownership across HTTP retries
tyrielv Oct 21, 2026
f9db2e7
fix: address review feedback - nonfatal body diagnostics
tyrielv Oct 28, 2026
14b635f
fix: address review feedback - distinguish prefetch-only clone failure
tyrielv Oct 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions GVFS/FastFetch/FastFetchVerb.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,14 +252,18 @@ private int ExecuteWithExitCode()
});

string error;
if (!enlistment.Authentication.TryInitialize(tracer, enlistment, out error))
RetryConfig retryConfig = RetryConfig.LoadFromGitConfig(tracer, enlistment);
if (!enlistment.Authentication.TryInitializeAndQueryGVFSConfig(
tracer, enlistment, retryConfig, out _, out error, out _))
{
tracer.RelatedError(error);
Console.WriteLine(error);
return ExitFailure;
}

RetryConfig retryConfig = new RetryConfig(this.MaxAttempts, TimeSpan.FromMinutes(RetryConfig.FetchAndCloneTimeoutMinutes));
retryConfig.MaxRetries = this.MaxAttempts;
retryConfig.Timeout = TimeSpan.FromMinutes(RetryConfig.FetchAndCloneTimeoutMinutes);

BlobPrefetcher prefetcher = this.GetFolderPrefetcher(tracer, enlistment, cacheServer, retryConfig);
if (!BlobPrefetcher.TryLoadFolderList(enlistment, this.FolderList, this.FolderListFile, prefetcher.FolderList, readListFromStdIn: false, error: out error))
{
Expand Down
1 change: 1 addition & 0 deletions GVFS/GVFS.Common/GVFSConstants.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public static class GitConfig
public const string GVFSPrefix = "gvfs.";
public const string MaxRetriesConfig = GVFSPrefix + "max-retries";
public const string TimeoutSecondsConfig = GVFSPrefix + "timeout-seconds";
public const string ReadTimeoutSecondsConfig = GVFSPrefix + "read-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";
Expand Down
4 changes: 3 additions & 1 deletion GVFS/GVFS.Common/Git/GitAuthentication.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,12 +222,14 @@ public bool TryGetCredentials(ITracer tracer, out string credentialString, out s
/// </summary>
public bool TryInitialize(ITracer tracer, Enlistment enlistment, out string errorMessage)
{
RetryConfig retryConfig = RetryConfig.LoadFromGitConfig(tracer, enlistment);

// Delegate to the combined method, discarding the config result.
// This avoids duplicating the anonymous-probe + credential-fetch logic.
return this.TryInitializeAndQueryGVFSConfig(
tracer,
enlistment,
new RetryConfig(),
retryConfig,
out _,
out errorMessage,
out _);
Expand Down
188 changes: 110 additions & 78 deletions GVFS/GVFS.Common/Git/GitObjects.cs
View file
Open in desktop

Large diffs are not rendered by default.

120 changes: 26 additions & 94 deletions GVFS/GVFS.Common/Http/GitEndPointResponseData.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using System.Net;
using System.Net.Http;
using System.Threading;
using System.Threading.Tasks;

namespace GVFS.Common.Http
{
Expand All @@ -32,7 +31,12 @@ public GitEndPointResponseData(HttpStatusCode statusCode, Exception error, bool
public GitEndPointResponseData(HttpStatusCode statusCode, string contentType, Stream responseStream, HttpResponseMessage message, Action onResponseDisposed)
: this(statusCode, null, false, message, onResponseDisposed)
{
this.Stream = responseStream == null ? null : new ReadErrorTrackingStream(responseStream);
this.Stream = responseStream == null
? null
: responseStream as HttpResponseStream ?? new HttpResponseStream(
responseStream,
TimeSpan.FromSeconds(RetryConfig.DefaultReadTimeoutSeconds),
CancellationToken.None);
this.ContentType = MapContentType(contentType);
}

Expand All @@ -44,9 +48,11 @@ public GitEndPointResponseData(HttpStatusCode statusCode, string contentType, St

public Stream Stream { get; private set; }

public string ResponseId => (this.Stream as HttpResponseStream)?.ResponseId;

public bool StreamReadFailed
{
get { return this.Stream is ReadErrorTrackingStream trackingStream && trackingStream.ReadFailed; }
get { return this.Stream is HttpResponseStream trackingStream && trackingStream.ReadFailed; }
}

public bool HasErrors
Expand Down Expand Up @@ -129,26 +135,29 @@ public void Dispose(bool disposing)
{
if (disposing)
{
if (this.message != null)
{
this.message.Dispose();
this.message = null;
}

if (this.Stream != null)
Stream stream = this.Stream;
HttpResponseMessage responseMessage = this.message;
Action releaseResponse = this.onResponseDisposed;
this.Stream = null;
this.message = null;
this.onResponseDisposed = null;
try
{
this.Stream.Dispose();
this.Stream = null;
stream?.Dispose();
}

if (this.onResponseDisposed != null)
finally
{
this.onResponseDisposed();
this.onResponseDisposed = null;
try
{
responseMessage?.Dispose();
}
finally
{
releaseResponse?.Invoke();
}
}
}
}

/// <summary>
/// Convert from a string-based Content-Type to <see cref="GitObjectsHttpRequestor.ContentType"/>
/// </summary>
Expand All @@ -167,82 +176,5 @@ private static GitObjectContentType MapContentType(string contentType)
}
}

private sealed class ReadErrorTrackingStream : Stream
{
private readonly Stream innerStream;

public ReadErrorTrackingStream(Stream innerStream)
{
this.innerStream = innerStream;
}

public bool ReadFailed { get; private set; }

public override bool CanRead => this.innerStream.CanRead;

public override bool CanSeek => this.innerStream.CanSeek;

public override bool CanWrite => this.innerStream.CanWrite;

public override long Length => this.innerStream.Length;

public override long Position
{
get { return this.innerStream.Position; }
set { this.innerStream.Position = value; }
}

public override void Flush() => this.innerStream.Flush();

public override int Read(byte[] buffer, int offset, int count) =>
this.TrackRead(() => this.innerStream.Read(buffer, offset, count));

public override int ReadByte() => this.TrackRead(this.innerStream.ReadByte);

public override Task<int> ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) =>
this.TrackReadAsync(() => this.innerStream.ReadAsync(buffer, offset, count, cancellationToken));

public override long Seek(long offset, SeekOrigin origin) => this.innerStream.Seek(offset, origin);

public override void SetLength(long value) => this.innerStream.SetLength(value);

public override void Write(byte[] buffer, int offset, int count) => this.innerStream.Write(buffer, offset, count);

protected override void Dispose(bool disposing)
{
if (disposing)
{
this.innerStream.Dispose();
}

base.Dispose(disposing);
}

private T TrackRead<T>(Func<T> read)
{
try
{
return read();
}
catch (Exception ex) when (!(ex is OperationCanceledException))
{
this.ReadFailed = true;
throw;
}
}

private async Task<T> TrackReadAsync<T>(Func<Task<T>> read)
{
try
{
return await read().ConfigureAwait(false);
}
catch (Exception ex) when (!(ex is OperationCanceledException))
{
this.ReadFailed = true;
throw;
}
}
}
}
}
84 changes: 73 additions & 11 deletions GVFS/GVFS.Common/Http/HttpRequestor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,15 @@ protected GitEndPointResponseData SendRequest(

responseMetadata.Add("CacheName", GetSingleHeaderOrEmpty(response.Headers, "X-Cache-Name"));
responseMetadata.Add("StatusCode", response.StatusCode);
EventMetadata bodyMetadata = new EventMetadata
{
{ "RequestId", requestId },
{ "StatusCode", response.StatusCode },
{ "CacheName", GetCacheNameForTelemetry((string)responseMetadata["CacheName"]) },
{ "RequestAuthority", GetAuthorityForTelemetry(requestUri) },
{ "HeaderWaitMs", responseWaitTime.TotalMilliseconds },
{ "ContentLength", response.Content.Headers.ContentLength ?? -1 },
};

if (response.StatusCode == HttpStatusCode.OK)
{
Expand All @@ -210,18 +219,47 @@ protected GitEndPointResponseData SendRequest(
this.authentication.ApproveCredentials(this.Tracer, authString);
}

Stream responseStream = response.Content.ReadAsStreamAsync().GetAwaiter().GetResult();
Stream responseStream = response.Content.ReadAsStreamAsync(cancellationToken).GetAwaiter().GetResult();

gitEndPointResponseData = new GitEndPointResponseData(
response.StatusCode,
contentType,
responseStream,
new HttpResponseStream(
responseStream,
this.RetryConfig.ReadTimeout,
cancellationToken,
this.Tracer,
bodyMetadata),
Comment on lines +227 to +232

@KeithIsSleeping Keith Klein (KeithIsSleeping) Oct 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[KeithBot]: 🟡 Medium · Reliability — Cancelled hydration aborts shared blob download; other ProjFS reads can hang

Body reads now honor the request's CancellationToken, but GVFSGitObjects.TryDownloadAndSaveObject runs one shared download per SHA under the first caller's token. Cancelling that caller mid-body (e.g. its process is killed) fails every coalesced waiter: a second ProjFS GetFileData for the same blob is never completed, so that reader hangs, and a joined git read-object request gets DownloadFailed. At the merge base both waiters succeed.

Evidence:
Scratch NUnit fixture: real GVFSGitObjects + GitObjectsHttpRequestor against a loopback server streaming the object in 64-byte chunks every 50 ms, plus the real WindowsFileSystemVirtualizer on MockVirtualizationInstance; GetFileData commands 1 and 2 for one SHA, then OnCancelCommand(1) mid-body.

> GVFS.UnitTests.exe --test=GVFS.UnitTests.Windows.Virtualization.CoalescedCancelRepro
head: RESULT projfs ... command2Completed=False completions=[] cancelledEvents=2
base: RESULT projfs ... command2Completed=True completions=[2:Ok] cancelledEvents=1
head: RESULT direct secondIsNamedPipe=True ... second=EXCEPTION TaskCanceledException
base: RESULT direct secondIsNamedPipe=True ... second=RETURNED Success

Fix:
In GVFSGitObjects.TryDownloadAndSaveObject, restart the download instead of inheriting another caller's cancellation (add before the existing finally):

catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested)
{
    this.TryRemoveInflightDownload(objectId, lazy);
    return this.TryDownloadAndSaveObject(objectId, cancellationToken, requestSource, retryOnFailure);
}

🤖 KeithBot · 1/3 models · verified locally

message: response,
onResponseDisposed: () => availableConnections.Release());
}
else
{
errorMessage = response.Content.ReadAsStringAsync().GetAwaiter().GetResult();
string bodyReadFailure = null;
try
{
using (HttpResponseStream responseStream = new HttpResponseStream(
response.Content.ReadAsStreamAsync(cancellationToken).GetAwaiter().GetResult(),
this.RetryConfig.ReadTimeout,
cancellationToken,
this.Tracer,
bodyMetadata))
using (StreamContent content = new StreamContent(responseStream))
{
content.Headers.ContentType = response.Content.Headers.ContentType;
errorMessage = content.ReadAsStringAsync(cancellationToken).GetAwaiter().GetResult();
}
}
catch (Exception e) when (e is IOException || e is HttpRequestException)
{
cancellationToken.ThrowIfCancellationRequested();
this.Tracer.RelatedWarning(
new EventMetadata(bodyMetadata) { { "Exception", e.ToString() } },
"The server returned an error status, but the response body read failed.",
Keywords.Network | Keywords.Telemetry);
bodyReadFailure = "The response body read failed: " + (e.InnerException?.Message ?? e.Message);
errorMessage = bodyReadFailure;
}
int statusInt = (int)response.StatusCode;

bool shouldRetry = ShouldRetry(response.StatusCode);
Expand All @@ -231,6 +269,10 @@ protected GitEndPointResponseData SendRequest(
{
shouldRetry = false;
errorMessage = "Anonymous request was rejected with a 401";
if (bodyReadFailure != null)
{
errorMessage += ". " + bodyReadFailure;
}
}
else if (ShouldRejectCredentials(response.StatusCode, errorMessage))
{
Expand Down Expand Up @@ -294,17 +336,28 @@ protected GitEndPointResponseData SendRequest(
responseMetadata.Add("connectionWaitTimeMS", $"{connectionWaitTimeElapsed.TotalMilliseconds:F4}");
responseMetadata.Add("responseWaitTimeMS", $"{responseWaitTime.TotalMilliseconds:F4}");

this.Tracer.RelatedEvent(EventLevel.Informational, "NetworkResponse", responseMetadata);

if (gitEndPointResponseData == null)
try
{
this.Tracer.RelatedEvent(EventLevel.Informational, "NetworkResponse", responseMetadata);
}
catch
{
// If gitEndPointResponseData is null there was an unhandled exception
if (response != null)
gitEndPointResponseData?.Dispose();
throw;
}
finally
{
if (gitEndPointResponseData == null)
{
response.Dispose();
try
{
response?.Dispose();
}
finally
{
availableConnections.Release();
}
}

availableConnections.Release();
}
}

Expand Down Expand Up @@ -383,6 +436,15 @@ internal static string GetAuthorityForTelemetry(Uri uri)
return uri.Authority;
}

internal static string GetCacheNameForTelemetry(string cacheName)
{
const int MaxCacheNameLength = 128;
return new string(cacheName.Take(MaxCacheNameLength).Select(
character => char.IsLetterOrDigit(character) || character == '-' || character == '_' || character == '.'
? character
: '_').ToArray());
}

private static string GetSingleHeaderOrEmpty(HttpHeaders headers, string headerName)
{
IEnumerable<string> values;
Expand Down
Loading