Squirrel / Squirrel.Windows

An installation and update framework for Windows desktop apps
MIT License
7.42k stars 1.03k forks source link

FileDownloader's providedClient is used concurrently thus can throw "WebClient does not support concurrent I/O operations" #880

Open szanto90balazs opened 8 years ago

szanto90balazs commented 8 years ago

I'm getting exceptions originating from UpdateManager.DownloadReleases with the following message

WebClient does not support concurrent I/O operations

and it's because I use my own ✨WebClient✨ instance.

In the FileDownloader's constructor a WebClient providedClient can be specified. Because UpdateManager.DownloadReleases downloads the packages parallel, FileDownloader.DownloadFile can be called concurrently, which uses a "singleton" WebClient concurrently, however WebClient only supports a single operation, it cannot download multiple files at once, that's why it's throwing exceptions.

The file downloader works by default because Utility.CreateWebClient(); is called on every DownloadFile.

public class FileDownloader : IFileDownloader, IEnableLogger
{
    private readonly WebClient _providedClient;

    public FileDownloader(WebClient providedClient = null)
    {
        this._providedClient = providedClient;
    }

    public async Task DownloadFile(string url, string targetFile, Action<int> progress)
    {
        WebClient wc = this._providedClient ?? Utility.CreateWebClient();

        ...

I can workaround this in my codebase, but I'd like to change it in Squirrel to something like this below. What do you think?

public class FileDownloader : IFileDownloader, IEnableLogger
{
    private readonly Func<WebClient> _webClientFactory;

    public FileDownloader(Func<WebClient> providedClientFactory = null)
    {
        this._webClientFactory = providedClientFactory ?? Utility.CreateWebClient
    }

    public async Task DownloadFile(string url, string targetFile, Action<int> progress)
    {
        WebClient wc = _webClientFactory(); 

        ...
shiftkey commented 5 years ago

Related to #886