diff --git a/src/modules/cmdpal/Microsoft.CmdPal.Common/Helpers/InterlockedBoolean.cs b/src/modules/cmdpal/Microsoft.CmdPal.Common/Helpers/InterlockedBoolean.cs new file mode 100644 index 0000000000..8113ef9990 --- /dev/null +++ b/src/modules/cmdpal/Microsoft.CmdPal.Common/Helpers/InterlockedBoolean.cs @@ -0,0 +1,46 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.Threading; + +namespace Microsoft.CmdPal.Common.Helpers; + +/// +/// Thread-safe boolean implementation using atomic operations +/// +public struct InterlockedBoolean(bool initialValue = false) +{ + private int _value = initialValue ? 1 : 0; + + /// + /// Gets or sets the boolean value atomically + /// + public bool Value + { + get => Volatile.Read(ref _value) == 1; + set => Interlocked.Exchange(ref _value, value ? 1 : 0); + } + + /// + /// Atomically sets the value to true + /// + /// True if the value was previously false, false if it was already true + public bool Set() + { + return Interlocked.Exchange(ref _value, 1) == 0; + } + + /// + /// Atomically sets the value to false + /// + /// True if the value was previously true, false if it was already false + public bool Clear() + { + return Interlocked.Exchange(ref _value, 0) == 1; + } + + public override int GetHashCode() => Value.GetHashCode(); + + public override string ToString() => Value.ToString(); +} diff --git a/src/modules/cmdpal/Microsoft.CmdPal.Core.ViewModels/ListViewModel.cs b/src/modules/cmdpal/Microsoft.CmdPal.Core.ViewModels/ListViewModel.cs index 57ae504dff..8d54dd141f 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.Core.ViewModels/ListViewModel.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.Core.ViewModels/ListViewModel.cs @@ -6,6 +6,7 @@ using System.Collections.ObjectModel; using CommunityToolkit.Mvvm.ComponentModel; using CommunityToolkit.Mvvm.Input; using CommunityToolkit.Mvvm.Messaging; +using Microsoft.CmdPal.Common.Helpers; using Microsoft.CmdPal.Core.ViewModels.Messages; using Microsoft.CmdPal.Core.ViewModels.Models; using Microsoft.CommandPalette.Extensions; @@ -31,7 +32,7 @@ public partial class ListViewModel : PageViewModel, IDisposable private readonly Lock _listLock = new(); - private bool _isLoading; + private InterlockedBoolean _isLoading; private bool _isFetching; public event TypedEventHandler? ItemsUpdated; @@ -121,7 +122,7 @@ public partial class ListViewModel : PageViewModel, IDisposable ItemsUpdated?.Invoke(this, EventArgs.Empty); UpdateEmptyContent(); - _isLoading = false; + _isLoading.Clear(); } } @@ -221,7 +222,7 @@ public partial class ListViewModel : PageViewModel, IDisposable } ItemsUpdated?.Invoke(this, EventArgs.Empty); - _isLoading = false; + _isLoading.Clear(); }); } @@ -469,21 +470,38 @@ public partial class ListViewModel : PageViewModel, IDisposable return; } - if (model.HasMoreItems && !_isLoading) + if (!_isLoading.Set()) { - _isLoading = true; - _ = Task.Run(() => + return; + + // NOTE: May miss newly available items until next scroll if model + // state changes between our check and this reset + } + + _ = Task.Run(() => + { + // Execute all COM calls on background thread to avoid reentrancy issues with UI + // with the UI thread when COM starts inner message pump + try { - try + if (model.HasMoreItems) { model.LoadMore(); + + // _isLoading flag will be set as a result of LoadMore, + // which must raise ItemsChanged to end the loading. } - catch (Exception ex) + else { - ShowException(ex, model.Name); + _isLoading.Clear(); } - }); - } + } + catch (Exception ex) + { + _isLoading.Clear(); + ShowException(ex, model.Name); + } + }); } protected override void FetchProperty(string propertyName)