From 6f92b4c12bffc25626324ec7d979f05a219b12c3 Mon Sep 17 00:00:00 2001 From: Drew Noakes Date: Mon, 10 Dec 2018 16:23:04 +0000 Subject: [PATCH] Avoid blocking UI via WebClient.OpenReadTaskAsync 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 #5859. Follows and replaces #5868. --- GitUI/Avatars/AvatarDownloader.cs | 19 +++++++++++++++++++ GitUI/UserControls/AvatarControl.cs | 2 ++ Plugins/Gource/GourceStart.cs | 3 +++ 3 files changed, 24 insertions(+) diff --git a/GitUI/Avatars/AvatarDownloader.cs b/GitUI/Avatars/AvatarDownloader.cs index 72271a9d86f..9ebd47a1fe8 100644 --- a/GitUI/Avatars/AvatarDownloader.cs +++ b/GitUI/Avatars/AvatarDownloader.cs @@ -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 @@ -25,6 +29,8 @@ public sealed class AvatarDownloader : IAvatarProvider */ private static readonly Regex _gitHubEmailRegex = new Regex(@"^(\d+\+)?(?[^@]+)@users\.noreply\.github\.com$", RegexOptions.Compiled | RegexOptions.IgnoreCase); + private static readonly SemaphoreSlim _downloadSemaphore = new SemaphoreSlim(initialCount: MaxConcurrentDownloads); + private static readonly ConcurrentDictionary)> _downloads = new ConcurrentDictionary)>(); /// @@ -146,6 +152,15 @@ async Task DownloadImageAsync(Uri imageUrl) async Task 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 }) @@ -163,6 +178,10 @@ async Task Download() // catch IO errors Trace.WriteLine(ex.Message); } + finally + { + _downloadSemaphore.Release(); + } return null; } diff --git a/GitUI/UserControls/AvatarControl.cs b/GitUI/UserControls/AvatarControl.cs index 142cd052855..9b7ed0d4568 100644 --- a/GitUI/UserControls/AvatarControl.cs +++ b/GitUI/UserControls/AvatarControl.cs @@ -104,6 +104,8 @@ private async Task UpdateAvatarAsync() if (!token.IsCancellationRequested) { + await this.SwitchToMainThreadAsync(); + RefreshImage(image); } } diff --git a/Plugins/Gource/GourceStart.cs b/Plugins/Gource/GourceStart.cs index b71478b4809..96be05222d0 100644 --- a/Plugins/Gource/GourceStart.cs +++ b/Plugins/Gource/GourceStart.cs @@ -73,6 +73,9 @@ private void Button1Click(object sender, EventArgs e) GitWorkingDir = WorkingDir.Text; RunRealCmdDetached(GourcePath.Text, arguments); + + await this.SwitchToMainThreadAsync(); + Close(); }); }