From 86c3e6c0ac2cff97bc3f110afa4e5cb1467db862 Mon Sep 17 00:00:00 2001 From: Lluis Sanchez Date: Wed, 15 Jun 2022 19:20:14 +0200 Subject: 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. --- .../Mono.Addins.Database/DatabaseConfiguration.cs | 9 +- Mono.Addins/Mono.Addins/AddinEngine.cs | 2 +- Mono.Addins/Mono.Addins/ExtensionContext.cs | 167 ++++++------ .../Mono.Addins/ExtensionContextTransaction.cs | 17 ++ Mono.Addins/Mono.Addins/ExtensionNode.cs | 6 +- Mono.Addins/Mono.Addins/ExtensionTree.cs | 37 +-- Mono.Addins/Mono.Addins/TreeNode.cs | 285 +++++++++++++++------ Mono.Addins/Mono.Addins/TreeNodeBuilder.cs | 221 ---------------- Test/FileExtender/FileExtender.csproj | 2 + 9 files changed, 346 insertions(+), 400 deletions(-) delete mode 100644 Mono.Addins/Mono.Addins/TreeNodeBuilder.cs 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.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> 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 (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 (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 (); + 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 (); @@ -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 unregisteredAssemblyResolvePaths; List addinLoadEvents; List addinUnloadEvents; + List 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 (); addinUnloadEvents.Add (id); } + + public void RegisterChildrenUpdateTransaction (TreeNode node) + { + if (treeNodeTransactions == null) + treeNodeTransactions = new List (); + 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 list = new List (nodeChildren.Length); + List list = new List (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 addedNodes) + public void LoadExtension (ExtensionContextTransaction transaction, TreeNode tnode, string addin, Extension extension, List 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 addedNodes) + void LoadExtensionElement (ExtensionContextTransaction transaction, TreeNode parentNode, string addin, ExtensionNodeDescriptionCollection extension, ModuleDescription module, ref int curPos, BaseCondition parentCondition, bool inComplextCondition, List 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 children; + + ImmutableArray.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.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 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 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 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)childrenBuilder ?? (IReadOnlyList)children; } } - public TreeNode Clone (AddinEngine engine) + + IReadOnlyList GetLoadedChildren () { - return Clone (engine, extensionPoint != null); + if (IsInChildrenUpdateTransaction) + return childrenBuilder; + else + return children; } - TreeNode Clone (AddinEngine engine, bool cloneChildren) + /// + /// Returns true if the tree node has a child update transaction in progress, + /// and the current thread is the one that created the transaction. + /// + 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 (); - 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 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 (); - 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.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; - } - } -} diff --git a/Test/FileExtender/FileExtender.csproj b/Test/FileExtender/FileExtender.csproj index b2d6155..c40029f 100644 --- a/Test/FileExtender/FileExtender.csproj +++ b/Test/FileExtender/FileExtender.csproj @@ -11,6 +11,8 @@ 2.0 FileExtender 0.0.0.0 + $(DotNetCoreTarget) + false True -- cgit v1.2.3