Thread safety fixes

Remove the concept of TreeNodeBuilder, it was getting
too complex. An easier solution is to add a transaction
mode to TreeNode, which when enabled handles children
in a list builder, and which is committed when ending
the context transaction.

Fixed several bugs.

8 files changed, 344 insertions, 400 deletions

diff --git a/Mono.Addins/Mono.Addins.Database/DatabaseConfiguration.cs b/Mono.Addins/Mono.Addins.Database/DatabaseConfiguration.cs
index 9c73d25..fa208d0 100644
--- a/Mono.Addins/Mono.Addins.Database/DatabaseConfiguration.cs
+++ b/Mono.Addins/Mono.Addins.Database/DatabaseConfiguration.cs
@@ -49,7 +49,10 @@ namespace Mono.Addins.Database
 				ConfigEnabled = configEnabled;
 				SessionEnabled = sessionEnabled;
 				Uninstalled = uninstalled;
-				Files = files;
+				if (files.IsDefault)
+					Files = ImmutableArray<string>.Empty;
+				else
+					Files = files;
 			}
 
 			AddinStatus Copy ()
@@ -66,9 +69,9 @@ namespace Mono.Addins.Database
 			{
 				var copy = Copy ();
 				if (sessionOnly)
-					copy.SessionEnabled = true;
+					copy.SessionEnabled = enabled;
 				else {
-					copy.ConfigEnabled = true;
+					copy.ConfigEnabled = enabled;
 					copy.SessionEnabled = null;
 				}
 				return copy;
diff --git a/Mono.Addins/Mono.Addins/AddinEngine.cs b/Mono.Addins/Mono.Addins/AddinEngine.cs
index 1e0a615..1e2c39a 100644
--- a/Mono.Addins/Mono.Addins/AddinEngine.cs
+++ b/Mono.Addins/Mono.Addins/AddinEngine.cs
@@ -632,7 +632,7 @@ namespace Mono.Addins
 
 		internal void BulkRegisterAssemblyResolvePaths (IEnumerable<KeyValuePair<string,RuntimeAddin>> registrations)
 		{
-			assemblyResolvePaths.SetItems (registrations);
+			assemblyResolvePaths = assemblyResolvePaths.SetItems (registrations);
 		}
 
 		internal void InsertExtensionPoint (ExtensionContextTransaction transaction, RuntimeAddin addin, ExtensionPoint ep)
diff --git a/Mono.Addins/Mono.Addins/ExtensionContext.cs b/Mono.Addins/Mono.Addins/ExtensionContext.cs
index 00db5d1..58a296f 100644
--- a/Mono.Addins/Mono.Addins/ExtensionContext.cs
+++ b/Mono.Addins/Mono.Addins/ExtensionContext.cs
@@ -214,6 +214,11 @@ namespace Mono.Addins
 				info.CondType = conditionTypeObject;
 				conditionTypes = conditionTypes.Add (id, info);
 			} else {
+				// If CondType is not changing, nothing else to do
+				if (conditionTypeObject == null)
+					return info;
+
+				// Replace the old CondType
 				var oldType = info.CondType as ConditionType;
 				info.CondType = conditionTypeObject;
 				if (oldType != null)
@@ -841,6 +846,8 @@ namespace Mono.Addins
 		{
 			return new ExtensionContextTransaction (this);
 		}
+
+		internal bool IsCurrentThreadInTransaction => Monitor.IsEntered (LocalLock);
 		
 		void OnConditionChanged (object s, EventArgs a)
 		{
@@ -938,11 +945,8 @@ namespace Mono.Addins
 					foreach (Extension ext in data.Extensions) {
 						TreeNode node = GetNode (ext.Path);
 						if (node != null && node.ExtensionNodeSet != null) {
-							if (node.ChildrenLoaded) {
-								var builder = TreeNodeBuilder.FromNode (node);
-								LoadModuleExtensionNodes (transaction, builder, ext, data.AddinId);
-								builder.Build (transaction);
-							}
+							if (node.ChildrenFromExtensionsLoaded)
+								LoadModuleExtensionNodes (transaction, node, ext, data.AddinId);
 						}
 						else
 							AddinEngine.ReportError ("Extension node not found or not extensible: " + ext.Path, id, null, false);
@@ -970,7 +974,7 @@ namespace Mono.Addins
 			// Remove each node and notify the change
 			foreach (TreeNode node in list) {
 				node.NotifyAddinUnloaded ();
-				node.Remove (transaction);
+				node.Parent?.RemoveChild (transaction, node);
 			}
 
 			// Notify global extension point changes.
@@ -991,7 +995,7 @@ namespace Mono.Addins
 					}
 				}
 				foreach (string path in paths)
-					NotifyExtensionsChanged (new ExtensionEventArgs (path));
+					transaction.NotifyExtensionsChangedEvent (path);
 			}
 		}
 		
@@ -1041,51 +1045,65 @@ namespace Mono.Addins
 			
 			return newlist != null ? newlist : col;
 		}
-		
+
 		// Load the extension nodes at the specified path. If the path
 		// contains extension nodes implemented in an add-in which is
 		// not loaded, the add-in will be automatically loaded
