Merge #4492 Catch forbidden GUI calls via Util.Debounce

HebaruSan · HebaruSan · commit 8e5bf58da4ff · 2026-01-11T13:02:30.000-06:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,7 +12,7 @@ All notable changes to this project will be documented in this file.
 
 - [Core] Ensure encoding is UTF-8 when saving JSON (#4475, #4478 by: HebaruSan)
 - [GUI] Display release date in local time zone (#4488 by: HebaruSan)
-- [GUI] Fix freezing after startup with empty mod list (#4491 by: HebaruSan)
+- [GUI] Fix freezing after startup with empty mod list (#4491, #4492 by: HebaruSan)
 
 ### Internal
 
diff --git a/GUI/Controls/EditModSearch.cs b/GUI/Controls/EditModSearch.cs
@@ -140,7 +140,7 @@ private void ImmediateHandler(object? sender, EventArgs? e)
                     // Sync the search boxes immediately
                     currentSearch = ModSearch.Parse(ModuleLabelList.ModuleLabels, inst, FilterCombinedTextBox.Text);
                 }
-                SearchToEditor();
+                Util.Invoke(this, SearchToEditor);
             }
             catch (Kraken k)
             {
diff --git a/GUI/Controls/ManageMods.cs b/GUI/Controls/ManageMods.cs
@@ -1173,9 +1173,9 @@ private void InstallAllCheckbox_CheckedChanged(object? sender, EventArgs? e)
                                               Properties.Resources.InstallAllCheckboxConfirmationNo)
                    is false)
             {
-                InstallAllCheckbox.CheckedChanged -= this.InstallAllCheckbox_CheckedChanged;
+                InstallAllCheckbox.CheckedChanged -= InstallAllCheckbox_CheckedChanged;
                 InstallAllCheckbox.Checked = !InstallAllCheckbox.Checked;
-                InstallAllCheckbox.CheckedChanged += this.InstallAllCheckbox_CheckedChanged;
+                InstallAllCheckbox.CheckedChanged += InstallAllCheckbox_CheckedChanged;
             }
             else
             {
diff --git a/Tests/GUI/ThreadSafetyTests.cs b/Tests/GUI/ThreadSafetyTests.cs
@@ -10,6 +10,7 @@
 using Mono.Cecil.Cil;
 using NUnit.Framework;
 
+using CKAN.Extensions;
 using CKAN.GUI.Attributes;
 
 namespace Tests.GUI
@@ -35,6 +36,7 @@ public void GUIAssemblyModule_MethodsWithForbidGUICalls_DontCallGUI()
             var calls = allMethods.Where(hasForbidGUIAttribute)
                                   .Select(meth => new MethodCall() { meth })
                                   .Concat(allMethods.SelectMany(FindStartedTasks))
+                                  .Concat(allMethods.SelectMany(FindDebouncedTasks))
                                   .SelectMany(GetAllCallsWithoutForbidGUI);
 
             // Assert
@@ -48,15 +50,6 @@ public void GUIAssemblyModule_MethodsWithForbidGUICalls_DontCallGUI()
             });
         }
 
