Fixed several exceptions that happen when unloading nodes

Moved notification queue from ExtensionContext to to add-in engine. The problem
was that when a node is removed, it is not bound to a context anymore, but it
may still need to make use of the event queue.

7 files changed, 68 insertions, 24 deletions

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..3bedf9e 100644
--- a/Test/UnitTests/TestEvents.cs
+++ b/Test/UnitTests/TestEvents.cs
@@ -439,5 +439,44 @@ 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.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..c6719a5 100644
--- a/Version.props
+++ b/Version.props
@@ -1,6 +1,6 @@
 <Project>
   <PropertyGroup>
-    <PackageVersion>1.4.1</PackageVersion>
+    <PackageVersion>1.4.2-alpha.1</PackageVersion>
     <Authors>Microsoft</Authors>
     <Owners>microsoft, xamarin</Owners>
     <PackageLicenseUrl>https://github.com/mono/mono-addins/blob/main/COPYING</PackageLicenseUrl>