如何在使用Winforms应用程序中的单例Http客户端时避免死锁

huangapple go评论57阅读模式
英文:

How to avoid Deadlock when using singleton Http Client in Winforms application

问题

以下是您提供的代码的翻译:

我有一个传统的Windows Forms应用程序,我正在进行开发,我对http客户端进行了一些更改,我希望将其设为单例,以便在整个应用程序中重复使用。似乎这导致了一个死锁。

我将粘贴我认为涉及的所有代码如下:

这是调用代码,其中UI被冻结,永远不会解冻。

```csharp
private async void lbGroup_SelectedIndexChanged_1(object sender, EventArgs e)
{
    int groupId = this.lbGroup.SelectedIndex + 1;
    await LoadStores(groupId);

    // 下面的代码导致了应用程序冻结
    this.lbStore.DataSource = _stores;
    
    this.txtSearch.Enabled = true;
    this.lbStore.Enabled = true;
}

这是使用httpClient的LoadStores方法:

private async Task LoadStores(int group)
{
    try
    {
        HttpResponseMessage res = await _httpClient.GetAsync("api/GetStoresByGroup/" + group.ToString());
        res.EnsureSuccessStatusCode();
        if (res.IsSuccessStatusCode)
        {
            var serializedStores = await res.Content.ReadAsStringAsync();
            _stores = JsonConvert.DeserializeObject<IEnumerable<Store>>(serializedStores).Select(s => s.StoreName).ToList();
            res.Content.Dispose();
        }
    }
    catch (Exception ex)
    {
        ErrorLogger.LogError("Installation", $"获取商店列表时出错:{ex.Message}");
    }
}

这是Http单例类:

public static class HttpClientSingleton
{
    private static readonly HttpClient _instance;

    static HttpClientSingleton()
    {
        _instance = new HttpClient();
        _instance.BaseAddress = new Uri("https://www.i-city.co.za/");
        _instance.DefaultRequestHeaders.Accept.Clear();
        _instance.DefaultRequestHeaders.Accept.Add(new System.Net.Http.Headers.MediaTypeWithQualityHeaderValue("application/json"));
    }

    public static HttpClient Instance
    {
        get
        {
            return _instance;
        }
    }
}

这是窗体构造函数,其中初始化了HttpClient:

public partial class frmInstallationHelper : Form
{
    private static string _configDir;
    private static string _localConfigDir;
    private static int _storeID;
    private static Activation _activation;
    private static HttpClient _httpClient = HttpClientSingleton.Instance;
    private static IEnumerable<string> _stores;
    private static IEnumerable<string> _franchisees;
    private int _smsCounter;
}

如果我在LoadStores方法内部将http请求包装在using语句中,应用程序会正常运行,但我不想释放http Client,因为那违背了将其设为单例的目的。

更新:找到问题

在遵循@MongZhu的建议后,我复制了该程序并确认上述任何代码实际上都没有导致死锁。死锁是由另一个方法引起的,该方法由lbStore列表框的onSelectChange事件触发,如下所示:

private void lbStore_SelectedIndexChanged_1(object sender, EventArgs e)
{
    string store = this.lbStore.GetItemText(this.lbStore.SelectedItem);
    LoadFranchisees(store).Wait();
    this.lbFranchisees.DataSource = _franchisees;
}

我解决问题的方法是将其更改为如下所示:

private async void lbStore_SelectedIndexChanged_1(object sender, EventArgs e)
{
    string store = this.lbStore.GetItemText(this.lbStore.SelectedItem);
    await LoadFranchisees(store);
    this.lbFranchisees.DataSource = _franchisees;
}

我一直在将所有的.Wait()方法改成异步/等待,可能忘记了这个。

英文:

I have a legacy Windows Forms application that I am working on, I made some changes to the http client, I wanted to make it a singleton so that it could be reused throughout the application. It seems to be causing a deadlock.

I am going to paste all the code that I believe is involved below:

This is the calling code where the UI gets frozen, it never unfreezes.

private async void lbGroup_SelectedIndexChanged_1(object sender, EventArgs e)
{
    int groupId = this.lbGroup.SelectedIndex + 1;
    await LoadStores(groupId);

    //The code below freezes the application
    this.lbStore.DataSource = _stores;
    
    this.txtSearch.Enabled = true;
    this.lbStore.Enabled = true;
}

This is the LoadStores Method where the httpClient is used:

private async Task LoadStores(int group)
{
    try
    {
        HttpResponseMessage res = await _httpClient.GetAsync(&quot;api/GetStoresByGroup/&quot; + group.ToString());
        res.EnsureSuccessStatusCode();
        if (res.IsSuccessStatusCode)
        {
            var serializedStores = await res.Content.ReadAsStringAsync();
            _stores = JsonConvert.DeserializeObject&lt;IEnumerable&lt;Store&gt;&gt;(serializedStores).Select(s =&gt; s.StoreName).ToList();
            res.Content.Dispose();
        }
    }
    catch (Exception ex)
    {
        ErrorLogger.LogError(&quot;Installation&quot;, $&quot;Error getting stores list: {ex.Message}&quot;);
    }
}

This is the Http Singleton Class:

public static class HttpClientSingleton
{
    private static readonly HttpClient _instance;

    static HttpClientSingleton()
    {
        _instance = new HttpClient();
        _instance.BaseAddress = new Uri(&quot;https://www.i-city.co.za/&quot;);
        _instance.DefaultRequestHeaders.Accept.Clear();
        _instance.DefaultRequestHeaders.Accept.Add(new System.Net.Http.Headers.MediaTypeWithQualityHeaderValue(&quot;application/json&quot;));
    }

    public static HttpClient Instance
    {
        get
        {
            return _instance;
        }
    }
}

This is the form constructor where the HttpClient gets initiliazed:

public partial class frmInstallationHelper : Form
{
    private static string _configDir;
    private static string _localConfigDir;
    private static int _storeID;
    private static Activation _activation;
    private static HttpClient _httpClient = HttpClientSingleton.Instance;
    private static IEnumerable&lt;string&gt; _stores;
    private static IEnumerable&lt;string&gt; _franchisees;
    private int _smsCounter;

If I wrap the http request in a using statement inside of the LoadStores method, the app runs fine, but I don't want to dispose of the http Client as that defeats the purpose of making it a singleton.

Update: Problem Found

After following @MongZhu's lead I replicated the program and confirmed that none of the above code was actually causing the deadlock. It was caused by another method that was triggered by the lbStore list Box onSelectChange event displayd below:

private void lbStore_SelectedIndexChanged_1(object sender, EventArgs e)
    {
        string store = this.lbStore.GetItemText(this.lbStore.SelectedItem);
        LoadFranchisees(store).Wait();
        this.lbFranchisees.DataSource = _franchisees;
    }

The way I solved the problem was by changing it to look as follows:

private async void lbStore_SelectedIndexChanged_1(object sender, EventArgs e)
    {
         string store = this.lbStore.GetItemText(this.lbStore.SelectedItem);
         await LoadFranchisees(store);
         this.lbFranchisees.DataSource = _franchisees;
    }

I was busy changing all the .wait() methods to async / await, and I must have forgotten this one.

答案1

得分: 2

以下是您要翻译的内容:

The deadlock arises because you used Wait in a method which was triggered by an async operation. Unfortunately it was masked very well by the apparent hanging in the line of the initialization of the DataSource. But this initialization triggered the SelectedIndexChanged of the listbox which had the evil Wait call in it. Making this method async and await the result will evaporate the deadlock.

死锁的原因是因为您在由异步操作触发的方法中使用了 Wait。不幸的是,在 DataSource 的初始化行中,这一点被掩盖得很好。但这个初始化触发了列表框的 SelectedIndexChanged 事件,而这个事件中包含了有害的 Wait 调用。将这个方法改为异步并等待结果将消除死锁。

private async void lbStore_SelectedIndexChanged_1(object sender, EventArgs e)
{
    string store = this.lbStore.GetItemText(this.lbStore.SelectedItem);
    _franchisees = await LoadFranchisees(store);
    this.lbFranchisees.DataSource = _franchisees;
}

我建议直接从方法中返回 stores,而不是使用类变量作为传输器。这样,您还可以避免竞态条件(使用类变量的方法很容易出现竞态条件)。如果需要,您可以将返回的值存储在 _stores 变量中。但一个加载数据的方法应该返回结果,而不是将它秘密地存储在方法的用户不可见的地方。

private async Task<List<Store>> LoadStores(int group)
{
    try
    {
        HttpResponseMessage res = await _httpClient.GetAsync("api/GetStoresByGroup/" + group.ToString());
        res.EnsureSuccessStatusCode();
        if (res.IsSuccessStatusCode)
        {
            var serializedStores = await res.Content.ReadAsStringAsync();
            res.Content.Dispose();
            return JsonConvert.DeserializeObject<IEnumerable<Store>>(serializedStores).Select(s => s.StoreName).ToList();
        }
    }
    catch (Exception ex)
    {
        ErrorLogger.LogError("Installation", $"Error getting stores list: {ex.Message}");
    }
}

您可以在事件中等待结果:

private async void lbGroup_SelectedIndexChanged_1(object sender, EventArgs e)
{
    int groupId = this.lbGroup.SelectedIndex + 1;
    _stores = await LoadStores(groupId);
    this.lbStore.DataSource = _stores;
    
    this.txtSearch.Enabled = true;
    this.lbStore.Enabled = true;
}

相同的逻辑适用于 LoadFranchisees 方法,重构它以返回数据。这将使您的代码更容易理解。不要隐藏方法的信息。也许在 6 个月后,您会尝试弄清楚您在那里做了什么... 至少要善待未来的自己 如何在使用Winforms应用程序中的单例Http客户端时避免死锁

英文:

The deadlock arises because you used Wait in a method which was triggered by an async opertaion. Unfortunately it was masked very good by the apparent hanging in the line of the initialization of the DataSource. But this initialization triggered the SelectedIndexChanged of the listbox which had the evil Wait call in it. Making this method async and await the result will evaporate the deadlock.

private async void lbStore_SelectedIndexChanged_1(object sender, EventArgs e)
{
    string store = this.lbStore.GetItemText(this.lbStore.SelectedItem);
    _franchisees = await LoadFranchisees(store);
    this.lbFranchisees.DataSource = _franchisees;
}

I would suggest to return the stores directly from the method instead of using a class variable as transmitter. This way you would also avoid race conditions (to which methods that use class variables are very much prone) If you need it further you could store the returning value inside the _stores variable. But a loading method should rather return the results instead of secretely storing it somewhere hidden from the user of this method.

private async Task&lt;List&lt;Store&gt;&gt; LoadStores(int group)
{
    try
    {
        HttpResponseMessage res = await _httpClient.GetAsync(&quot;api/GetStoresByGroup/&quot; + group.ToString()))
        res.EnsureSuccessStatusCode();
        if (res.IsSuccessStatusCode)
        {
            var serializedStores = await res.Content.ReadAsStringAsync();
            res.Content.Dispose();
            return JsonConvert.DeserializeObject&lt;IEnumerable&lt;Store&gt;&gt;(serializedStores).Select(s =&gt; s.StoreName).ToList();

        }
    }
    catch (Exception ex)
    {
        ErrorLogger.LogError(&quot;Installation&quot;, $&quot;Error getting stores list: {ex.Message}&quot;);
    }
}

You can await the result in the event:

private async void lbGroup_SelectedIndexChanged_1(object sender, EventArgs e)
{
    int groupId = this.lbGroup.SelectedIndex + 1;
    _stores = await LoadStores(groupId);
    this.lbStore.DataSource = _stores;
    
    this.txtSearch.Enabled = true;
    this.lbStore.Enabled = true;
}

The same logic applies to the LoadFranchisees method, refactor it so that it returns the data. This makes your code much more understandable. Don't hide information from the reader of a method. It could be you in 6 Month trying to figure out what da heck you did there.... Be nice to your future self at least 如何在使用Winforms应用程序中的单例Http客户端时避免死锁

huangapple
  • 本文由 发表于 2023年2月8日 16:02:49
  • 转载请务必保留本文链接:https://go.coder-hub.com/75382816.html
匿名

发表评论

匿名网友

:?: :razz: :sad: :evil: :!: :smile: :oops: :grin: :eek: :shock: :???: :cool: :lol: :mad: :twisted: :roll: :wink: :idea: :arrow: :neutral: :cry: :mrgreen:

确定