-        // The sequence to start a task seems to be:
-        // 1. ldftn the function to start
-        // 2. newobj a System.Action to hold it
-        // 3. callvirt StartNew
-        // ... so find the operand of the ldftn most immediately preceding the call
-        private static MethodDefinition? FindStartNewArgument(Instruction instr)
-            => instr.OpCode.Name == "ldftn" ? instr.Operand as MethodDefinition
-                                            : FindStartNewArgument(instr.Previous);
-
         private static IEnumerable<MethodCall> GetAllCallsWithoutForbidGUI(MethodCall initialStack)
             => VisitMethodDefinition(initialStack,
                                      initialStack.Last(),
@@ -79,9 +72,14 @@ private static bool unsafeCall(MethodDefinition md)
                 && unsafeType(md.DeclaringType);
 
         // Any method on a type in WinForms or inheriting from anything in WinForms is presumed unsafe
-        private static bool unsafeType(TypeDefinition t)
-            => t.Namespace == winformsNamespace
-                || (t.BaseType != null && unsafeType(t.BaseType.Resolve()));
+        private static bool unsafeType(TypeDefinition td)
+            => td.TraverseNodes(t => t.BaseType?.Resolve())
+                 .Any(t => t.Namespace == winformsNamespace)
+               && !isEventArgs(td);
+
+        private static bool isEventArgs(TypeDefinition td)
+            => td.TraverseNodes(t => t.BaseType?.Resolve())
+                 .Any(t => t is { Namespace: "System", Name: "EventArgs" });
 
         private static readonly Type   forbidAttrib      = typeof(ForbidGUICallsAttribute);
         private static readonly string winformsNamespace = typeof(Control).Namespace!;
diff --git a/Tests/MonoCecilAnalysisTestsBase.cs b/Tests/MonoCecilAnalysisTestsBase.cs
@@ -4,6 +4,7 @@
 
 using Mono.Cecil;
 using Mono.Cecil.Cil;
+
 using CKAN.Extensions;
 
 namespace Tests
@@ -74,9 +75,11 @@ protected static IEnumerable<TypeDefinition> GetAllNestedTypes(TypeDefinition td
                          .Concat(td.NestedTypes.SelectMany(GetAllNestedTypes));
 
         protected static IEnumerable<MethodCall> FindStartedTasks(MethodDefinition md)
-            => StartNewCalls(md).Select(FindStartNewArgument)
-                                .OfType<MethodDefinition>()
-                                .Select(taskArg => new MethodCall() { md, taskArg });
+            => StartNewCalls(md).SelectMany(sn =>
+                   sn.Operand is MethodReference snMethod
+                   && FindStartNewArgument(sn) is MethodDefinition taskArg
+                       ? Enumerable.Repeat(new MethodCall() { md, snMethod.Resolve(), taskArg, }, 1)
+                       : Enumerable.Empty<MethodCall>());
 
         private static IEnumerable<Instruction> StartNewCalls(MethodDefinition md)
             => md.Body?.Instructions.Where(instr => callOpCodes.Contains(instr.OpCode.Name)
@@ -85,16 +88,48 @@ private static IEnumerable<Instruction> StartNewCalls(MethodDefinition md)
                       ?? Enumerable.Empty<Instruction>();
 
         private static bool isStartNew(MethodReference mr)
-            => (mr.DeclaringType.Namespace == "System.Threading.Tasks"
-                && mr.DeclaringType.Name   == "TaskFactory"
-                && mr.Name                 == "StartNew")
-               || (mr.DeclaringType.Namespace == "System.Threading.Tasks"
-                   && mr.DeclaringType.Name   == "Task"
-                   && mr.Name                 == "Run");
+            => mr is
+               {
+                   DeclaringType: { Namespace: "System.Threading.Tasks", Name: "TaskFactory" },
+                   Name:          "StartNew",
+               }
+               or
+               {
+                   DeclaringType: { Namespace: "System.Threading.Tasks", Name: "Task" },
+                   Name:          "Run",
+               };
 
         private static MethodDefinition? FindStartNewArgument(Instruction instr)
-            => instr.OpCode.Name == "ldftn" ? instr.Operand as MethodDefinition
-                                            : FindStartNewArgument(instr.Previous);
+            => FindFuncArguments(instr).FirstOrDefault();
+
+        protected static IEnumerable<MethodCall> FindDebouncedTasks(MethodDefinition md)
+            => DebounceCalls(md).SelectMany(db =>
+                   db.Operand is MethodReference dbMethod
+                       ? FindDebounceArguments(db)
+                             .Select(dbArg => new MethodCall() { md, dbMethod.Resolve(), dbArg, })
+                       : Enumerable.Empty<MethodCall>());
+
+        private static IEnumerable<Instruction> DebounceCalls(MethodDefinition md)
+            => md.Body?.Instructions.Where(instr => callOpCodes.Contains(instr.OpCode.Name)
+                                                    && instr.Operand is MethodReference mr
+                                                    && isDebounce(mr))
+                      ?? Enumerable.Empty<Instruction>();
+
+        private static bool isDebounce(MethodReference mr)
+            => mr is
+               {
+                   DeclaringType: { Namespace: "CKAN.GUI", Name: "Util" },
+                   Name:          "Debounce",
+               };
+
+        private static IEnumerable<MethodDefinition> FindDebounceArguments(Instruction instr)
+            => FindFuncArguments(instr).Take(4);
+
+        private static IEnumerable<MethodDefinition> FindFuncArguments(Instruction instr)
+            => instr.TraverseNodes(i => i.Previous)
+                    .Where(i => i.OpCode.Name == "ldftn")
+                    .Select(i => i.Operand)
+                    .OfType<MethodDefinition>();
 
         private static readonly HashSet<string> callOpCodes = new HashSet<string>
         {

Original file line number	Diff line number	Diff line change
`@@ -140,7 +140,7 @@ private void ImmediateHandler(object? sender, EventArgs? e)`
`140`	`140`	`// Sync the search boxes immediately`
`141`	`141`	`currentSearch = ModSearch.Parse(ModuleLabelList.ModuleLabels, inst, FilterCombinedTextBox.Text);`
`142`	`142`	`}`
`143`		`- SearchToEditor();`
	`143`	`+ Util.Invoke(this, SearchToEditor);`
`144`	`144`	`}`
`145`	`145`	`catch (Kraken k)`
`146`	`146`	`{`
Original file line number	Diff line number	Diff line change
`@@ -1173,9 +1173,9 @@ private void InstallAllCheckbox_CheckedChanged(object? sender, EventArgs? e)`
`1173`	`1173`	`Properties.Resources.InstallAllCheckboxConfirmationNo)`
`1174`	`1174`	`is false)`
`1175`	`1175`	`{`
`1176`		`- InstallAllCheckbox.CheckedChanged -= this.InstallAllCheckbox_CheckedChanged;`
	`1176`	`+ InstallAllCheckbox.CheckedChanged -= InstallAllCheckbox_CheckedChanged;`
`1177`	`1177`	`InstallAllCheckbox.Checked = !InstallAllCheckbox.Checked;`
`1178`		`- InstallAllCheckbox.CheckedChanged += this.InstallAllCheckbox_CheckedChanged;`
	`1178`	`+ InstallAllCheckbox.CheckedChanged += InstallAllCheckbox_CheckedChanged;`
`1179`	`1179`	`}`
`1180`	`1180`	`else`
`1181`	`1181`	`{`