-		
-		internal void LoadExtensions (ExtensionContextTransaction transaction, ExtensionPoint ep, TreeNodeBuilder node)
+
+		internal void LoadExtensions (ExtensionContextTransaction transaction, string requestedExtensionPath, TreeNode node)
 		{
-			// Collect extensions to be loaded from add-ins. Before loading the extensions,
-			// they must be sorted, that's why loading is split in two steps (collecting + loading).
-
-			var addins = GetAddinsForPath (ep.Addins);
-			var loadData = new List<ExtensionLoadData> (addins.Count);
-
-			foreach (string addin in addins) {
-				ExtensionLoadData ed = GetAddinExtensions (addin, ep);
-				if (ed != null) {
-					// Insert the addin data taking into account dependencies.
-					// An add-in must be processed after all its dependencies.
-					bool added = false;
-					for (int n=0; n<loadData.Count; n++) {
-						ExtensionLoadData other = loadData [n];
-						if (AddinEngine.Registry.AddinDependsOn (other.AddinName, ed.AddinName)) {
-							loadData.Insert (n, ed);
-							added = true;
-							break;
+			if (node == null)
+				throw new InvalidOperationException ("Extension point not defined: " + requestedExtensionPath);
+
+			ExtensionPoint ep = node.ExtensionPoint;
+
+			if (ep != null) {
+
+				// Collect extensions to be loaded from add-ins. Before loading the extensions,
+				// they must be sorted, that's why loading is split in two steps (collecting + loading).
+
+				var addins = GetAddinsForPath (ep.Addins);
+				var loadData = new List<ExtensionLoadData> (addins.Count);
+
+				foreach (string addin in addins) {
+					ExtensionLoadData ed = GetAddinExtensions (addin, ep);
+					if (ed != null) {
+						// Insert the addin data taking into account dependencies.
+						// An add-in must be processed after all its dependencies.
+						bool added = false;
+						for (int n = 0; n < loadData.Count; n++) {
+							ExtensionLoadData other = loadData [n];
+							if (AddinEngine.Registry.AddinDependsOn (other.AddinName, ed.AddinName)) {
+								loadData.Insert (n, ed);
+								added = true;
+								break;
+							}
 						}
+						if (!added)
+							loadData.Add (ed);
 					}
-					if (!added)
-						loadData.Add (ed);
 				}
-			}
-				
-			// Now load the extensions
-				
-			foreach (ExtensionLoadData data in loadData) {
-				foreach (Extension ext in data.Extensions) {
-					var cnode = node.GetNode (ext.Path);
-					if (cnode != null)
-						LoadModuleExtensionNodes (transaction, cnode, ext, data.AddinId);
-					else
-						AddinEngine.ReportError ("Extension node not found or not extensible: " + ext.Path, data.AddinId, null, false);
+
+				// Now load the extensions
+
+				var loadedNodes = new List<TreeNode> ();
+				foreach (ExtensionLoadData data in loadData) {
+					foreach (Extension ext in data.Extensions) {
+						TreeNode cnode = GetNode (ext.Path);
+						if (cnode != null && cnode.ExtensionNodeSet != null)
+							LoadModuleExtensionNodes (transaction, cnode, ext, data.AddinId);
+						else
+							AddinEngine.ReportError ("Extension node not found or not extensible: " + ext.Path, data.AddinId, null, false);
+					}
 				}
+				// Call the OnAddinLoaded method on nodes, if the add-in is already loaded
+				foreach (TreeNode nod in loadedNodes)
+					nod.ExtensionNode.OnAddinLoaded ();
+
+				NotifyExtensionsChanged (new ExtensionEventArgs (requestedExtensionPath));
 			}
 		}
-		
+
 		ExtensionLoadData GetAddinExtensions (string id, ExtensionPoint ep)
 		{
 			Addin pinfo = null;
@@ -1137,7 +1155,7 @@ namespace Mono.Addins
 			}
 		}
 		
-		void LoadModuleExtensionNodes (ExtensionContextTransaction transaction, TreeNodeBuilder node, Extension extension, string addinId)
+		void LoadModuleExtensionNodes (ExtensionContextTransaction transaction, TreeNode node, Extension extension, string addinId)
 		{
 			// Now load the extensions
 			var addedNodes = new List<TreeNode> ();
@@ -1186,44 +1204,47 @@ namespace Mono.Addins
 			TreeNode srcNode = parentContext.tree;
 			TreeNode dstNode = tree;
 
-			foreach (string part in parts) {
+			ExtensionContextTransaction transaction = null;
 
-				// Look for the node in the source tree (from parent context)
+			try {
+				foreach (string part in parts) {
 
-				srcNode = srcNode.GetChildNode (part);
-				if (srcNode == null)
-					return null;
+					// Look for the node in the source tree (from parent context)
 
-				// Now get the node in the target tree
+					srcNode = srcNode.GetChildNode (part);
+					if (srcNode == null)
+						return null;
 
-				var dstNodeChild = dstNode.GetChildNode (part);
-				if (dstNodeChild != null) {
-					dstNode = dstNodeChild;
-				}
-				else {
-					using var transaction = BeginTransaction ();
+					// Now get the node in the target tree
 
-					// Check again just in case the node was created while taking the transaction
-					dstNodeChild = dstNode.GetChildNode (part);
-					if (dstNodeChild != null)
+					var dstNodeChild = dstNode.GetChildNode (part);
+					if (dstNodeChild != null) {
 						dstNode = dstNodeChild;
-					else {
-
-						// Create if not found
-						// Copy extension data from the parent context node
-
-						TreeNode newNode = srcNode.Clone (AddinEngine);
-
-						if (newNode.Condition != null)
-							transaction.RegisterNodeCondition (newNode, newNode.Condition);
-
-						// Don't rise extension change events since we are just building the tree, not modifying it
-						transaction.DisableEvents = true;
-
-						dstNode.AddChildNode (transaction, newNode);
-						dstNode = newNode;
+					} else {
+						if (transaction == null)
+							transaction = BeginTransaction ();
+
+						// Check again just in case the node was created while taking the transaction
+						dstNodeChild = dstNode.GetChildNode (part);
+						if (dstNodeChild != null)
+							dstNode = dstNodeChild;
+						else {
+
+							// Create if not found
+							TreeNode newNode = new TreeNode (AddinEngine, part);
+
+							// Copy extension data
+							newNode.ExtensionNodeSet = srcNode.ExtensionNodeSet;
+							newNode.ExtensionPoint = srcNode.ExtensionPoint;
+							newNode.Condition = srcNode.Condition;
+
+							dstNode.AddChildNode (transaction, newNode);
+							dstNode = newNode;
+						}
 					}
 				}
+			} finally {
+				transaction?.Dispose ();
 			}
 			
 			return dstNode;
diff --git a/Mono.Addins/Mono.Addins/ExtensionContextTransaction.cs b/Mono.Addins/Mono.Addins/ExtensionContextTransaction.cs
index f961d8c..3507f49 100644
--- a/Mono.Addins/Mono.Addins/ExtensionContextTransaction.cs
+++ b/Mono.Addins/Mono.Addins/ExtensionContextTransaction.cs
@@ -46,6 +46,7 @@ namespace Mono.Addins
 		List<string> unregisteredAssemblyResolvePaths;
 		List<RuntimeAddin> addinLoadEvents;
 		List<string> addinUnloadEvents;
+		List<TreeNode> treeNodeTransactions;
 
 		public ExtensionContextTransaction (ExtensionContext context)
 		{
@@ -67,6 +68,13 @@ namespace Mono.Addins
 				if (nodeConditionUnregistrations != null) {
 					Context.BulkUnregisterNodeConditions (this, nodeConditionUnregistrations);
 				}
+
+				// Commit tree node transactions
+				if (treeNodeTransactions != null) {
+					foreach (var node in treeNodeTransactions)
+						node.CommitChildrenUpdateTransaction ();
+				}
+
 			} finally {
 				Monitor.Exit (Context.LocalLock);
 			}
@@ -152,6 +160,8 @@ namespace Mono.Addins
 			if (nodeConditionUnregistrations == null)
 				nodeConditionUnregistrations = new List<(TreeNode Node, BaseCondition Condition)> ();
 			nodeConditionUnregistrations.Add ((node, cond));
+			if (nodeConditions != null)
+				nodeConditions.Remove ((node, cond));
 		}
 
 		public void RegisterAutoTypeExtensionPoint (string typeName, string path)
@@ -195,5 +205,12 @@ namespace Mono.Addins
 				addinUnloadEvents = new List<string> ();
 			addinUnloadEvents.Add (id);
 		}
+
+		public void RegisterChildrenUpdateTransaction (TreeNode node)
+		{
+			if (treeNodeTransactions == null)
+				treeNodeTransactions = new List<TreeNode> ();
+			treeNodeTransactions.Add (node);
+		}
 	}
 }
diff --git a/Mono.Addins/Mono.Addins/ExtensionNode.cs b/Mono.Addins/Mono.Addins/ExtensionNode.cs
index 404881e..ffb9243 100644
--- a/Mono.Addins/Mono.Addins/ExtensionNode.cs
+++ b/Mono.Addins/Mono.Addins/ExtensionNode.cs
@@ -463,7 +463,7 @@ namespace Mono.Addins
 			var nodeChildren = treeNode.Children;
 
 			try {
-				if (nodeChildren.Length == 0) {
+				if (nodeChildren.Count == 0) {
 					return ExtensionNodeList.Empty;
 				}
 			} catch (Exception ex) {
@@ -471,14 +471,14 @@ namespace Mono.Addins
 				return ExtensionNodeList.Empty;
 			}
 
-			List<ExtensionNode> list = new List<ExtensionNode> (nodeChildren.Length);
+			List<ExtensionNode> list = new List<ExtensionNode> (nodeChildren.Count);
 			foreach (TreeNode cn in nodeChildren) {
 
 				// For each node check if it is visible for the current context.
 				// If something fails while evaluating the condition, just ignore the node.
 
 				try {
-					if (cn.IsEnabled && cn.ExtensionNode != null)
+					if (cn.IsEnabled && cn.HasExtensionNode)
 						list.Add (cn.ExtensionNode);
 				} catch (Exception ex) {
 					addinEngine.ReportError (null, null, ex, false);
diff --git a/Mono.Addins/Mono.Addins/ExtensionTree.cs b/Mono.Addins/Mono.Addins/ExtensionTree.cs
index a3af268..37a16ba 100644
--- a/Mono.Addins/Mono.Addins/ExtensionTree.cs
+++ b/Mono.Addins/Mono.Addins/ExtensionTree.cs
@@ -53,7 +53,7 @@ namespace Mono.Addins
 		}
 
 		
-		public void LoadExtension (ExtensionContextTransaction transaction, TreeNodeBuilder tnode, string addin, Extension extension, List<TreeNode> addedNodes)
+		public void LoadExtension (ExtensionContextTransaction transaction, TreeNode tnode, string addin, Extension extension, List<TreeNode> addedNodes)
 		{
 			if (tnode == null) {
 				addinEngine.ReportError ("Can't load extensions for path '" + extension.Path + "'. Extension point not defined.", addin, null, false);
@@ -64,10 +64,10 @@ namespace Mono.Addins
 			LoadExtensionElement (transaction, tnode, addin, extension.ExtensionNodes, (ModuleDescription) extension.Parent, ref curPos, tnode.Condition, false, addedNodes);
 		}
 
-		void LoadExtensionElement (ExtensionContextTransaction transaction, TreeNodeBuilder parentNode, string addin, ExtensionNodeDescriptionCollection extension, ModuleDescription module, ref int curPos, BaseCondition parentCondition, bool inComplextCondition, List<TreeNode> addedNodes)
+		void LoadExtensionElement (ExtensionContextTransaction transaction, TreeNode parentNode, string addin, ExtensionNodeDescriptionCollection extension, ModuleDescription module, ref int curPos, BaseCondition parentCondition, bool inComplextCondition, List<TreeNode> addedNodes)
 		{
 			foreach (ExtensionNodeDescription elem in extension) {
-					
+
 				if (inComplextCondition) {
 					parentCondition = ReadComplexCondition (elem, parentCondition);
 					inComplextCondition = false;
@@ -78,22 +78,25 @@ namespace Mono.Addins
 					LoadExtensionElement (transaction, parentNode, addin, elem.ChildNodes, module, ref curPos, parentCondition, true, addedNodes);
 					continue;
 				}
-					
+
 				if (elem.NodeName == "Condition") {
 					Condition cond = new Condition (AddinEngine, elem, parentCondition);
 					LoadExtensionElement (transaction, parentNode, addin, elem.ChildNodes, module, ref curPos, cond, false, addedNodes);
 					continue;
 				}
 
-				ExtensionPoint extensionPoint = parentNode.GetExtensionPoint ();
-					
+				var pnode = parentNode;
+				ExtensionPoint extensionPoint = null;
+				while (pnode != null && (extensionPoint = pnode.ExtensionPoint) == null)
+					pnode = pnode.Parent;
+
 				string after = elem.GetAttribute ("insertafter");
 				if (after.Length == 0 && extensionPoint != null && curPos == -1)
 					after = extensionPoint.DefaultInsertAfter;
 				if (after.Length > 0) {
 					int i = parentNode.IndexOfChild (after);
 					if (i != -1)
-						curPos = i+1;
+						curPos = i + 1;
 				}
 				string before = elem.GetAttribute ("insertbefore");
 				if (before.Length == 0 && extensionPoint != null && curPos == -1)
@@ -106,29 +109,33 @@ namespace Mono.Addins
 
 				// If node position is not explicitly set, add the node at the end
 				if (curPos == -1)
-					curPos = parentNode.ChildrenCount;
-				
+					curPos = parentNode.Children.Count;
+
 				// Find the type of the node in this extension
 				ExtensionNodeType ntype = addinEngine.FindType (parentNode.ExtensionNodeSet, elem.NodeName, addin);
-				
+
 				if (ntype == null) {
 					addinEngine.ReportError ("Node '" + elem.NodeName + "' not allowed in extension: " + parentNode.GetPath (), addin, null, false);
 					continue;
 				}
-				
+
 				string id = elem.GetAttribute ("id");
 				if (id.Length == 0)
 					id = AutoIdPrefix + (++internalId);
 
-				TreeNodeBuilder childNode = TreeNodeBuilder.CreateNew (addinEngine, id, parentNode);
-				
+				TreeNode childNode = new TreeNode (addinEngine, id);
+
 				ExtensionNode enode = ReadNode (childNode, addin, ntype, elem, module);
 				if (enode == null)
 					continue;
 
+				// Enables bulk update of children
+				parentNode.BeginChildrenUpdateTransaction (transaction);
+
 				childNode.Condition = parentCondition;
 				childNode.ExtensionNodeSet = ntype;
-				parentNode.InsertChild (curPos, childNode);
+				parentNode.InsertChild (transaction, curPos, childNode);
+				addedNodes.Add (childNode);
 
 				// Load children
 				if (elem.ChildNodes.Count > 0) {
@@ -166,7 +173,7 @@ namespace Mono.Addins
 			return new NullCondition ();
 		}
 		
-		public ExtensionNode ReadNode (TreeNodeBuilder tnode, string addin, ExtensionNodeType ntype, ExtensionNodeDescription elem, ModuleDescription module)
+		public ExtensionNode ReadNode (TreeNode tnode, string addin, ExtensionNodeType ntype, ExtensionNodeDescription elem, ModuleDescription module)
 		{
 			try {
 				if (ntype.Type == null) {
diff --git a/Mono.Addins/Mono.Addins/TreeNode.cs b/Mono.Addins/Mono.Addins/TreeNode.cs
index b3e49ba..3a79e49 100644
--- a/Mono.Addins/Mono.Addins/TreeNode.cs
+++ b/Mono.Addins/Mono.Addins/TreeNode.cs
@@ -33,33 +33,35 @@ using System.Collections;
 using Mono.Addins.Description;
 using System.Collections.Generic;
 using System.Collections.Immutable;
-using System.Linq;
 
 namespace Mono.Addins
 {
 	class TreeNode
 	{
 		ImmutableArray<TreeNode> children;
+
+		ImmutableArray<TreeNode>.Builder childrenBuilder;
+		ExtensionContextTransaction buildTransaction;
+
 		ExtensionNode extensionNode;
-		bool childrenLoaded;
-		readonly string id;
+		bool childrenFromExtensionsLoaded;
+		string id;
 		TreeNode parent;
 		ExtensionNodeSet nodeTypes;
 		ExtensionPoint extensionPoint;
 		BaseCondition condition;
-		protected readonly AddinEngine addinEngine;
+		protected AddinEngine addinEngine;
 
-		public TreeNode (AddinEngine addinEngine, string id, TreeNode parent = null)
+		public TreeNode (AddinEngine addinEngine, string id)
 		{
 			this.id = id;
 			this.addinEngine = addinEngine;
-			this.parent = parent;
 
 			children = ImmutableArray<TreeNode>.Empty;
 
 			// Root node
 			if (id.Length == 0)
-				childrenLoaded = true;
+				childrenFromExtensionsLoaded = true;
 		}
 		
 		public AddinEngine AddinEngine {
@@ -100,13 +102,13 @@ namespace Mono.Addins
 
 		public bool HasExtensionNode {
 			get {
-				return extensionPoint != null;
+				return extensionNode != null || extensionPoint != null;
 			}
 		}
 
 		public string AddinId {
 			get {
-				return extensionPoint?.RootAddin;
+				return extensionNode != null ? extensionNode.AddinId : extensionPoint?.RootAddin;
 			}
 		}
 
@@ -152,38 +154,91 @@ namespace Mono.Addins
 			}
 		}
 		
-		public bool ChildrenLoaded {
-			get { return childrenLoaded; }
+		public bool ChildrenFromExtensionsLoaded {
+			get { return childrenFromExtensionsLoaded; }
 		}
-
+		
 		public void AddChildNode (ExtensionContextTransaction transaction, TreeNode node)
 		{
-			node.parent = this;
-			children = children.Add (node);
+			node.SetParent(transaction, this);
+
+			if (childrenBuilder != null)
+				childrenBuilder.Add (node);
+			else
+				children = children.Add (node);
+
 			transaction.ReportChildrenChanged (this);
 		}
 
-		internal void SetChildren (ExtensionContextTransaction transaction, ImmutableArray<TreeNode> children)
+		public void InsertChild (ExtensionContextTransaction transaction, int n, TreeNode node)
 		{
-			foreach (var node in children) {
-				node.parent = this;
-				if (node == this)
-					throw new InvalidOperationException ();
-			}
+			node.SetParent (transaction, this);
+
+			if (childrenBuilder != null)
+				childrenBuilder.Insert (n, node);
+			else
+				children = children.Insert (n, node);
+
+			transaction.ReportChildrenChanged (this);
+		}
+
+		public void RemoveChild (ExtensionContextTransaction transaction, TreeNode node)
+		{
+			node.SetParent (transaction, null);
+
+			if (childrenBuilder != null)
+				childrenBuilder.Remove (node);
+			else
+				children = children.Remove (node);
+
+			transaction.ReportChildrenChanged (this);
+		}
+
+		void SetParent (ExtensionContextTransaction transaction, TreeNode newParent)
+		{
+			if (parent == newParent)
+				return;
 
-			this.children = children;
+			if (this.parent != null && newParent != null)
+				throw new InvalidOperationException ("Node already has a parent");
 
+			var currentCtx = Context;
+			if (currentCtx != null && currentCtx != transaction.Context)
+				throw new InvalidOperationException ("Invalid context");
 
-			if (childrenLoaded) {
-				// If children were already loaded, we need to notify that they have changed
-				transaction.ReportChildrenChanged (this);
+			this.parent = newParent;
+
+			if (newParent != null) {
+				if (newParent.Context != null)
+					OnAttachedToContext (transaction);
 			} else {
-				// If children were not loaded, there is no need to notify change events,
-				// just set the loaded flag
-				childrenLoaded = true;
+				if (currentCtx != null)
+					OnDetachedFromContext (transaction);
 			}
 		}
 
+		void OnAttachedToContext (ExtensionContextTransaction transaction)
+		{
+			// Once the node is part of a context, let's register the condition
+			if (Condition != null)
+				transaction.RegisterNodeCondition (this, Condition);
+
+			// Propagate event to children
+			foreach (var child in GetLoadedChildren ())
+				child.OnAttachedToContext (transaction);
+		}
+
+		void OnDetachedFromContext (ExtensionContextTransaction transaction)
+		{
+			// Node being removed, unregister from context
+			if (Condition != null)
+				transaction.UnregisterNodeCondition (this, Condition);
+
+			// Propagate event to children
+			foreach (var child in GetLoadedChildren ())
+				child.OnDetachedFromContext (transaction);
+		}
+
 		public ExtensionNode GetExtensionNode (string path, string childId)
 		{
 			TreeNode node = GetNode (path, childId);
@@ -218,61 +273,131 @@ namespace Mono.Addins
 			TreeNode curNode = this;
 
 			foreach (string part in parts) {
-				int i = curNode.Children.IndexOfNode (part);
-				if (i != -1) {
-					curNode = curNode.Children [i];
+				var node = curNode.GetChildNode (part);
+				if (node != null) {
+					curNode = node;
 					continue;
 				}
 				
 				if (buildPath) {
-					TreeNode newNode = new TreeNode (addinEngine, part);
-					curNode.AddChildNode (newNode);
-					curNode = newNode;
+					var transaction = BeginContextTransaction (out var dispose);
+					try {
+						// Check again inside the lock, just in case
+						node = curNode.GetChildNode (part);
+						if (node != null) {
+							curNode = node;
+							continue;
+						}
+
+						TreeNode newNode = new TreeNode (addinEngine, part);
+						curNode.AddChildNode (transaction, newNode);
+						curNode = newNode;
+					} finally {
+						if (dispose)
+							transaction.Dispose ();
+					}
 				} else
 					return null;
 			}
 			return curNode;
 		}
-		public ImmutableArray<TreeNode> Children {
+
+		public TreeNode GetChildNode (string id)
+		{
+			var childrenList = Children;
+			foreach (var node in childrenList) {
+				if (node.Id == id)
+					return node;
+			}
+			return null;
+		}
+
+		public int IndexOfChild (string id)
+		{
+			var childrenList = Children;
+			for (int n = 0; n < childrenList.Count; n++) {
+				if (childrenList [n].Id == id)
+					return n;
+			}
+			return -1;
+		}
+
+		public IReadOnlyList<TreeNode> Children {
 			get {
-				if (!childrenLoaded) {
-					childrenLoaded = true;
-					if (extensionPoint != null)
-						Context.LoadExtensions (GetPath ());
-					// We have to keep the relation info, since add-ins may be loaded/unloaded
+				if (IsInChildrenUpdateTransaction) {
+					return childrenBuilder;
 				}
-				if (childrenList == null)
-					return TreeNodeCollection.Empty;
-				if (children == null)
-					children = new TreeNodeCollection (childrenList);
-				return children;
+
+				if (!childrenFromExtensionsLoaded) {
+					var transaction = BeginContextTransaction (out var disposeTransaction);
+					try {
+						if (!childrenFromExtensionsLoaded) {
+							if (extensionPoint != null) {
+								BeginChildrenUpdateTransaction (transaction);
+								Context.LoadExtensions (transaction, GetPath (), this);
+								// We have to keep the reference to the extension point, since add-ins may be loaded/unloaded
+							}
+						}
+					} finally {
+						if (disposeTransaction)
+							transaction.Dispose ();
+					}
+					childrenFromExtensionsLoaded = true;
+				}
+				return (IReadOnlyList<TreeNode>)childrenBuilder ?? (IReadOnlyList<TreeNode>)children;
 			}
 		}
-		public TreeNode Clone (AddinEngine engine)
+
+		IReadOnlyList<TreeNode> GetLoadedChildren ()
 		{
-			return Clone (engine, extensionPoint != null);
+			if (IsInChildrenUpdateTransaction)
+				return childrenBuilder;
+			else
+				return children;
 		}
 
-		TreeNode Clone (AddinEngine engine, bool cloneChildren)
+		/// <summary>
+		/// Returns true if the tree node has a child update transaction in progress,
+		/// and the current thread is the one that created the transaction.
+		/// </summary>
+		bool IsInChildrenUpdateTransaction => childrenBuilder != null && Context.IsCurrentThreadInTransaction;
+
+		ExtensionContextTransaction BeginContextTransaction (out bool dispose)
 		{
-			TreeNode newNode = new TreeNode (engine, Id);
-
-			// Copy extension data from the parent context node
-			newNode.ExtensionNodeSet = ExtensionNodeSet;
-			newNode.ExtensionPoint = ExtensionPoint;
-			newNode.Condition = Condition;
-
-			// If the node is an extension point, copy all the children to avoid
-			// having to load them from extensions again
-			if (cloneChildren && ChildrenLoaded) {
-				var builder = ImmutableArray.CreateBuilder<TreeNode> ();
-				foreach (var node in children) {
-					builder.Add (node.Clone (engine, true));
-				}
-				childrenLoaded = true;
-				newNode.children = builder.ToImmutableArray ();
+			if (IsInChildrenUpdateTransaction) {
+				dispose = false;
+				return buildTransaction;
+			} else {
+				dispose = true;
+				return Context.BeginTransaction ();
 			}
-			return newNode;
+		}
+
+		public void BeginChildrenUpdateTransaction (ExtensionContextTransaction transaction)
+		{
+			// If a transaction already started, just reuse it
+			if (buildTransaction != null)
+				return;
+
+			childrenBuilder = children.ToBuilder ();
+			this.buildTransaction = transaction;
+
+			transaction.RegisterChildrenUpdateTransaction (this);
+		}
+
+		internal void CommitChildrenUpdateTransaction ()
+		{
+			if (buildTransaction == null)
+				throw new InvalidOperationException ("No transaction started");
+
+			children = childrenBuilder.ToImmutable ();
+
+			var transaction = buildTransaction;
+			
+			childrenBuilder = null;
+			buildTransaction = null;
+
+			transaction.ReportChildrenChanged (this);
 		}
 
 		public string GetPath ()
@@ -298,7 +423,7 @@ namespace Mono.Addins
 		{
 			if (extensionNode != null && extensionNode.AddinId == ad.Addin.Id)
 				extensionNode.OnAddinLoaded ();
-			if (recursive && childrenLoaded) {
+			if (recursive && childrenFromExtensionsLoaded) {
 				foreach (TreeNode node in Children)
 					node.NotifyAddinLoaded (ad, true);
 			}
@@ -313,10 +438,10 @@ namespace Mono.Addins
 			TreeNode curNode = this;
 
 			foreach (string part in parts) {
-				int i = curNode.Children.IndexOfNode (part);
-				if (i != -1) {
-					curNode = curNode.Children [i];
-					if (!curNode.ChildrenLoaded)
+				var node = curNode.GetChildNode (part);
+				if (node != null) {
+					curNode = node;
+					if (!curNode.ChildrenFromExtensionsLoaded)
 						return null;
 					if (curNode.ExtensionPoint != null)
 						return curNode.ExtensionPoint;
@@ -335,7 +460,7 @@ namespace Mono.Addins
 				id = null;
 			}
 
-			if (childrenLoaded) {
+			if (childrenFromExtensionsLoaded) {
 				// Deep-first search, to make sure children are removed before the parent.
 				foreach (TreeNode node in Children)
 					node.FindAddinNodes (id, nodes);
@@ -375,16 +500,6 @@ namespace Mono.Addins
 			return false;
 		}
 		
-		public void Remove (ExtensionContextTransaction transaction)
-		{
-			if (parent != null) {
-				if (Condition != null)
-					transaction.UnregisterNodeCondition (this, Condition);
-				parent.children = parent.children.Remove(this);
-				transaction.ReportChildrenChanged(parent);
-			}
-		}
-		
 		public bool NotifyChildrenChanged ()
 		{
 			if (extensionNode != null)
@@ -395,17 +510,19 @@ namespace Mono.Addins
 
 		public void ResetCachedData (ExtensionContextTransaction transaction)
 		{
-			// Check IsInitialized since ResetCachedData is called while shutting down
-			if (extensionPoint != null && addinEngine.IsInitialized) {
+			if (extensionPoint != null) {
 				string aid = Addin.GetIdName (extensionPoint.ParentAddinDescription.AddinId);
 				RuntimeAddin ad = addinEngine.GetAddin (aid);
 				if (ad != null)
 					extensionPoint = ad.Addin.Description.ExtensionPoints [GetPath ()];
 			}
-			if (childrenLoaded) {
-				foreach (TreeNode cn in children)
+			if (childrenBuilder != null) {
+				foreach (TreeNode cn in childrenBuilder)
 					cn.ResetCachedData (transaction);
 			}
+
+			foreach (TreeNode cn in children)
+				cn.ResetCachedData (transaction);
 		}
 	}
 }
diff --git a/Mono.Addins/Mono.Addins/TreeNodeBuilder.cs b/Mono.Addins/Mono.Addins/TreeNodeBuilder.cs
deleted file mode 100644
index 29cf2f4..0000000
--- a/Mono.Addins/Mono.Addins/TreeNodeBuilder.cs
+++ /dev/null
@@ -1,221 +0,0 @@
-//
-// TreeNode.cs
-//
-// Author:
-//   Lluis Sanchez Gual
-//
-// Copyright (C) 2007 Novell, Inc (http://www.novell.com)
-//
-// Permission is hereby granted, free of charge, to any person obtaining
-// a copy of this software and associated documentation files (the
-// "Software"), to deal in the Software without restriction, including
-// without limitation the rights to use, copy, modify, merge, publish,
-// distribute, sublicense, and/or sell copies of the Software, and to
-// permit persons to whom the Software is furnished to do so, subject to
-// the following conditions:
-// 
-// The above copyright notice and this permission notice shall be
-// included in all copies or substantial portions of the Software.
-// 
-// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
-// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
-// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
-// NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
-// LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
-// OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
-// WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
-//
-
-
-using System;
-using Mono.Addins.Description;
-using System.Collections.Generic;
-using System.Collections.Immutable;
-using System.Linq;
-
-namespace Mono.Addins
-{
-    class TreeNodeBuilder
-	{
-		AddinEngine addinEngine;
-        string id;
-		List<TreeNodeBuilder> children;
-		ExtensionNode extensionNode;
-		string path;
-		TreeNode existingNode;
-		TreeNode builtNode;
-		TreeNodeBuilder parentNode;
-		bool built;
-
-		private TreeNodeBuilder ()
-		{
-		}
-
-		private TreeNodeBuilder (TreeNode existingNode)
-		{
-			this.existingNode = builtNode = existingNode;
-			this.id = existingNode.Id;
-			ExtensionNodeSet = existingNode.ExtensionNodeSet;
-		}
-
-		public static TreeNodeBuilder FromNode (TreeNode existingNode)
-		{
-			return new TreeNodeBuilder (existingNode);
-		}
-
-		public static TreeNodeBuilder CreateNew (AddinEngine addinEngine, string id, TreeNodeBuilder parentNode)
-		{
-			return new TreeNodeBuilder {
-				addinEngine = addinEngine,
-				id = id,
-				Parent = parentNode,
-				builtNode = new TreeNode (addinEngine, id, parentNode.builtNode)
-			};
-		}
-
-		public string Id => id;
-
-		public ExtensionPoint GetExtensionPoint ()
-		{
-			if (existingNode != null)
-				return existingNode.ExtensionPoint;
-			if (Parent != null)
-				return Parent.GetExtensionPoint ();
-			return null;
-		}
-
-		public TreeNodeBuilder Parent { get; private set; }
-
-		public BaseCondition Condition { get; set; }
-
-        public ExtensionNodeSet ExtensionNodeSet { get; set; }
-
-		void LoadChildren ()
-		{
-			if (children == null) {
-				children = new List<TreeNodeBuilder> ();
-				builtNode.LoadChildrenIntoBuilder (this);
-			}
-		}
-
-        public void AddChild (TreeNodeBuilder childBuilder)
-		{
-			LoadChildren ();
-
-			// The parent is specified in CreateNew
-			if (childBuilder.existingNode == null && childBuilder.Parent != this)
-				throw new InvalidOperationException ();
-
-			children.Add (childBuilder);
-		}
-
-		public void InsertChild (int curPos, TreeNodeBuilder childBuilder)
-		{
-			LoadChildren ();
-
-			// The parent is specified in CreateNew
-			if (childBuilder.Parent != this)
-				throw new InvalidOperationException ();
-
-			children.Insert (curPos, childBuilder);
-		}
-
-		public int IndexOfChild (string id)
-		{
-			LoadChildren ();
-			for (int n = 0; n < children.Count; n++) {
-				if (children[n].Id == id)
-					return n;
-			}
-			return -1;
-		}
-
-		public int ChildrenCount {
-			get {
-				LoadChildren ();
-				return children.Count;
-			}
-		}
-
-		public TreeNode Build (ExtensionContextTransaction transaction)
-		{
-			if (built)
-				return builtNode;
-
-			built = true;
-
-			var childNodes = children != null ? ImmutableArray.CreateRange (children.Select (builder => builder.Build (transaction))) : ImmutableArray<TreeNode>.Empty;
-
-			if (existingNode != null) {
-				if (children != null)
-					existingNode.SetChildren (transaction, childNodes);
-				return existingNode;
-			} else {
-				if (Condition != null)
-					builtNode.Condition = Condition;
-				if (ExtensionNodeSet != null)
-					builtNode.ExtensionNodeSet = ExtensionNodeSet;
-				if (extensionNode != null)
-					builtNode.AttachExtensionNode (extensionNode);
-
-				if (children != null)
-					builtNode.SetChildren (transaction, childNodes);
-
-				if (builtNode.Condition != null)
-					transaction.RegisterNodeCondition (builtNode, builtNode.Condition);
-
-				transaction.ReportLoadedNode (builtNode);
-				return builtNode;
-			}
-		}
-
-		internal void AttachExtensionNode (ExtensionNode node)
-        {
-			extensionNode = node;
-        }
-
-        internal string GetPath ()
-        {
-			if (path == null) {
-				if (existingNode != null)
-					path = existingNode.GetPath ();
-				else {
-					if (Parent != null)
-						path = Parent.GetPath () + "/" + id;
-					else
-						throw new InvalidOperationException (); // Should not happen
-				}
-			}
-			return path;
-        }
-
-		public TreeNodeBuilder GetNode (string path)
-		{
-			var thisPath = GetPath ();
-
-			if (path == thisPath)
-				return this;
-
-			if (!path.StartsWith (thisPath))
-				throw new InvalidOperationException ("Invalid extension path. Should not happen!");
-
-			var childPath = path.Substring (thisPath.Length);
-			string[] parts = childPath.Split ('/');
-
-			var node = this;
-
-			foreach (var part in parts) {
-				if (string.IsNullOrEmpty (part))
-					continue;
-				int i = node.IndexOfChild (part);
-				if (i == -1) {
-					var child = TreeNodeBuilder.CreateNew (addinEngine, part, node);
-					node.AddChild (child);
-					node = child;
-				} else
-					node = node.children [i];
-			}
-			return node;
-		}
-    }
-}