diff --git a/CHANGELOG.md b/CHANGELOG.md index 1455071dcc..a9da089984 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/Cmdline/Action/GameInstance.cs b/Cmdline/Action/GameInstance.cs index c26bfdd8df..5685605181 100644 --- a/Cmdline/Action/GameInstance.cs +++ b/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; } } diff --git a/Core/Extensions/EnumerableExtensions.cs b/Core/Extensions/EnumerableExtensions.cs index d315ba7a6d..0541187449 100644 --- a/Core/Extensions/EnumerableExtensions.cs +++ b/Core/Extensions/EnumerableExtensions.cs @@ -16,6 +16,7 @@ public static ICollection AsCollection(this IEnumerable source) } #if NET45 + public static HashSet ToHashSet(this IEnumerable source) { if (source == null) @@ -23,6 +24,10 @@ public static HashSet ToHashSet(this IEnumerable source) return new HashSet(source); } + + public static IEnumerable Append(this IEnumerable source, T next) + => source.Concat(Enumerable.Repeat(next, 1)); + #endif public static Dictionary ToDictionary(this IEnumerable> pairs) diff --git a/GUI/Attributes/ForbidGUICallsAttribute.cs b/GUI/Attributes/ForbidGUICallsAttribute.cs new file mode 100644 index 0000000000..0192a8893c --- /dev/null +++ b/GUI/Attributes/ForbidGUICallsAttribute.cs @@ -0,0 +1,13 @@ +using System; + +namespace CKAN.GUI.Attributes +{ + /// + /// Tag background functions with this, and a test will check + /// that they're not calling GUI code without Util.Invoke. + /// + [AttributeUsage(AttributeTargets.Constructor | AttributeTargets.Property + | AttributeTargets.Method | AttributeTargets.Delegate, + Inherited = true, AllowMultiple = false)] + public class ForbidGUICallsAttribute : Attribute { } +} diff --git a/GUI/CKAN-GUI.csproj b/GUI/CKAN-GUI.csproj index c6f6b68ed7..6ac5ebd64a 100644 --- a/GUI/CKAN-GUI.csproj +++ b/GUI/CKAN-GUI.csproj @@ -52,6 +52,7 @@ + Form diff --git a/GUI/Controls/ChooseProvidedMods.cs b/GUI/Controls/ChooseProvidedMods.cs index 1bb72773a5..84c03bb6de 100644 --- a/GUI/Controls/ChooseProvidedMods.cs +++ b/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 modules, NetModuleCache cache) { Util.Invoke(this, () => @@ -36,6 +39,7 @@ public void LoadProviders(string message, List modules, NetModuleCac }); } + [ForbidGUICalls] public CkanModule Wait() { task = new TaskCompletionSource(); diff --git a/GUI/Controls/ChooseRecommendedMods.cs b/GUI/Controls/ChooseRecommendedMods.cs index c7f33379e1..f8ad6575f2 100644 --- a/GUI/Controls/ChooseRecommendedMods.cs +++ b/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 toInstall, HashSet toUninstall, @@ -38,6 +41,7 @@ Dictionary> supporters }); } + [ForbidGUICalls] public HashSet Wait() { if (Platform.IsMono) diff --git a/GUI/Controls/DeleteDirectories.cs b/GUI/Controls/DeleteDirectories.cs index 6ddd6254bc..dab1b62d85 100644 --- a/GUI/Controls/DeleteDirectories.cs +++ b/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. /// /// Directories that the user may want to delete + [ForbidGUICalls] public void LoadDirs(GameInstance ksp, HashSet possibleConfigOnlyDirs) { instance = ksp; @@ -50,6 +53,7 @@ public void LoadDirs(GameInstance ksp, HashSet possibleConfigOnlyDirs) /// /// true if user chose to delete, false otherwise /// + [ForbidGUICalls] public bool Wait(out HashSet toDelete) { if (Platform.IsMono) diff --git a/GUI/Controls/EditModpack.cs b/GUI/Controls/EditModpack.cs index 684016046a..232e9f3c82 100644 --- a/GUI/Controls/EditModpack.cs +++ b/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 OnSelectedItemsChanged; + [ForbidGUICalls] public bool Wait(IUser user) { if (Platform.IsMono) diff --git a/GUI/Controls/ManageMods.cs b/GUI/Controls/ManageMods.cs index a11917715e..1005b85a50 100644 --- a/GUI/Controls/ManageMods.cs +++ b/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 sortColumns private List ChangeSet { get => currentChangeSet; + [ForbidGUICalls] set { var orig = currentChangeSet; @@ -111,6 +113,7 @@ private List ChangeSet } } + [ForbidGUICalls] private void ChangeSetUpdated() { Util.Invoke(this, () => @@ -149,12 +152,15 @@ private void ChangeSetUpdated() private Dictionary 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 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); } + [ForbidGUICalls] private bool _UpdateModsList(Dictionary old_modules = null) { log.Info("Updating the mod list"); @@ -1275,6 +1285,7 @@ private bool _UpdateModsList(Dictionary 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) diff --git a/GUI/Controls/ModInfoTabs/Relationships.cs b/GUI/Controls/ModInfoTabs/Relationships.cs index bf618316e7..e37851cd0e 100644 --- a/GUI/Controls/ModInfoTabs/Relationships.cs +++ b/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 diff --git a/GUI/Controls/ModInfoTabs/Versions.cs b/GUI/Controls/ModInfoTabs/Versions.cs index 8ce182066c..8f025dafe9 100644 --- a/GUI/Controls/ModInfoTabs/Versions.cs +++ b/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 versions) return items; } + [ForbidGUICalls] private void checkInstallable(ListViewItem[] items) { GameInstance currentInstance = Main.Instance.Manager.CurrentInstance; diff --git a/GUI/Controls/Wait.cs b/GUI/Controls/Wait.cs index 2d975275bc..69cb99a554 100644 --- a/GUI/Controls/Wait.cs +++ b/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 mainWork, Action postWork, bool cancelable, @@ -38,6 +42,7 @@ public void StartWaiting(Action 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, () => diff --git a/GUI/Dialogs/DownloadsFailedDialog.cs b/GUI/Dialogs/DownloadsFailedDialog.cs index 9f9d379cae..776e6d9526 100644 --- a/GUI/Dialogs/DownloadsFailedDialog.cs +++ b/GUI/Dialogs/DownloadsFailedDialog.cs @@ -8,6 +8,8 @@ using log4net; +using CKAN.GUI.Attributes; + namespace CKAN.GUI { /// @@ -66,6 +68,7 @@ public DownloadsFailedDialog( + BottomButtonPanel.Height); } + [ForbidGUICalls] public object[] Wait() => task.Task.Result; /// diff --git a/GUI/Dialogs/ErrorDialog.cs b/GUI/Dialogs/ErrorDialog.cs index 6edfc6179c..3c3b66387f 100644 --- a/GUI/Dialogs/ErrorDialog.cs +++ b/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, () => diff --git a/GUI/Main/Main.cs b/GUI/Main/Main.cs index 702c1c64e2..5ba7fb9dc6 100644 --- a/GUI/Main/Main.cs +++ b/GUI/Main/Main.cs @@ -16,6 +16,7 @@ using CKAN.Extensions; using CKAN.Versioning; +using CKAN.GUI.Attributes; // Don't warn if we use our own obsolete properties #pragma warning disable 0618 @@ -972,6 +973,7 @@ private void RefreshModList(bool allowAutoUpdate, Dictionary oldMo oldModules); } + [ForbidGUICalls] private void EnableMainWindow() { Util.Invoke(this, () => @@ -988,6 +990,7 @@ private void EnableMainWindow() }); } + [ForbidGUICalls] private void DisableMainWindow() { Util.Invoke(this, () => diff --git a/GUI/Main/MainDialogs.cs b/GUI/Main/MainDialogs.cs index 60eff2cd16..28afb67935 100644 --- a/GUI/Main/MainDialogs.cs +++ b/GUI/Main/MainDialogs.cs @@ -1,6 +1,8 @@ using System; using System.Windows.Forms; +using CKAN.GUI.Attributes; + namespace CKAN.GUI { public partial class Main @@ -23,6 +25,7 @@ public void RecreateDialogs() selectionDialog = controlFactory.CreateControl(); } + [ForbidGUICalls] public void AddStatusMessage(string text, params object[] args) { string msg = String.Format(text, args); @@ -33,6 +36,7 @@ public void AddStatusMessage(string text, params object[] args) Wait.AddLogMessage(msg); } + [ForbidGUICalls] public void ErrorDialog(string text, params object[] args) { errorDialog.ShowErrorDialog(String.Format(text, args)); diff --git a/GUI/Main/MainDownload.cs b/GUI/Main/MainDownload.cs index dd56cbe15d..6a59c24447 100644 --- a/GUI/Main/MainDownload.cs +++ b/GUI/Main/MainDownload.cs @@ -4,6 +4,8 @@ using System.ComponentModel; using System.Threading.Tasks; +using CKAN.GUI.Attributes; + namespace CKAN.GUI { public partial class Main @@ -37,6 +39,7 @@ public void StartDownload(GUIMod module) } } + [ForbidGUICalls] private void CacheMod(object sender, DoWorkEventArgs e) { ResetProgress(); @@ -83,6 +86,7 @@ public void PostModCaching(object sender, RunWorkerCompletedEventArgs e) } } + [ForbidGUICalls] private void UpdateCachedByDownloads(GUIMod module) { // Update all mods that share the same ZIP diff --git a/GUI/Main/MainInstall.cs b/GUI/Main/MainInstall.cs index 048e53bedc..3cb7e49073 100644 --- a/GUI/Main/MainInstall.cs +++ b/GUI/Main/MainInstall.cs @@ -6,6 +6,7 @@ using System.Transactions; using CKAN.Extensions; +using CKAN.GUI.Attributes; // Don't warn if we use our own obsolete properties #pragma warning disable 0618 @@ -65,6 +66,7 @@ public void InstallModuleDriver(IRegistryQuerier registry, IEnumerable + Util.Invoke(this, () => { dfd = new DownloadsFailedDialog( Properties.Resources.ModDownloadsFailedMessage, @@ -289,6 +291,7 @@ private void InstallMods(object sender, DoWorkEventArgs e) e.Result = new InstallResult(true, changes); } + [ForbidGUICalls] private void HandlePossibleConfigOnlyDirs(Registry registry, HashSet possibleConfigOnlyDirs) { if (possibleConfigOnlyDirs != null) diff --git a/GUI/Main/MainRecommendations.cs b/GUI/Main/MainRecommendations.cs index 153084a95a..0ef46c0cf4 100644 --- a/GUI/Main/MainRecommendations.cs +++ b/GUI/Main/MainRecommendations.cs @@ -6,6 +6,7 @@ using CKAN.Extensions; using CKAN.Versioning; +using CKAN.GUI.Attributes; // Don't warn if we use our own obsolete properties #pragma warning disable 0618 @@ -33,6 +34,7 @@ private void auditRecommendationsMenuItem_Click(object sender, EventArgs e) )); } + [ForbidGUICalls] private void AuditRecommendations(IRegistryQuerier registry, GameVersionCriteria versionCriteria) { var installer = new ModuleInstaller(CurrentInstance, Manager.Cache, currentUser); diff --git a/GUI/Main/MainRepo.cs b/GUI/Main/MainRepo.cs index 41fdcabae8..720c5d97f0 100644 --- a/GUI/Main/MainRepo.cs +++ b/GUI/Main/MainRepo.cs @@ -13,6 +13,7 @@ using CKAN.Versioning; using CKAN.Configuration; using CKAN.Extensions; +using CKAN.GUI.Attributes; // Don't warn if we use our own obsolete properties #pragma warning disable 0618 @@ -57,6 +58,7 @@ public void UpdateRepo(bool forceFullRefresh = false, bool refreshWithoutChanges ShowWaitDialog(); } + [ForbidGUICalls] private void UpdateRepo(object sender, DoWorkEventArgs e) { (bool forceFullRefresh, bool refreshWithoutChanges) = (RepoArgument)e.Argument; diff --git a/GUI/Main/MainWait.cs b/GUI/Main/MainWait.cs index 7e75504169..c2b13dd054 100644 --- a/GUI/Main/MainWait.cs +++ b/GUI/Main/MainWait.cs @@ -1,6 +1,8 @@ using System; using System.Windows.Forms; +using CKAN.GUI.Attributes; + // Don't warn if we use our own obsolete properties #pragma warning disable 0618 @@ -8,6 +10,7 @@ namespace CKAN.GUI { public partial class Main { + [ForbidGUICalls] public void ShowWaitDialog() { Util.Invoke(this, () => @@ -69,6 +72,7 @@ public void SetProgress(int progress) }); } + [ForbidGUICalls] public void ResetProgress() { Wait.ProgressIndeterminate = true; diff --git a/Tests/GUI/ThreadSafetyTests.cs b/Tests/GUI/ThreadSafetyTests.cs new file mode 100644 index 0000000000..15c266063c --- /dev/null +++ b/Tests/GUI/ThreadSafetyTests.cs @@ -0,0 +1,159 @@ +using System; +using System.Reflection; +using System.Collections.Generic; +using System.Linq; +using System.Windows.Forms; + +using Mono.Cecil; +using MethodAttributes = Mono.Cecil.MethodAttributes; +using Mono.Cecil.Cil; +using NUnit.Framework; + +using CKAN.Extensions; +using CKAN.GUI.Attributes; + +namespace Tests.GUI +{ + using MethodCall = List; + using CallsDict = Dictionary>; + + [TestFixture] + public class ThreadSafetyTests + { + [Test] + public void GUIAssemblyModule_MethodsWithForbidGUICalls_DontCallGUI() + { + // Arrange / Act + var mainModule = ModuleDefinition.ReadModule(Assembly.GetAssembly(typeof(CKAN.GUI.Main)) + .Location); + var allMethods = mainModule.Types + .SelectMany(t => GetAllNestedTypes(t)) + .SelectMany(t => t.Methods) + .ToArray(); + var calls = allMethods.Where(hasForbidGUIAttribute) + .Select(meth => new MethodCall() { meth }) + .Concat(allMethods.SelectMany(FindStartedTasks)) + .SelectMany(GetAllCallsWithoutForbidGUI); + + // Assert + Assert.Multiple(() => + { + foreach (var callStack in calls) + { + Assert.IsFalse(unsafeCall(callStack.Last()), + $"Forbidden GUI call: {string.Join(" -> ", callStack.Select(SimpleName))}"); + } + }); + } + + private IEnumerable GetAllNestedTypes(TypeDefinition td) + => Enumerable.Repeat(td, 1) + .Concat(td.NestedTypes.SelectMany(nested => GetAllNestedTypes(nested))); + + private IEnumerable FindStartedTasks(MethodDefinition md) + => StartNewCalls(md).Select(FindStartNewArgument) + .Select(taskArg => new MethodCall() { md, taskArg }); + + private IEnumerable StartNewCalls(MethodDefinition md) + => md.Body?.Instructions.Where(instr => callOpCodes.Contains(instr.OpCode.Name) + && instr.Operand is MethodReference mr + && isStartNew(mr)) + ?? Enumerable.Empty(); + + private 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"); + + // 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 MethodDefinition FindStartNewArgument(Instruction instr) + => instr.OpCode.Name == "ldftn" ? instr.Operand as MethodDefinition + : FindStartNewArgument(instr.Previous); + + private IEnumerable GetAllCallsWithoutForbidGUI(MethodCall initialStack) + => VisitMethodDefinition(initialStack, initialStack.Last(), new CallsDict(), hasForbidGUIAttribute, unsafeCall); + + private string SimpleName(MethodDefinition md) => $"{md.DeclaringType.Name}.{md.Name}"; + + // https://gist.github.com/lnicola/b48db1a6ff3617bdac2a + private IEnumerable VisitMethodDefinition(MethodCall fullStack, + MethodDefinition methDef, + CallsDict calls, + Func skip, + Func stopAfter) + { + var called = calls[methDef] = methodsCalledBy(methDef).Distinct().ToList(); + foreach (var calledMeth in called) + { + if (!calls.ContainsKey(calledMeth) && !skip(calledMeth)) + { + var newStack = fullStack.Append(calledMeth).ToList(); + yield return newStack; + if (!stopAfter(calledMeth)) + { + // yield from, please! + foreach (var subcall in VisitMethodDefinition(newStack, calledMeth, calls, skip, stopAfter)) + { + yield return subcall; + } + } + } + } + } + + private IEnumerable methodsCalledBy(MethodDefinition methDef) + => methDef.Body + .Instructions + .Where(instr => callOpCodes.Contains(instr.OpCode.Name)) + .Select(instr => instr.Operand as MethodDefinition + ?? GetSetterDef(instr.Operand as MethodReference)) + .Where(calledMeth => calledMeth?.Body != null); + + // Property setters are virtual and have references instead of definitions + private MethodDefinition GetSetterDef(MethodReference mr) + => (mr?.Name.StartsWith("set_") ?? false) ? mr.Resolve() + : null; + + private bool hasForbidGUIAttribute(MethodDefinition md) + => md.CustomAttributes.Any(attr => attr.AttributeType.Namespace == forbidAttrib.Namespace + && attr.AttributeType.Name == forbidAttrib.Name); + + private bool unsafeCall(MethodDefinition md) + // If it has [ForbidGUICalls], then treat as safe because it'll be checked on its own + => !hasForbidGUIAttribute(md) + // Adding an event handler is OK + && !md.IsAddOn + // Getting a property is OK + && !md.IsGetter + // Otherwise treat anything on a System.Windows.Forms object as unsafe + && unsafeType(md.DeclaringType); + + // Any method on a type in WinForms or inheriting from anything in WinForms is presumed unsafe + private bool unsafeType(TypeDefinition t) + => t.Namespace == winformsNamespace + ? true + : (t.BaseType != null && unsafeType(t.BaseType.Resolve())); + + private static readonly Type forbidAttrib = typeof(ForbidGUICallsAttribute); + private static readonly string winformsNamespace = typeof(Control).Namespace; + + private static HashSet callOpCodes = new HashSet + { + // Constructors + "newobj", + + // Normal function calls + "call", + + // Virtual function calls (includes property setters) + "callvirt", + }; + } +} diff --git a/Tests/Tests.csproj b/Tests/Tests.csproj index 01a614d6bf..f3d9b58235 100644 --- a/Tests/Tests.csproj +++ b/Tests/Tests.csproj @@ -41,7 +41,7 @@ true - +