diff --git a/.gitattributes b/.gitattributes index 87ed07e..155255a 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,2 +1,3 @@ *.cs eol=crlf *.xaml eol=crlf +*.ps1 eol=crlf diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 9bc2a3d..8ae2b6b 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -1,6 +1,12 @@ Stylet Changelog ================ +v1.1.15 +------- + + - Fix re-entrancy issue in the EventAggregator, if you publish events from within an event handler + - Improve Stylet.Start package + v1.1.14 ------- diff --git a/NuGet/Stylet.Start.nuspec b/NuGet/Stylet.Start.nuspec index 661c246..56e359f 100644 --- a/NuGet/Stylet.Start.nuspec +++ b/NuGet/Stylet.Start.nuspec @@ -22,6 +22,6 @@ - + diff --git a/NuGet/start/content/net45/App.xaml.pp b/NuGet/start/content/net45/App.xaml.pp deleted file mode 100644 index 7edc53e..0000000 --- a/NuGet/start/content/net45/App.xaml.pp +++ /dev/null @@ -1,13 +0,0 @@ - - - - - - - - - diff --git a/NuGet/start/content/net45/Bootstrapper.cs.pp b/NuGet/start/content/net45/Bootstrapper.cs.pp deleted file mode 100644 index 4a9633b..0000000 --- a/NuGet/start/content/net45/Bootstrapper.cs.pp +++ /dev/null @@ -1,24 +0,0 @@ -using Stylet; -using StyletIoC; -using $rootnamespace$.Pages; -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; - -namespace $rootnamespace$ -{ - public class Bootstrapper : Bootstrapper - { - protected override void ConfigureIoC(IStyletIoCBuilder builder) - { - // Configure the IoC container in here - } - - protected override void Configure() - { - // Perform any other configuration before the application starts - } - } -} diff --git a/NuGet/start/content/net45/Pages/ShellView.xaml.pp b/NuGet/start/content/net45/Pages/ShellView.xaml.pp deleted file mode 100644 index 61cb58f..0000000 --- a/NuGet/start/content/net45/Pages/ShellView.xaml.pp +++ /dev/null @@ -1,23 +0,0 @@ - - - - Hello Stylet! - - - Now delete MainWindow.xaml. - - - diff --git a/NuGet/start/content/net45/Pages/ShellViewModel.cs.pp b/NuGet/start/content/net45/Pages/ShellViewModel.cs.pp deleted file mode 100644 index 703c30a..0000000 --- a/NuGet/start/content/net45/Pages/ShellViewModel.cs.pp +++ /dev/null @@ -1,13 +0,0 @@ -using Stylet; -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; - -namespace $rootnamespace$.Pages -{ - public class ShellViewModel : Screen - { - } -} diff --git a/NuGet/start/tools/install.ps1 b/NuGet/start/tools/install.ps1 new file mode 100644 index 0000000..f2389d7 --- /dev/null +++ b/NuGet/start/tools/install.ps1 @@ -0,0 +1,312 @@ +param($installPath, $toolsPath, $package, $project) + +# Testing: call with Invoke-Expression "path\to\install.ps1" +# $project = Get-Project + +$rootNamespace = $project.Properties.Item("RootNamespace").Value +$rootPath = $project.Properties.Item("LocalPath").Value + + +# Modify App.xaml +# This is a real ballache: the previous Stylet.Start package (which uses NuGet's 'content' approach) +# will delete App.xaml and App.xaml.cs on uninstallation if they haven't been modified by the user. +# Obviously that's bad, so we'll need to put them back + +$appXamlPath = [System.IO.Path]::Combine($rootPath, "App.xaml") +$existingAppXaml = $project.ProjectItems | Where { $_.Name -eq "App.xaml" } | Select -First 1 +if ($existingAppXaml -eq $null) +{ + Write-Host ">>>> App.xaml is missing: creating it from scratch (it was probably removed by NuGet)" + + $appXamlContent = " + + + + + + + +" + + [System.IO.File]::WriteAllText($appXamlPath, $appXamlContent) + $appXaml = $project.ProjectItems.AddFromFile($appXamlPath) + $appXaml.Properties.Item("BuildAction").Value = 4 # ApplicationDefinition +} +else +{ + $doc = [System.Xml.Linq.XDocument]::Load($appXamlPath) + + $styletNs = [System.Xml.Linq.XNamespace]::Get("https://github.com/canton7/Stylet") + $ns = $doc.Root.GetDefaultNamespace() + $localNs = $doc.Root.GetNamespaceOfPrefix("local") + + $startupUri = $doc.Root.Attribute("StartupUri") + if ($startupUri -ne $null) + { + $startupUri.Remove() + } + + $existingApplicationLoader = $doc.Root.Descendants($styletNs.GetName("ApplicationLoader")) | Select -First 1 + if ($existingApplicationLoader -ne $null) + { + Write-Host ">>>> Not modifying App.xaml as it already has an element" + } + else + { + if ($doc.Root.Attribute([System.Xml.Linq.XNamespace]::Xmlns.GetName("s")) -eq $null) + { + $doc.Root.Add((New-Object System.Xml.Linq.XAttribute([System.Xml.Linq.XNamespace]::Xmlns.GetName("s"), $styletNs))) + } + + $resources = $doc.Root.Element($ns.GetName("Application.Resources")) + $existingResources = @($resources.Nodes()) + + $bootstrapper = New-Object System.Xml.Linq.XElement($localNs.GetName("Bootstrapper")) + $bootstrapperProperty = New-Object System.Xml.Linq.XElement($styletNs.GetName("ApplicationLoader.Bootstrapper"), $bootstrapper) + $applicationLoader = New-Object System.Xml.Linq.XElement($styletNs.GetName("ApplicationLoader"), $bootstrapperProperty) + + if ($existingResources.Count -gt 0) + { + $mergedDictionaries = New-Object System.Xml.Linq.XElement($ns.GetName("ResourceDictionary.MergedDictionaries"), $applicationLoader) + $resourceDictionary = New-Object System.Xml.Linq.XElement($ns.GetName("ResourceDictionary"), $mergedDictionaries, $existingResources) + $resources.ReplaceNodes($resourceDictionary) + } + else + { + $resources.Add($applicationLoader) + } + + $settings = New-Object System.Xml.XmlWriterSettings + $settings.Indent = $True + $settings.NewLineOnAttributes = $True + $settings.OmitXmlDeclaration = $True + $settings.IndentChars = " " + + $stringWriter = New-Object System.IO.StringWriter + $xmlWriter = [System.Xml.XmlWriter]::Create($stringWriter, $settings) + $doc.WriteTo($xmlWriter) + $xmlWriter.Close() + + $appXaml = $stringWriter.ToString(); + $stringWriter.Close() + + # Manually fudge the newlines after xmlns declarations, since XmlWriter won't do them itself + $appXaml = [System.Text.RegularExpressions.Regex]::Replace($appXaml, "Application\s+x:Class=", "Application x:Class=") + $appXaml = $appXaml.Replace(" xmlns=", "`r`n xmlns=") + $appXaml = $appXaml.Replace(" xmlns:", "`r`n xmlns:") + + [System.IO.File]::WriteAllText($appXamlPath, $appXaml) + } +} + +# Add bootstrapper + +# Only do this if Bootstrapper.cs doesn't already exist... +$existingBootstrapper = $project.ProjectItems | Where { $_.Name -eq "Bootstrapper.cs" } | Select -First 1 +if ($existingBootstrapper -ne $null) +{ + Write-Host ">>>> Not creating Bootstrapper.cs as it already exists" +} +else +{ + $bootstrapperContent = "using System; +using Stylet; +using StyletIoC; +using ${rootNamespace}.Pages; + +namespace ${rootNamespace} +{ + public class Bootstrapper : Bootstrapper + { + protected override void ConfigureIoC(IStyletIoCBuilder builder) + { + // Configure the IoC container in here + } + + protected override void Configure() + { + // Perform any other configuration before the application starts + } + } +} +" + $bootstrapperPath = [System.IO.Path]::Combine($rootPath, "Bootstrapper.cs") + [System.IO.File]::WriteAllText($bootstrapperPath, $bootstrapperContent) + $null = $project.ProjectItems.AddFromFile($bootstrapperPath) +} + +# Add Pages/ folder + +$pages = $project.ProjectItems | Where { $_.Name -Eq "Pages" } | Select -First 1 +if ($pages -eq $null) +{ + # This also creates the folder on disk + $pages = $project.ProjectItems.AddFolder("Pages") +} + +# Add Pages/ShellView.xaml + +# Used in multiple places, potentially +$standaloneShellViewContent = " + + Hello Stylet! + +" + +$standaloneShellViewCsContent = "using System; +using System.Windows; + +namespace ${rootNamespace}.Pages +{ + /// + /// Interaction logic for ShellView.xaml + /// + public partial class ShellView : Window + { + public ShellView() + { + InitializeComponent(); + } + } +} +" + +# Only do this if ShellView doesn't already exist... +$existingShellView = $pages.ProjectItems | Where { $_.Name -eq "ShellView.xaml" } | Select -First 1 +if ($existingShellView -ne $null) +{ + Write-Host ">>>> Not creating Pages/ShellView.xaml as it already exists. " +} +else +{ + $mainWindow = ($project.ProjectItems | Where { $_.Name -Eq "MainWindow.xaml" } | Select -First 1) + if ($mainWindow -eq $null) + { + Write-Host ">>>> Creating Pages/ShellView.xaml from scratch, as MainWindow.xaml doesn't exist" + + $shellViewContent = $standaloneShellViewContent + $shellViewCsContent = $standaloneShellViewCsContent + } + else + { + $mainWindowPath = $mainWindow.Properties.Item("FullPath").Value + $mainWindowCsPath = $mainWindowPath + ".cs" + + $shellViewContent = [System.IO.File]::ReadAllText($mainWindowPath) + $match = [System.Text.RegularExpressions.Regex]::Match($shellViewContent, "", [System.Text.RegularExpressions.RegexOptions]::Singleline) + if ($match.Success) + { + $mainWindow.Remove() + + $originalShellViewAttributes = $match.Groups[1].Value + $shellViewAttributes = $originalShellViewAttributes + + $shellViewAttributes = $shellViewAttributes.Replace("x:Class=""${rootNamespace}.MainWindow""", "x:Class=""${rootNamespace}.Pages.ShellView""") + $shellViewAttributes = $shellViewAttributes.Replace('Title="MainWindow"', 'Title="Stylet Start Project"') + if (!$shellViewAttributes.Contains("xmlns:local")) + { + $shellViewAttributes += "`r`n xmlns:local=""clr-namespace:${rootNamespace}.Pages""" + } + else + { + $shellViewAttributes = $shellViewAttributes.Replace("xmlns:local=""clr-namespace:${rootNamespace}""", "xmlns:local=""clr-namespace:${rootNamespace}.Pages""") + } + if (!$shellViewAttributes.Contains("https://github.com/canton7/Stylet")) + { + $shellViewAttributes += "`r`n xmlns:s=""https://github.com/canton7/Stylet""" + } + if (!$shellViewAttributes.Contains('mc:Ignorable="d"')) + { + $shellViewAttributes += "`r`n mc:Ignorable=""d""" + } + if (!$shellViewAttributes.Contains("d:DataContext=")) + { + $shellViewAttributes += "`r`n d:DataContext=""{d:DesignInstance local:ShellViewModel}""" + } + + $shellViewContent = $shellViewContent.Replace($originalShellViewAttributes, $shellViewAttributes); + + $shellViewContent = [System.Text.RegularExpressions.Regex]::Replace($shellViewContent, "\s*", " + Hello Stylet! + ") + + [System.IO.File]::Delete($mainWindowPath) + + if ([System.IO.File]::Exists($mainWindowCsPath)) + { + $shellViewCsContent = [System.IO.File]::ReadAllText($mainWindowCsPath) + $shellViewCsContent = $shellViewCsContent.Replace("namespace " + $rootNamespace, "namespace " + $rootNamespace + ".Pages") + $shellViewCsContent = $shellViewCsContent.Replace("class MainWindow", "class ShellView") + $shellViewCsContent = $shellViewCsContent.Replace("public MainWindow()", "public ShellView()") + $shellViewCsContent = $shellViewCsContent.Replace("/// Interaction logic for MainWindow.xaml", "/// Interaction logic for ShellView.xaml") + + [System.IO.File]::Delete($mainWindowCsPath) + } + else + { + Write-Host ">>>> MainWindow.xaml.cs doesn't exist. Creating Pages/ShellView.xaml.cs from scratch" + $shellViewCsContent = $standaloneShellViewCsContent + } + + } + else + { + Write-Host ">>>> WARNING: MainWindow.xaml is not a Window, so creating Pages/ShellView.xaml from scratch" + + $shellViewContent = $standaloneShellViewContent + $shellViewCsContent = $standaloneShellViewCsContent + } + } + + $shellViewPath = [System.IO.Path]::Combine($rootPath, "Pages", "ShellView.xaml") + $shellViewCsPath = $shellViewPath + ".cs" + + [System.IO.File]::WriteAllText($shellViewPath, $shellViewContent) + [System.IO.File]::WriteAllText($shellViewCsPath, $shellViewCsContent) + + $shellView = $pages.ProjectItems.AddFromFile($shellViewPath) + # This should have been added automagically, but just in case... + $null = $shellView.ProjectItems.AddFromFile($shellViewCsPath) +} + +# Add Pages/ShellViewModel.cs + +# Only do this if ShellViewModel doesn't already exist +$existingShellViewModel = $pages.ProjectItems | Where { $_.Name -eq "ShellViewModel.cs" } | Select -First 1 +if ($existingShellViewModel -ne $null) +{ + Write-Host ">>>> Not creating Pages/ShellViewModel.cs as it already exists" +} +else +{ + $shellViewModelContent = "using System; +using Stylet; + +namespace ${rootNamespace}.Pages +{ + public class ShellViewModel : Screen + { + } +} +" + + $shellViewModelPath = [System.IO.Path]::Combine($rootPath, "Pages", "ShellViewModel.cs") + [System.IO.File]::WriteAllText($shellViewModelPath, $shellViewModelContent) + $null = $pages.ProjectItems.AddFromFile($shellViewModelPath) +} + +Uninstall-Package Stylet.Start diff --git a/Stylet/EventAggregator.cs b/Stylet/EventAggregator.cs index 57a42dc..893abaa 100644 --- a/Stylet/EventAggregator.cs +++ b/Stylet/EventAggregator.cs @@ -108,23 +108,37 @@ namespace Stylet /// Channel(s) to publish the message to. Defaults to EventAggregator.DefaultChannel none given public void PublishWithDispatcher(object message, Action dispatcher, params string[] channels) { + // We have to be re-entrant, since a handler can fire another message, or subscribe. This means that we can't + // be in the middle of iterating this.handlers when we invoke a handler. + lock (this.handlersLock) { + // Start by clearing up dead handlers + this.handlers.RemoveAll(x => !x.IsAlive); + var messageType = message.GetType(); - var deadHandlers = this.handlers.Where(x => !x.Handle(messageType, message, dispatcher, channels)).ToArray(); - foreach (var deadHandler in deadHandlers) + var invokers = this.handlers.SelectMany(x => x.GetInvokers(messageType, channels)).ToArray(); + + foreach (var invoker in invokers) { - this.handlers.Remove(deadHandler); + dispatcher(() => invoker.Invoke(message)); } } } private class Handler { + private static readonly string[] DefaultChannelArray = new[] { DefaultChannel }; + private readonly WeakReference target; private readonly List invokers = new List(); private readonly HashSet channels = new HashSet(); + public bool IsAlive + { + get { return this.target.IsAlive; } + } + public Handler(object handler, string[] channels) { var handlerType = handler.GetType(); @@ -133,11 +147,11 @@ namespace Stylet foreach (var implementation in handler.GetType().GetInterfaces().Where(x => x.IsGenericType && typeof(IHandle).IsAssignableFrom(x))) { var messageType = implementation.GetGenericArguments()[0]; - this.invokers.Add(new HandlerInvoker(handlerType, messageType, implementation.GetMethod("Handle"))); + this.invokers.Add(new HandlerInvoker(this.target, handlerType, messageType, implementation.GetMethod("Handle"))); } if (channels.Length == 0) - channels = new[] { DefaultChannel }; + channels = DefaultChannelArray; this.SubscribeToChannels(channels); } @@ -160,35 +174,31 @@ namespace Stylet return this.channels.Count == 0; } - public bool Handle(Type messageType, object message, Action dispatcher, string[] channels) + public IEnumerable GetInvokers(Type messageType, string[] channels) { - var target = this.target.Target; - if (target == null) - return false; + if (!this.IsAlive) + return Enumerable.Empty(); if (channels.Length == 0) - channels = new[] { DefaultChannel }; + channels = DefaultChannelArray; // We're not subscribed to any of the channels if (!channels.All(x => this.channels.Contains(x))) - return true; + return Enumerable.Empty(); - foreach (var invoker in this.invokers) - { - invoker.Invoke(target, messageType, message, dispatcher); - } - - return true; + return this.invokers.Where(x => x.CanInvoke(messageType)); } } private class HandlerInvoker { + private readonly WeakReference target; private readonly Type messageType; private readonly Action invoker; - public HandlerInvoker(Type targetType, Type messageType, MethodInfo invocationMethod) + public HandlerInvoker(WeakReference target, Type targetType, Type messageType, MethodInfo invocationMethod) { + this.target = target; this.messageType = messageType; var targetParam = Expression.Parameter(typeof(object), "target"); var messageParam = Expression.Parameter(typeof(object), "message"); @@ -198,10 +208,17 @@ namespace Stylet this.invoker = Expression.Lambda>(callExpression, targetParam, messageParam).Compile(); } - public void Invoke(object target, Type messageType, object message, Action dispatcher) + public bool CanInvoke(Type messageType) { - if (this.messageType.IsAssignableFrom(messageType)) - dispatcher(() => this.invoker(target, message)); + return this.messageType.IsAssignableFrom(messageType); + } + + public void Invoke(object message) + { + var target = this.target.Target; + // Just in case it's expired... + if (target != null) + this.invoker(target, message); } } } diff --git a/StyletUnitTests/EventAggregatorTests.cs b/StyletUnitTests/EventAggregatorTests.cs index b908146..c93843c 100644 --- a/StyletUnitTests/EventAggregatorTests.cs +++ b/StyletUnitTests/EventAggregatorTests.cs @@ -30,6 +30,26 @@ namespace StyletUnitTests public void Handle(M1 message) { throw new Exception("Should not be called. Ever"); } } + public class C4 : IHandle + { + public EventAggregator EventAggregator; + + public void Handle(M1 message) + { + this.EventAggregator.Publish("foo"); + } + } + + public class C5 : IHandle + { + public EventAggregator EventAggregator; + + public void Handle(M1 message) + { + this.EventAggregator.Subscribe(new C4()); + } + } + private EventAggregator ea; [SetUp] @@ -230,5 +250,34 @@ namespace StyletUnitTests Assert.AreEqual(1, target.ReceivedMessageCount); } + + [Test] + public void PublishingInsideHandlerDoesNotThrow() + { + var target = new C4(); + target.EventAggregator = this.ea; + this.ea.Subscribe(target); + + // Add this as a dummy - it has to be a dead reference, which triggers modification of the + // 'handlers' collection + var dummyTarget = new C1(); + this.ea.Subscribe(dummyTarget); + dummyTarget = null; + GC.Collect(); + + Assert.DoesNotThrow(() => this.ea.Publish(new M1())); + + GC.KeepAlive(target); + } + + [Test] + public void SubscribingInsideHandlerDoesNotThrow() + { + var target = new C5(); + target.EventAggregator = this.ea; + this.ea.Subscribe(target); + + this.ea.Publish(new M1()); + } } }