Merge pull request #198 from mono/dev/lluis/fix-more-unload-issues

Fixed several exceptions that happen when unloading nodes 

9 files changed, 75 insertions, 51 deletions

diff --git a/Mono.Addins.Setup/Mono.Addins.Setup.csproj b/Mono.Addins.Setup/Mono.Addins.Setup.csproj
index abefad1..6649b34 100644
--- a/Mono.Addins.Setup/Mono.Addins.Setup.csproj
+++ b/Mono.Addins.Setup/Mono.Addins.Setup.csproj
@@ -1,9 +1,7 @@
-<?xml version="1.0" encoding="utf-8"?>
 <Project Sdk="Microsoft.NET.Sdk">
   <Import Project="..\TargetFrameworks.props" />
   <Import Project="..\Version.props" />
   <PropertyGroup>
-    <ProjectGuid>{A85C9721-C054-4BD8-A1F3-0227615F0A36}</ProjectGuid>
     <OutputType>Library</OutputType>
     <AssemblyName>Mono.Addins.Setup</AssemblyName>
     <RootNamespace>Mono.Addins.Setup</RootNamespace>
@@ -11,33 +9,13 @@
     <AssemblyOriginatorKeyFile>..\mono-addins.snk</AssemblyOriginatorKeyFile>
     <PackageId>Mono.Addins.Setup</PackageId>
     <Description>Mono.Addins is a framework for creating extensible applications, and for creating add-ins which extend applications. Mono.Addins.Setup provides an API for managing add-ins, creating add-in packages and publishing add-ins in on-line repositories.</Description>
-    <ProductVersion>8.0.30703</ProductVersion>
-    <SchemaVersion>2.0</SchemaVersion>
-    <PackageRequireLicenseAcceptance>true</PackageRequireLicenseAcceptance>
     <AssemblyVersion>1.0.0.0</AssemblyVersion>
     <DocumentationFile>..\bin\$(TargetFramework)\Mono.Addins.Setup.xml</DocumentationFile>
-  </PropertyGroup>
-  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
-    <DebugSymbols>True</DebugSymbols>
-    <DebugType>full</DebugType>
-    <Optimize>false</Optimize>
-    <OutputPath>..\bin</OutputPath>
-    <ErrorReport>prompt</ErrorReport>
-    <WarningLevel>4</WarningLevel>
-    <CheckForOverflowUnderflow>True</CheckForOverflowUnderflow>
-    <ConsolePause>False</ConsolePause>
+    <DebugType>embedded</DebugType>
     <NoWarn>1574;1591</NoWarn>
-  </PropertyGroup>
-  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
-    <DebugType>pdbonly</DebugType>
-    <Optimize>True</Optimize>
     <OutputPath>..\bin</OutputPath>
-    <ErrorReport>prompt</ErrorReport>
-    <WarningLevel>4</WarningLevel>
-    <CheckForOverflowUnderflow>True</CheckForOverflowUnderflow>
-    <ConsolePause>False</ConsolePause>
-    <DebugSymbols>true</DebugSymbols>
-    <NoWarn>1574;1591</NoWarn>
+    <LangVersion>9.0</LangVersion>
+    <PackageRequireLicenseAcceptance>true</PackageRequireLicenseAcceptance>
   </PropertyGroup>
   <ItemGroup>
     <PackageReference Include="SharpZipLib" Version="1.3.3" />
diff --git a/Mono.Addins/Mono.Addins.Database/DatabaseConfiguration.cs b/Mono.Addins/Mono.Addins.Database/DatabaseConfiguration.cs
index fa208d0..6eaa301 100644
--- a/Mono.Addins/Mono.Addins.Database/DatabaseConfiguration.cs
+++ b/Mono.Addins/Mono.Addins.Database/DatabaseConfiguration.cs
@@ -77,11 +77,12 @@ namespace Mono.Addins.Database
 				return copy;
 			}
 
-			public AddinStatus AsUninstalled ()
+			public AddinStatus AsUninstalled (ImmutableArray<string> oldAddinFiles)
 			{
 				var copy = Copy ();
 				copy.ConfigEnabled = false;
 				copy.Uninstalled = true;
+				copy.Files = oldAddinFiles;
 				return copy;
 			}
 
@@ -140,7 +141,7 @@ namespace Mono.Addins.Database
 			if (!addinStatus.TryGetValue (addinId, out s))
 				s = new AddinStatus (addinId);
 			
-			s = s.AsUninstalled ();
+			s = s.AsUninstalled (ImmutableArray<string>.Empty.AddRange(files));
 			addinStatus = addinStatus.SetItem (addinId, s);
 		}
 
diff --git a/Mono.Addins/Mono.Addins/AddinEngine.cs b/Mono.Addins/Mono.Addins/AddinEngine.cs
index 3bfe794..9986416 100644
--- a/Mono.Addins/Mono.Addins/AddinEngine.cs
+++ b/Mono.Addins/Mono.Addins/AddinEngine.cs
@@ -58,7 +58,9 @@ namespace Mono.Addins
 		string startupDirectory;
 		AddinRegistry registry;
 		IAddinInstaller installer;
