Merge #3914 Add test to check GUI thread safety

CHANGELOG.md

@@ -37,6 +37,7 @@ All notable changes to this project will be documented in this file.
 - [Tooling] Deduce primary branch name in merge script (#3884 by: HebaruSan; reviewed: techman83)
 - [CLI] Parse quoted strings for `ckan prompt` (#3889 by: HebaruSan; reviewed: techman83)
 - [Build] Remove log4net, newtonsoft deps from deb package (#3900 by: HebaruSan; reviewed: techman83)
+- [GUI] Add test to check GUI thread safety (#3914 by: HebaruSan; reviewed: techman83)
 
 ## v1.33.2 (Laplace)
 

Cmdline/Action/GameInstance.cs

@@ -104,20 +104,20 @@ internal class CloneOptions : CommonOptions
 
     internal class RenameOptions : CommonOptions
     {
-        [GameInstancesAttribute]
+        [GameInstances]
         [ValueOption(0)] public string old_name { get; set; }
         [ValueOption(1)] public string new_name { get; set; }
     }
 
     internal class ForgetOptions : CommonOptions
     {
-        [GameInstancesAttribute]
+        [GameInstances]
         [ValueOption(0)] public string name { get; set; }
     }
 
     internal class DefaultOptions : CommonOptions
     {
-        [GameInstancesAttribute]
+        [GameInstances]
         [ValueOption(0)] public string name { get; set; }
     }
 

Core/Extensions/EnumerableExtensions.cs

@@ -16,13 +16,18 @@ public static ICollection<T> AsCollection<T>(this IEnumerable<T> source)
         }
 
 #if NET45
+
         public static HashSet<T> ToHashSet<T>(this IEnumerable<T> source)
         {
             if (source == null)
                 throw new ArgumentNullException(nameof(source));
 
             return new HashSet<T>(source);
         }
+
+        public static IEnumerable<T> Append<T>(this IEnumerable<T> source, T next)
+            => source.Concat(Enumerable.Repeat<T>(next, 1));
+
 #endif
 
         public static Dictionary<K, V> ToDictionary<K, V>(this IEnumerable<KeyValuePair<K, V>> pairs)

GUI/Attributes/ForbidGUICallsAttribute.cs

@@ -0,0 +1,13 @@
+using System;
+
+namespace CKAN.GUI.Attributes
+{
+    /// <summary>
+    /// Tag background functions with this, and a test will check
+    /// that they're not calling GUI code without Util.Invoke.
+    /// </summary>
+    [AttributeUsage(AttributeTargets.Constructor | AttributeTargets.Property
+                    | AttributeTargets.Method    | AttributeTargets.Delegate,
+                    Inherited = true, AllowMultiple = false)]
+    public class ForbidGUICallsAttribute : Attribute { }
+}

GUI/CKAN-GUI.csproj

@@ -52,6 +52,7 @@
     <Reference Include="System.Xml" />
   </ItemGroup>
   <ItemGroup>
+    <Compile Include="Attributes\ForbidGUICallsAttribute.cs" />
     <Compile Include="Dialogs\AboutDialog.cs">
       <SubType>Form</SubType>
     </Compile>

GUI/Controls/ChooseProvidedMods.cs

@@ -4,6 +4,8 @@
 using System.Collections.Generic;
 using System.Windows.Forms;
 
+using CKAN.GUI.Attributes;
+
 namespace CKAN.GUI
 {
     public partial class ChooseProvidedMods : UserControl
@@ -13,6 +15,7 @@ public ChooseProvidedMods()
             InitializeComponent();
         }
 
+        [ForbidGUICalls]
         public void LoadProviders(string message, List<CkanModule> modules, NetModuleCache cache)
         {
             Util.Invoke(this, () =>
@@ -36,6 +39,7 @@ public void LoadProviders(string message, List<CkanModule> modules, NetModuleCac
             });
         }
 
+        [ForbidGUICalls]
         public CkanModule Wait()
         {
             task = new TaskCompletionSource<CkanModule>();

GUI/Controls/ChooseRecommendedMods.cs

@@ -4,8 +4,10 @@
 using System.Linq;
 using System.Threading.Tasks;
 using System.Windows.Forms;
+
 using CKAN.Extensions;
 using CKAN.Versioning;
+using CKAN.GUI.Attributes;
 
 namespace CKAN.GUI
 {
@@ -16,6 +18,7 @@ public ChooseRecommendedMods()
             InitializeComponent();
         }
 
+        [ForbidGUICalls]
         public void LoadRecommendations(
             IRegistryQuerier registry,
             List<CkanModule> toInstall, HashSet<CkanModule> toUninstall,
@@ -38,6 +41,7 @@ Dictionary<CkanModule, HashSet<string>> supporters
             });
         }
 
+        [ForbidGUICalls]
         public HashSet<CkanModule> Wait()
         {
             if (Platform.IsMono)

GUI/Controls/DeleteDirectories.cs

@@ -4,7 +4,9 @@
 using System.Threading.Tasks;
 using System.Collections.Generic;
 using System.Windows.Forms;
+
 using CKAN.Extensions;
+using CKAN.GUI.Attributes;
 
 namespace CKAN.GUI
 {
@@ -21,6 +23,7 @@ public DeleteDirectories()
         /// before the calling code switches to the tab.
         /// </summary>
         /// <param name="possibleConfigOnlyDirs">Directories that the user may want to delete</param>
+        [ForbidGUICalls]
         public void LoadDirs(GameInstance ksp, HashSet<string> possibleConfigOnlyDirs)
         {
             instance = ksp;
@@ -50,6 +53,7 @@ public void LoadDirs(GameInstance ksp, HashSet<string> possibleConfigOnlyDirs)
         /// <returns>
         /// true if user chose to delete, false otherwise
         /// </returns>
+        [ForbidGUICalls]
         public bool Wait(out HashSet<string> toDelete)
         {
             if (Platform.IsMono)

GUI/Controls/EditModpack.cs

@@ -8,6 +8,7 @@
 
 using CKAN.Versioning;
 using CKAN.Types;
+using CKAN.GUI.Attributes;
 
 namespace CKAN.GUI
 {
@@ -33,6 +34,7 @@ public EditModpack()
             this.ToolTip.SetToolTip(ExportModpackButton,     Properties.Resources.EditModpackTooltipExport);
         }
 
+        [ForbidGUICalls]
         public void LoadModule(CkanModule module, IRegistryQuerier registry)
         {
             this.module = module;
@@ -64,6 +66,7 @@ public void LoadModule(CkanModule module, IRegistryQuerier registry)
 
         public event Action<ListView.SelectedListViewItemCollection> OnSelectedItemsChanged;
 
+        [ForbidGUICalls]
         public bool Wait(IUser user)
         {
             if (Platform.IsMono)

GUI/Controls/ManageMods.cs

@@ -12,6 +12,7 @@
 
 using CKAN.Extensions;
 using CKAN.Versioning;
+using CKAN.GUI.Attributes;
 
 namespace CKAN.GUI
 {
@@ -102,6 +103,7 @@ private List<string> sortColumns
         private List<ModChange> ChangeSet
         {
             get => currentChangeSet;
+            [ForbidGUICalls]
             set
             {
                 var orig = currentChangeSet;
@@ -111,6 +113,7 @@ private List<ModChange> ChangeSet
             }
         }
 
+        [ForbidGUICalls]
         private void ChangeSetUpdated()
         {
             Util.Invoke(this, () =>
@@ -149,12 +152,15 @@ private void ChangeSetUpdated()
         private Dictionary<GUIMod, string> Conflicts
         {
             get => conflicts;
+            [ForbidGUICalls]
             set
             {
                 var orig = conflicts;
                 conflicts = value;
                 if (orig != value)
-                    ConflictsUpdated(orig);
+                {
+                    Util.Invoke(this, () => ConflictsUpdated(orig));
+                }
             }
         }
 
@@ -1033,6 +1039,7 @@ private void reinstallToolStripMenuItem_Click(object sender, EventArgs e)
             }
         }
 
+        [ForbidGUICalls]
         public Dictionary<string, GUIMod> AllGUIMods()
             => mainModList.Modules.ToDictionary(guiMod => guiMod.Identifier,
                                                 guiMod => guiMod);
@@ -1087,6 +1094,7 @@ private void EditModSearches_SurrenderFocus()
             Util.Invoke(this, () => ModGrid.Focus());
         }
 
+        [ForbidGUICalls]
         public void UpdateFilters()
         {
             Util.Invoke(this, _UpdateFilters);
@@ -1135,11 +1143,13 @@ private void _UpdateFilters()
             }
         }
 
+        [ForbidGUICalls]
         public void Update(object sender, DoWorkEventArgs e)
         {
             e.Result = _UpdateModsList(e.Argument as Dictionary<string, bool>);
         }
 
+        [ForbidGUICalls]
         private bool _UpdateModsList(Dictionary<string, bool> old_modules = null)
         {
             log.Info("Updating the mod list");
@@ -1275,6 +1285,7 @@ private bool _UpdateModsList(Dictionary<string, bool> old_modules = null)
             return true;
         }
 
+        [ForbidGUICalls]
         public void MarkModForInstall(string identifier, bool uncheck = false)
         {
             Util.Invoke(this, () => _MarkModForInstall(identifier, uncheck));
@@ -1649,6 +1660,7 @@ public void InstanceUpdated(GameInstance inst)
             Conflicts = null;
         }
 
+        [ForbidGUICalls]
         public void UpdateChangeSetAndConflicts(GameInstance inst, IRegistryQuerier registry)
         {
             if (freezeChangeSet)

GUI/Controls/ModInfoTabs/Relationships.cs

@@ -10,6 +10,7 @@
 
 using CKAN.Versioning;
 using CKAN.Extensions;
+using CKAN.GUI.Attributes;
 
 namespace CKAN.GUI
 {
@@ -158,6 +159,7 @@ private void BeforeExpand(object sender, TreeViewCancelEventArgs args)
             }
         }
 
+        [ForbidGUICalls]
         private void ExpandOnePage(IRegistryQuerier registry, TreeNode parent, int start, int length)
         {
             // Should already have children, since the user is expanding it

GUI/Controls/ModInfoTabs/Versions.cs

@@ -9,6 +9,7 @@
 using Autofac;
 
 using CKAN.Versioning;
+using CKAN.GUI.Attributes;
 
 namespace CKAN.GUI
 {
@@ -88,6 +89,7 @@ private void VersionsListView_ItemCheck(object sender, ItemCheckEventArgs e)
             }
         }
 
+        [ForbidGUICalls]
         private bool installable(ModuleInstaller installer, CkanModule module, IRegistryQuerier registry)
         {
             return module.IsCompatible(Main.Instance.CurrentInstance.VersionCriteria())
@@ -199,6 +201,7 @@ private ListViewItem[] getItems(GUIMod gmod, List<CkanModule> versions)
             return items;
         }
 
+        [ForbidGUICalls]
         private void checkInstallable(ListViewItem[] items)
         {
             GameInstance     currentInstance = Main.Instance.Manager.CurrentInstance;

GUI/Controls/Wait.cs

@@ -6,6 +6,8 @@
 using System.Windows.Forms;
 using Timer = System.Windows.Forms.Timer;
 
+using CKAN.GUI.Attributes;
+
 namespace CKAN.GUI
 {
     public partial class Wait : UserControl
@@ -19,6 +21,8 @@ public Wait()
             bgWorker.RunWorkerCompleted += RunWorkerCompleted;
         }
 
+
+        [ForbidGUICalls]
         public void StartWaiting(Action<object, DoWorkEventArgs>             mainWork,
                                  Action<object, RunWorkerCompletedEventArgs> postWork,
                                  bool cancelable,
@@ -38,6 +42,7 @@ public void StartWaiting(Action<object, DoWorkEventArgs>             mainWork,
 
         public bool RetryEnabled
         {
+            [ForbidGUICalls]
             set
             {
                 Util.Invoke(this, () =>
@@ -58,6 +63,7 @@ public int ProgressValue
 
         public bool ProgressIndeterminate
         {
+            [ForbidGUICalls]
             set
             {
                 Util.Invoke(this, () =>
@@ -217,6 +223,7 @@ private void ClearProgressBars()
             });
         }
 
+        [ForbidGUICalls]
         public void Reset(bool cancelable)
         {
             Util.Invoke(this, () =>
@@ -250,12 +257,14 @@ public void SetDescription(string message)
                 MessageTextBox.Text = "(" + message + ")");
         }
 
+        [ForbidGUICalls]
         private void ClearLog()
         {
             Util.Invoke(this, () =>
                 LogTextBox.Text = "");
         }
 
+        [ForbidGUICalls]
         public void AddLogMessage(string message)
         {
             Util.Invoke(this, () =>

GUI/Dialogs/DownloadsFailedDialog.cs

@@ -8,6 +8,8 @@
 
 using log4net;
 
+using CKAN.GUI.Attributes;
+
 namespace CKAN.GUI
 {
     /// <summary>
@@ -66,6 +68,7 @@ public DownloadsFailedDialog(
                 + BottomButtonPanel.Height);
         }
 
+        [ForbidGUICalls]
         public object[] Wait() => task.Task.Result;
 
         /// <summary>

GUI/Dialogs/ErrorDialog.cs

@@ -1,8 +1,11 @@
 using System;
 using System.Drawing;
 using System.Windows.Forms;
+
 using log4net;
 
+using CKAN.GUI.Attributes;
+
 namespace CKAN.GUI
 {
     public partial class ErrorDialog : Form
@@ -12,6 +15,7 @@ public ErrorDialog()
             InitializeComponent();
         }
 
+        [ForbidGUICalls]
         public void ShowErrorDialog(string text, params object[] args)
         {
             Util.Invoke(Main.Instance, () =>