Uploaded image for project: 'Jira Data Center'
  1. Jira Data Center
  2. JRASERVER-64200

Race condition in DefaultVelocityManager leads to unrendered template


      NOTE: This bug report is for JIRA Server. Using JIRA Cloud? See the corresponding bug report.

      Raising on behalf of Aron Gombas. Originall report is https://ecosystem.atlassian.net/browse/DEVHELP-376

       We are relying on the Velocity rendering facilities in JIRA. I guess a tons of other add-on does it, too, so this bug may affect many.
      For that, we let the framework to inject VelocityManager into our service tier. We received a DefaultVelocityManager implementation class, on which we invoke the following method:

      void writeEncodedBodyForContent(Writer writer, String contentFragment, Context context)

      It has been working perfectly for ~6 years, but recently (starting from JIRA 7.0 or 7.1) we randomly receive a rendered string that has unrendered Velocity macros. The #foreach() and #if() directives are processed, but the macro appear in their original form in the output.
      This is nearly impossible to reproduce manually, but it was reported multiple times by users running large JIRA instance.
      Our investigation found to this issue: https://issues.apache.org/jira/browse/VELOCITY-776
      We figured out that macros are cached to the one namespace used by multiple threads. If one of the thread decides to flush the cache, the macros are gone for the other threads, too. We wrote a Python script that triggers our rendering and the JIRA web interface simultaneously (also using Velocity), and the bug consistently appears.
      The root cause
      The writeEncodedBodyForContent() is using a single logTag (that acts as a sort of namespace for macros) for all threads!!!
      It may be useful for most, but leads to bugs for others.

      getVe().evaluate(context, writer, "getEncodedBodyFromContent", contentFragment);

      We should be able to pass our own logTag, to avoid namespace collisions.
      Our workaround
      We copied the implementation of this method, hacked a Reflection based method to retrieve the "ve" instance, and we are now using our own namespace.
      The problem disappeared.

            Unassigned Unassigned
            ssmith Steve Smith (Inactive)
            0 Vote for this issue
            1 Start watching this issue