-		
+
+		NotificationQueue notificationQueue;
+
 		bool checkAssemblyLoadConflicts;
 
 		// This collection is only used during a transaction, so it doesn't need to be immutable
@@ -117,9 +119,10 @@ namespace Mono.Addins
 		/// </summary>
 		public AddinEngine ()
 		{
+			notificationQueue = new NotificationQueue(this);
 		}
 
-        internal override ExtensionContextSnapshot CreateSnapshot ()
+		internal override ExtensionContextSnapshot CreateSnapshot ()
         {
 			return new AddinEngineSnapshot();
 		}
@@ -980,5 +983,10 @@ namespace Mono.Addins
 				},null);
 			}
 		}
+
+		internal void InvokeCallback(Action action, object source)
+		{
+			notificationQueue.Invoke(action, source);
+		}
 	}
 }
diff --git a/Mono.Addins/Mono.Addins/ExtensionContext.cs b/Mono.Addins/Mono.Addins/ExtensionContext.cs
index 7d03a1e..e46e657 100644
--- a/Mono.Addins/Mono.Addins/ExtensionContext.cs
+++ b/Mono.Addins/Mono.Addins/ExtensionContext.cs
@@ -55,8 +55,6 @@ namespace Mono.Addins
 		ExtensionContext parentContext;
 		ExtensionTree tree;
 
-		NotificationQueue notificationQueue;
-
 		// runTimeEnabledAddins and runTimeDisabledAddins are modified only within a transaction,
 		// so they don't need to be immutable and don't need to be in the snapshot
 		HashSet<string> runTimeEnabledAddins = new HashSet<string>();
@@ -99,7 +97,6 @@ namespace Mono.Addins
 		
 		internal void Initialize (AddinEngine addinEngine)
 		{
-			notificationQueue = new NotificationQueue(addinEngine);
 			SetSnapshot(CreateSnapshot());
 			tree = new ExtensionTree (addinEngine, this);
 		}
@@ -123,11 +120,6 @@ namespace Mono.Addins
 		}
 #pragma warning restore 1591
 
