Unicorn variables cannot be used in global sitecore 9.1.1 variables

[JaspervdM80]

Do you want to request a feature or report a bug? BUG

What is the current behavior? When starting Sitecore you receive a YSOD

A read lock may not be acquired with the write lock held in this mode.

Description: An unhandled exception occurred during the execution of the current web request. Please review the stack trace for more information about the error and where it originated in the code.

Exception Details: System.Threading.LockRecursionException: A read lock may not be acquired with the write lock held in this mode.

If the current behavior is a bug, please provide the steps to reproduce. Add a variable to a config with a reference to a unicorn variable, like: <sc.variable name="test" value="$(layer)" />

What is the expected behavior? That you do not receive a YSOD and that unicorn variables can be used in global variables.

Please mention your Sitecore version and Unicorn version. Sitecore 9.1.1, SXA 1.8.1 with Unicorn 4.1.0

[JaspervdM80]

In then namespace Sitecore.Configuration.ConfigReader within Sitecore.Kernel.dll there is a new function ResolveVariables which allows you to reference variables with variables that causes this issue.

[cassidydotdk]

I assume this only happens when assigning a Unicorn token replacer value to an sc.variable, right? as such it was never really intended to be used in that way.

What is your use case here?

[JaspervdM80]

Sorry for closing and opening the issue. But the change is generic, in previous versions form sitecore you could "use" variables which were not defined in other variable such as the unicorn variables.

We used the layer and the module variable in a placeholder variable for re-using purposes.

[cassidydotdk]

For internal notes:

From a 9.1.1 ConfigReader. Will compare this to previous releases.

    internal static string ReplaceVariables(string value, XmlNode node, string[] parameters)
    {
      Assert.ArgumentNotNull((object) value, nameof (value));
      Assert.ArgumentNotNull((object) node, nameof (node));
      for (node = node.ParentNode; node != null && node.NodeType == XmlNodeType.Element && value.IndexOf("$(", StringComparison.InvariantCulture) >= 0; node = node.ParentNode)
      {
        foreach (XmlAttribute attribute in (XmlNamedNodeMap) node.Attributes)
        {
          string oldValue = "$(" + attribute.LocalName + ")";
          value = value.Replace(oldValue, attribute.Value);
        }
        value = value.Replace("$(name)", node.LocalName);
      }
      if (parameters != null)
      {
        for (int index = 0; index < parameters.Length; ++index)
          value = value.Replace("$(" + (object) index + ")", parameters[index]);
      }
      return value;
    }

[cassidydotdk]

9.1.1 ResolveVariables

The intent of this seems to be, that it is a Warning condition if any variables are found that it could not resolve.

    protected internal virtual StringDictionary ResolveVariables(
      StringDictionary variables)
    {
      StringDictionary stringDictionary = new StringDictionary();
      StringDictionary resolvedVariables = new StringDictionary();
      foreach (KeyValuePair<string, string> variable in (SafeDictionary<string, string>) variables)
      {
        if (variable.Value.IndexOf("$(", StringComparison.Ordinal) < 0)
          resolvedVariables.Add(variable.Key, variable.Value);
        else
          stringDictionary.Add(variable.Key, variable.Value);
      }
      while (stringDictionary.Count > 0)
      {
        bool flag = true;
        foreach (string key in stringDictionary.Keys.ToList<string>())
        {
          string str = stringDictionary[key];
          string value = str;
          this.FindVariables(value).ForEach<string>((Action<string>) (v =>
          {
            if (!resolvedVariables.ContainsKey(v))
              return;
            value = value.Replace(v, resolvedVariables[v]);
          }));
          if (value != str)
          {
            flag = false;
            if (value.IndexOf("$(", StringComparison.Ordinal) < 0)
            {
              resolvedVariables.Add(key, value);
              stringDictionary.Remove(key);
            }
            else
              stringDictionary[key] = value;
          }
        }
        if (flag)
        {
          using (IEnumerator<KeyValuePair<string, string>> enumerator = stringDictionary.GetEnumerator())
          {
            while (enumerator.MoveNext())
            {
              KeyValuePair<string, string> current = enumerator.Current;
              Log.Warn("There is unresolved variable " + current.Key + " with " + current.Value + " value found in the config.", (object) this);
            }
            break;
          }
        }
      }
      resolvedVariables.AddRange((SafeDictionary<string, string>) stringDictionary);
      return resolvedVariables;
    }

[cassidydotdk]

9.0.x version:

    /// <summary>Replaces global variables.</summary>
    /// <param name="rootNode">The root node.</param>
    protected virtual void ReplaceGlobalVariables(XmlNode rootNode)
    {
      Assert.ArgumentNotNull((object) rootNode, nameof (rootNode));
      XmlNodeList xmlNodeList = rootNode.SelectNodes(".//sc.variable");
      StringDictionary variables = new StringDictionary();
      foreach (XmlAttribute attribute in (XmlNamedNodeMap) rootNode.Attributes)
      {
        string name = attribute.Name;
        string str = Sitecore.StringUtil.GetString(attribute.Value);
        if (name.Length > 0)
        {
          string index = "$(" + name + ")";
          variables[index] = str;
        }
      }
      for (int index1 = 0; index1 < xmlNodeList.Count; ++index1)
      {
        string attribute1 = Sitecore.Xml.XmlUtil.GetAttribute("name", xmlNodeList[index1]);
        string attribute2 = Sitecore.Xml.XmlUtil.GetAttribute("value", xmlNodeList[index1]);
        if (attribute1.Length > 0)
        {
          string index2 = "$(" + attribute1 + ")";
          variables[index2] = attribute2;
        }
      }
      if (variables.Count == 0)
        return;
      this.ReplaceGlobalVariables(rootNode, variables);
    }

    /// <summary>Replaces global variables.</summary>
    /// <param name="node">The root node.</param>
    /// <param name="variables">The variables.</param>
    protected virtual void ReplaceGlobalVariables(XmlNode node, StringDictionary variables)
    {
      Assert.ArgumentNotNull((object) node, nameof (node));
      Assert.ArgumentNotNull((object) variables, nameof (variables));
      foreach (XmlAttribute attribute in (XmlNamedNodeMap) node.Attributes)
      {
        string str = attribute.Value;
        if (str.IndexOf('$') >= 0)
        {
          foreach (string key in variables.Keys)
            str = str.Replace(key, variables[key]);
          attribute.Value = str;
        }
      }
      foreach (XmlNode childNode in node.ChildNodes)
      {
        if (childNode.NodeType == XmlNodeType.Element)
          this.ReplaceGlobalVariables(childNode, variables);
      }
    }

[cassidydotdk]

Looks to me like it's the intent of Sitecore, moving forward, to insist on resolving all of these global variables itself, and fail if it cannot. Even if the exception Exception Details: System.Threading.LockRecursionException: A read lock may not be acquired with the write lock held in this mode. seems to be a bug in their handling (on first glance) - the intent of the code is clear in that it will Log.Warn any variables it cannot resolve. Which is undesirable in any event.

I've raised this concern with Sitecore and it's possible some new options for variable handling will arise in the future. For now however, there is little to be done from the viewpoint of this here project.

Also, as mentioned, I'm not sure why you even WANT to go via Global Variables like this for the Helix Variable replacement feature. But that's a side note to this all.