Skip to content

Commit

Permalink
Avoid blocking UI via WebClient.OpenReadTaskAsync
Browse files Browse the repository at this point in the history
This method can block synchronously. This change moves calls to the
thread pool, and limits the number of concurrent downloads to prevent
thread pool exhaustion.

Fixes gitextensions#5859.

Follows and replaces gitextensions#5868.
  • Loading branch information
drewnoakes committed Dec 10, 2018
1 parent a18967d commit 6f92b4c
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 0 deletions.
19 changes: 19 additions & 0 deletions GitUI/Avatars/AvatarDownloader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,17 @@
using System.Security.Cryptography;
using System.Text;
using System.Text.RegularExpressions;
using System.Threading;
using System.Threading.Tasks;
using GitCommands;
using Microsoft.VisualStudio.Threading;

namespace GitUI.Avatars
{
public sealed class AvatarDownloader : IAvatarProvider
{
private const int MaxConcurrentDownloads = 10;

/* A brief skim through the GitExtensions repo history shows GitHub emails with the following user names:
*
* 25421792+mserfli
Expand All @@ -25,6 +29,8 @@ public sealed class AvatarDownloader : IAvatarProvider
*/
private static readonly Regex _gitHubEmailRegex = new Regex(@"^(\d+\+)?(?<username>[^@]+)@users\.noreply\.github\.com$", RegexOptions.Compiled | RegexOptions.IgnoreCase);

private static readonly SemaphoreSlim _downloadSemaphore = new SemaphoreSlim(initialCount: MaxConcurrentDownloads);

private static readonly ConcurrentDictionary<Uri, (DateTime, Task<Image>)> _downloads = new ConcurrentDictionary<Uri, (DateTime, Task<Image>)>();

/// <inheritdoc />
Expand Down Expand Up @@ -146,6 +152,15 @@ async Task<Image> DownloadImageAsync(Uri imageUrl)

async Task<Image> Download()
{
// WebClient.OpenReadTaskAsync can block before returning a task, so make sure we
// make such calls on the thread pool, and limit the number of concurrent requests.

// Get onto background thread
await TaskScheduler.Default;

// Limit the number of concurrent download requests
await _downloadSemaphore.WaitAsync();

try
{
using (var webClient = new WebClient { Proxy = WebRequest.DefaultWebProxy })
Expand All @@ -163,6 +178,10 @@ async Task<Image> Download()
// catch IO errors
Trace.WriteLine(ex.Message);
}
finally
{
_downloadSemaphore.Release();
}

return null;
}
Expand Down
2 changes: 2 additions & 0 deletions GitUI/UserControls/AvatarControl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ private async Task UpdateAvatarAsync()

if (!token.IsCancellationRequested)
{
await this.SwitchToMainThreadAsync();

RefreshImage(image);
}
}
Expand Down
3 changes: 3 additions & 0 deletions Plugins/Gource/GourceStart.cs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ private void Button1Click(object sender, EventArgs e)
GitWorkingDir = WorkingDir.Text;
RunRealCmdDetached(GourcePath.Text, arguments);
await this.SwitchToMainThreadAsync();
Close();
});
}
Expand Down

0 comments on commit 6f92b4c

Please sign in to comment.