-		internal void InvokeCallback(Action action, object source)
-		{
-			notificationQueue.Invoke(action, source);
-		}
-		
 		internal void ClearContext ()
 		{
 			SetSnapshot(CreateSnapshot());
@@ -863,7 +855,7 @@ namespace Mono.Addins
 		{
 			if (ExtensionChanged != null)
 			{
-				notificationQueue.Invoke(() =>
+				AddinEngine.InvokeCallback(() =>
 				{
 					ExtensionChanged?.Invoke(this, args);
 				}, null);
diff --git a/Mono.Addins/Mono.Addins/ExtensionContextTransaction.cs b/Mono.Addins/Mono.Addins/ExtensionContextTransaction.cs
index 99cba20..ba2a245 100644
--- a/Mono.Addins/Mono.Addins/ExtensionContextTransaction.cs
+++ b/Mono.Addins/Mono.Addins/ExtensionContextTransaction.cs
@@ -165,9 +165,7 @@ namespace Mono.Addins
 			{
 				foreach (var node in childrenChanged)
 				{
-					// It may happen that a node is removed while updating its parent. In this case the parent
-					// will be set to null, and then there is no need to notify changes
-					if (node.Parent != null && node.NotifyChildrenChanged())
+					if (node.NotifyChildrenChanged())
 						NotifyExtensionsChangedEvent(node.GetPath());
 				}
 			}
diff --git a/Mono.Addins/Mono.Addins/ExtensionNode.cs b/Mono.Addins/Mono.Addins/ExtensionNode.cs
index 36f227d..0947a89 100644
--- a/Mono.Addins/Mono.Addins/ExtensionNode.cs
+++ b/Mono.Addins/Mono.Addins/ExtensionNode.cs
@@ -158,7 +158,7 @@ namespace Mono.Addins
 			get {
 				if (addin == null && addinId != null) {
 					addin = addinEngine.GetOrLoadAddin(addinId, true);
-					if (addin != null)
+					if (addin != null && module != null)
 						addin = addin.GetModule (module);
 				}
 				if (addin == null)
@@ -179,7 +179,7 @@ namespace Mono.Addins
 		public event ExtensionNodeEventHandler ExtensionNodeChanged {
 			add
 			{
-				ExtensionContext.InvokeCallback(() =>
+				addinEngine.InvokeCallback(() =>
 				{
 					// The invocation here needs to be done right to make sure there are no duplicate or missing events.
 
@@ -208,7 +208,7 @@ namespace Mono.Addins
 				}, this);
 			}
 			remove {
-				ExtensionContext.InvokeCallback(() =>
+				addinEngine.InvokeCallback(() =>
 				{
 					// This is done inside a InvokeCallback call for simetry with the 'add' block. Since the 'add'
 					// block runs on a callback that can be queued, doing the same here ensures that the unsubscription
@@ -564,20 +564,20 @@ namespace Mono.Addins
 					var node = change.Node;
 					if (change.Added)
 					{
-						ExtensionContext.InvokeCallback(() =>
+						addinEngine.InvokeCallback(() =>
 						{
 							OnChildNodeAdded(node);
 						}, this);
 					}
 					else
 					{
-						ExtensionContext.InvokeCallback(() =>
+						addinEngine.InvokeCallback(() =>
 						{
 							OnChildNodeRemoved(node);
 						}, this);
 					}
 				}
-				ExtensionContext.InvokeCallback(OnChildrenChanged, this);
+				addinEngine.InvokeCallback(OnChildrenChanged, this);
 				return true;
 			} else
 				return false;
@@ -585,12 +585,12 @@ namespace Mono.Addins
 
 		internal void NotifyAddinLoaded()
 		{
-			ExtensionContext.InvokeCallback(OnAddinLoaded, this);
+			addinEngine.InvokeCallback(OnAddinLoaded, this);
 		}
 
 		internal void NotifyAddinUnloaded()
 		{
-			ExtensionContext.InvokeCallback(OnAddinUnloaded, this);
+			addinEngine.InvokeCallback(OnAddinUnloaded, this);
 		}
 
 		/// <summary>
diff --git a/Test/UnitTests/TestEvents.cs b/Test/UnitTests/TestEvents.cs
index c0ca45a..2b4b988 100644
--- a/Test/UnitTests/TestEvents.cs
+++ b/Test/UnitTests/TestEvents.cs
@@ -439,5 +439,45 @@ namespace UnitTests
 		{
 			counters[0].Update (args);
 		}
-	}
+
+		[Test()]
+		public void TestUnloadedNode()
+		{
+			try
+			{
+                AddinManager.AddinLoadError += AddinManager_AddinLoadError;
+				Assert.AreEqual(4, AddinManager.GetExtensionNodes("/SimpleApp/Writers").Count, "count 1");
+				AddinManager.AddExtensionNodeHandler("/SimpleApp/Writers", OnExtensionChange2);
+				AddinManager.Registry.DisableAddin("SimpleApp.FileContentExtension,0.1.0");
+				Assert.AreEqual(3, AddinManager.GetExtensionNodes("/SimpleApp/Writers").Count, "count 2");
+			}
+			finally
+			{
+				AddinManager.RemoveExtensionNodeHandler("/SimpleApp/Writers", OnExtensionChange2);
+				AddinManager.AddinLoadError -= AddinManager_AddinLoadError;
+				AddinManager.Registry.EnableAddin("SimpleApp.FileContentExtension,0.1.0");
+			}
+		}
+
+        private void AddinManager_AddinLoadError (object sender, AddinErrorEventArgs args)
+        {
+			throw new Exception(args.Message);
+        }
+
+        void OnExtensionChange2(object s, ExtensionNodeEventArgs args)
+		{
+			if (args.Change == ExtensionChange.Add)
+			{
+				args.ExtensionNode.ExtensionNodeChanged += ExtensionNode_ExtensionNodeChanged;
+			}
+			else
+			{
+				args.ExtensionNode.ExtensionNodeChanged -= ExtensionNode_ExtensionNodeChanged;
+			}
+		}
+
+        private void ExtensionNode_ExtensionNodeChanged (object sender, ExtensionNodeEventArgs args)
+        {
+        }
+    }
 }
diff --git a/Test/UnitTests/TestExtensions.cs b/Test/UnitTests/TestExtensions.cs
index 6e3812f..6befd43 100644
--- a/Test/UnitTests/TestExtensions.cs
+++ b/Test/UnitTests/TestExtensions.cs
@@ -247,5 +247,12 @@ namespace UnitTests
 			Assert.AreEqual ("n3", ids[6]);
 			Assert.AreEqual ("n4", ids[7]);
 		}
+
+		[Test]
+		public void TreeNodeHasAddin()
+		{
+			var node = AddinManager.GetExtensionNode("/SimpleApp/DefaultInsertBefore");
+			Assert.IsNotNull(node.Addin);
+		}
 	}
 }
diff --git a/Version.props b/Version.props
index d6968ef..cc2b09f 100644
--- a/Version.props
+++ b/Version.props
@@ -1,6 +1,6 @@
 <Project>
   <PropertyGroup>
-    <PackageVersion>1.4.1</PackageVersion>
+    <PackageVersion>1.4.2-alpha.3</PackageVersion>
     <Authors>Microsoft</Authors>
     <Owners>microsoft, xamarin</Owners>
     <PackageLicenseUrl>https://github.com/mono/mono-addins/blob/main/COPYING</PackageLicenseUrl>