velocity-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From by...@apache.org
Subject svn commit: r736027 - in /velocity/engine/trunk: experimental/benchmark/ src/java/org/apache/velocity/runtime/ src/java/org/apache/velocity/runtime/directive/ src/java/org/apache/velocity/runtime/parser/ src/parser/ src/test/org/apache/velocity/test/
Date Tue, 20 Jan 2009 14:19:55 GMT
Author: byron
Date: Tue Jan 20 06:19:54 2009
New Revision: 736027

URL: http://svn.apache.org/viewvc?rev=736027&view=rev
Log:
VELOCITY-669 Fix multiple thread race condition for macro initialization

Modified:
    velocity/engine/trunk/experimental/benchmark/Benchmark.java
    velocity/engine/trunk/experimental/benchmark/main.vm
    velocity/engine/trunk/experimental/benchmark/run.sh
    velocity/engine/trunk/src/java/org/apache/velocity/runtime/VelocimacroManager.java
    velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/Macro.java
    velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/RuntimeMacro.java
    velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/Parser.java
    velocity/engine/trunk/src/parser/Parser.jjt
    velocity/engine/trunk/src/test/org/apache/velocity/test/ParseExceptionTestCase.java

Modified: velocity/engine/trunk/experimental/benchmark/Benchmark.java
URL: http://svn.apache.org/viewvc/velocity/engine/trunk/experimental/benchmark/Benchmark.java?rev=736027&r1=736026&r2=736027&view=diff
==============================================================================
--- velocity/engine/trunk/experimental/benchmark/Benchmark.java (original)
+++ velocity/engine/trunk/experimental/benchmark/Benchmark.java Tue Jan 20 06:19:54 2009
@@ -33,8 +33,8 @@
 
 public class Benchmark
 {
-  int threadCnt = 25;
-  int runCnt = 1000;
+  int threadCnt = 10;
+  int runCnt = 500;
   
   public static final void main(String[] argv) throws Exception
   {
@@ -55,9 +55,9 @@
     props.setProperty(RuntimeConstants.VM_LIBRARY, "vmlib1.vm,vmlib2.vm");
     props.setProperty(RuntimeConstants.RESOURCE_MANAGER_DEFAULTCACHE_SIZE, "20");
     props.setProperty(RuntimeConstants.RUNTIME_REFERENCES_STRICT, "true");
-           
-    VelocityEngine vengine = new VelocityEngine();
-    vengine.init(props);
+    VelocityEngine vengine = new VelocityEngine(props);
+    vengine.init();
+    System.out.println("blaa:  " + vengine.getProperty(RuntimeConstants.RUNTIME_REFERENCES_STRICT));
 
     log("Starting " + threadCnt + " threads which will run " + runCnt + " times");
     ArrayList list = new ArrayList(threadCnt);    
@@ -164,65 +164,23 @@
 
 class FastWriter extends Writer
 {
+  char[] buffer = new char[20000];
 
-  @Override
-  public Writer append(char c) throws IOException
-  {
-    throw new AssertionError("Bad method call");
-  }
-
-  @Override
-  public Writer append(CharSequence csq, int start, int end)
-      throws IOException
-  {
-    throw new AssertionError("Bad method call");
-  }
-
-  @Override
-  public Writer append(CharSequence csq) throws IOException
-  {
-    throw new AssertionError("Bad method call");
-  }
-
-  @Override
-  public void close() throws IOException
-  {
-    throw new AssertionError("Bad method call");
-  }
-
-  @Override
-  public void flush() throws IOException
-  {
-    throw new AssertionError("Bad method call");
-  }
-
-  @Override
-  public void write(char[] cbuf, int off, int len) throws IOException
-  {
-    throw new AssertionError("Bad method call");
-  }
-
-  @Override
-  public void write(char[] cbuf) throws IOException
-  {
-  }
-
-  @Override
-  public void write(int c) throws IOException
+  public FastWriter()
   {
-    throw new AssertionError("Bad method call");
+    super();
   }
 
-  @Override
-  public void write(String str, int off, int len) throws IOException
+  public void write(char[] buf, int off, int len)
   {
-    throw new AssertionError("Bad method call");
-  }
+    for (int i=0; i < len; i++)
+    {
+      buffer[i] = buf[i+off];
+    }    
+  }  
 
-  @Override
-  public void write(String str) throws IOException
-  {
-  }
+  public void close() {}
+  public void flush() {}
   
 }
 

Modified: velocity/engine/trunk/experimental/benchmark/main.vm
URL: http://svn.apache.org/viewvc/velocity/engine/trunk/experimental/benchmark/main.vm?rev=736027&r1=736026&r2=736027&view=diff
==============================================================================
--- velocity/engine/trunk/experimental/benchmark/main.vm (original)
+++ velocity/engine/trunk/experimental/benchmark/main.vm Tue Jan 20 06:19:54 2009
@@ -6,6 +6,8 @@
 These are some lines of text 1
 These are some lines of text 1
 
+#macro(stuck)this is text#end
+
 #foreach($i in $outerList) ## iterate 10 times
 
   #set($tests = [$test1, $test2, $test3, $test4, $test5])
@@ -16,6 +18,8 @@
 
   #foreach($j in $innerList) ## iterate 10 times
 
+    #stuck()
+
     #vm_macro1($test4 $test5)
     #vm_macro2($test8 $test9)
 

Modified: velocity/engine/trunk/experimental/benchmark/run.sh
URL: http://svn.apache.org/viewvc/velocity/engine/trunk/experimental/benchmark/run.sh?rev=736027&r1=736026&r2=736027&view=diff
==============================================================================
--- velocity/engine/trunk/experimental/benchmark/run.sh (original)
+++ velocity/engine/trunk/experimental/benchmark/run.sh Tue Jan 20 06:19:54 2009
@@ -36,7 +36,7 @@
   JRAT_SWITCH=-javaagent:shiftone-jrat.jar
 fi
 
-CP=$COMMON_LANG:$COMMON_COLL:$VELOCITY_PATH:.
+CP=.:$COMMON_LANG:$COMMON_COLL:$VELOCITY_PATH
 
 COMPILE="javac -cp $CP Benchmark.java"
 

Modified: velocity/engine/trunk/src/java/org/apache/velocity/runtime/VelocimacroManager.java
URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/runtime/VelocimacroManager.java?rev=736027&r1=736026&r2=736027&view=diff
==============================================================================
--- velocity/engine/trunk/src/java/org/apache/velocity/runtime/VelocimacroManager.java (original)
+++ velocity/engine/trunk/src/java/org/apache/velocity/runtime/VelocimacroManager.java Tue
Jan 20 06:19:54 2009
@@ -23,6 +23,8 @@
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
+
+import org.apache.velocity.exception.VelocityException;
 import org.apache.velocity.runtime.directive.VelocimacroProxy;
 import org.apache.velocity.runtime.parser.node.Node;
 import org.apache.velocity.runtime.parser.node.SimpleNode;
@@ -100,7 +102,7 @@
         {
             // happens only if someone uses this class without the Macro directive
             // and provides a null value as an argument
-            throw new RuntimeException("Null AST for "+vmName+" in "+namespace);
+            throw new VelocityException("Null AST for "+vmName+" in "+namespace);
         }
 
         MacroEntry me = new MacroEntry(vmName, macroBody, argArray, namespace, rsvc);

Modified: velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/Macro.java
URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/Macro.java?rev=736027&r1=736026&r2=736027&view=diff
==============================================================================
--- velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/Macro.java (original)
+++ velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/Macro.java Tue Jan
20 06:19:54 2009
@@ -99,30 +99,17 @@
     {
         super.init(rs, context, node);
 
-        /*
-         * again, don't do squat.  We want the AST of the macro
-         * block to hang off of this but we don't want to
-         * init it... it's useless...
-         */
+        
+        // Add this macro to the VelocimacroManager now that it has been initialized.   
    
+        String argArray[] = getArgArray(node, rs);
+        int numArgs = node.jjtGetNumChildren();
+        rs.addVelocimacro(argArray[0], node.jjtGetChild(numArgs - 1), argArray, node.getTemplateName());
     }
-
+    
     /**
-     *  Used by Parser.java to process VMs during the parsing process.
-     *
-     *  This method does not render the macro to the output stream,
-     *  but rather <i>processes the macro body</i> into the internal
-     *  representation used by {#link
-     *  org.apache.velocity.runtime.directive.VelocimacroProxy}
-     *  objects, and if not currently used, adds it to the macro
-     *  Factory.
-     * @param rs
-     * @param t
-     * @param node
-     * @param sourceTemplate
-     * @throws IOException
-     * @throws ParseException
+     *  Used by Parser.java to do further parameter checking for macro arguments.
      */
-    public static void processAndRegister(RuntimeServices rs,  Token t, Node node,
+    public static void checkArgs(RuntimeServices rs,  Token t, Node node,
                                           String sourceTemplate)
         throws IOException, ParseException
     {
@@ -131,14 +118,12 @@
          *  the name of the VM.  Note that 0 following
          *  args is ok for naming blocks of HTML
          */
-
         int numArgs = node.jjtGetNumChildren();
 
         /*
          *  this number is the # of args + 1.  The + 1
          *  is for the block tree
          */
-
         if (numArgs < 2)
         {
 
@@ -146,7 +131,6 @@
              *  error - they didn't name the macro or
              *  define a block
              */
-
             rs.getLog().error("#macro error : Velocimacro must have name as 1st " +
                               "argument to #macro(). #args = " + numArgs);
 
@@ -157,34 +141,16 @@
         /*
          *  lets make sure that the first arg is an ASTWord
          */
-
         int firstType = node.jjtGetChild(0).getType();
-
         if(firstType != ParserTreeConstants.JJTWORD)
         {
             throw new MacroParseException("First argument to #macro() must be a"
                     + " token without surrounding \' or \", which specifies"
                     + " the macro name.  Currently it is a "
                     + ParserTreeConstants.jjtNodeName[firstType], sourceTemplate, t);
-        }
-
-        // get the arguments to the use of the VM - element 0 contains the macro name
-        String argArray[] = getArgArray(node, rs);
-
-        /* 
-         * we already have the macro parsed as AST so there is no point to
-         * transform it into a String again
-         */ 
-        rs.addVelocimacro(argArray[0], node.jjtGetChild(numArgs - 1), argArray, sourceTemplate);
-        
-        /*
-         * Even if the add attempt failed, we don't log anything here.
-         * Logging must be done at VelocimacroFactory or VelocimacroManager because
-         * those classes know the real reason.
-         */ 
+        }                
     }
 
-
     /**
      * Creates an array containing the literal text from the macro
      * arguement(s) (including the macro's name as the first arg).

Modified: velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/RuntimeMacro.java
URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/RuntimeMacro.java?rev=736027&r1=736026&r2=736027&view=diff
==============================================================================
--- velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/RuntimeMacro.java
(original)
+++ velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/RuntimeMacro.java
Tue Jan 20 06:19:54 2009
@@ -81,7 +81,7 @@
      * template stored for later use.
      *
      * @param macroName name of the macro
-\     */
+     */
     public RuntimeMacro(String macroName)
     {
         if (macroName == null)
@@ -287,7 +287,7 @@
             if (badArgsErrorMsg != null)
             {
                 throw new TemplateInitException(badArgsErrorMsg,
-                  context.getCurrentTemplateName(), 0, 0);
+                  context.getCurrentTemplateName(), node.getColumn(), node.getLine());
             }
 
             try

Modified: velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/Parser.java
URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/Parser.java?rev=736027&r1=736026&r2=736027&view=diff
==============================================================================
--- velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/Parser.java (original)
+++ velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/Parser.java Tue Jan
20 06:19:54 2009
@@ -33,6 +33,11 @@
     private Map directives = new HashMap();
 
     /**
+     * Keep track of defined macros, used for escape processing
+     */
+    private Map macroNames = new HashMap();
+
+    /**
      *  Name of current template we are parsing.  Passed to us in parse()
      */
     public String currentTemplateName = "";
@@ -189,7 +194,7 @@
         {
            bRecognizedDirective = true;
         }
-        else if ( rsvc.isVelocimacro(dirTag, currentTemplateName))
+        else if (macroNames.containsKey(dirTag))
         {
             bRecognizedDirective = true;
         }
@@ -949,7 +954,13 @@
 
         if (doItNow)
         {
-            Macro.processAndRegister(rsvc, t, jjtn000, currentTemplateName);
+            // Further checking of macro arguments
+            Macro.checkArgs(rsvc, t, jjtn000, currentTemplateName);
+
+            // Add the macro name so that we can peform escape processing
+            // on defined macros
+            String macroName = jjtn000.jjtGetChild(0).getFirstToken().image;
+            macroNames.put(macroName, macroName);
         }
 
         /*
@@ -2758,26 +2769,6 @@
     finally { jj_save(11, xla); }
   }
 
-  final private boolean jj_3R_64() {
-    if (jj_scan_token(WORD)) return true;
-    return false;
-  }
-
-  final private boolean jj_3_10() {
-    if (jj_3R_33()) return true;
-    return false;
-  }
-
-  final private boolean jj_3R_60() {
-    if (jj_scan_token(IDENTIFIER)) return true;
-    return false;
-  }
-
-  final private boolean jj_3R_31() {
-    if (jj_3R_40()) return true;
-    return false;
-  }
-
   final private boolean jj_3_8() {
     if (jj_3R_33()) return true;
     return false;
@@ -2788,13 +2779,8 @@
     return false;
   }
 
-  final private boolean jj_3_4() {
-    Token xsp;
-    xsp = jj_scanpos;
-    if (jj_scan_token(31)) jj_scanpos = xsp;
-    xsp = jj_scanpos;
-    if (jj_3R_27()) jj_scanpos = xsp;
-    if (jj_3R_28()) return true;
+  final private boolean jj_3R_60() {
+    if (jj_scan_token(IDENTIFIER)) return true;
     return false;
   }
 
@@ -2825,6 +2811,16 @@
     return false;
   }
 
+  final private boolean jj_3_4() {
+    Token xsp;
+    xsp = jj_scanpos;
+    if (jj_scan_token(31)) jj_scanpos = xsp;
+    xsp = jj_scanpos;
+    if (jj_3R_27()) jj_scanpos = xsp;
+    if (jj_3R_28()) return true;
+    return false;
+  }
+
   final private boolean jj_3R_63() {
     if (jj_3R_73()) return true;
     return false;
@@ -2845,11 +2841,6 @@
     return false;
   }
 
-  final private boolean jj_3R_65() {
-    if (jj_scan_token(STRING_LITERAL)) return true;
-    return false;
-  }
-
   final private boolean jj_3R_70() {
     if (jj_scan_token(TRUE)) return true;
     return false;
@@ -2890,13 +2881,13 @@
     return false;
   }
 
-  final private boolean jj_3R_40() {
-    if (jj_scan_token(INTEGER_LITERAL)) return true;
+  final private boolean jj_3R_82() {
+    if (jj_3R_66()) return true;
     return false;
   }
 
-  final private boolean jj_3R_82() {
-    if (jj_3R_66()) return true;
+  final private boolean jj_3R_65() {
+    if (jj_scan_token(STRING_LITERAL)) return true;
     return false;
   }
 
@@ -2956,8 +2947,8 @@
     return false;
   }
 
-  final private boolean jj_3R_67() {
-    if (jj_scan_token(FLOATING_POINT_LITERAL)) return true;
+  final private boolean jj_3R_40() {
+    if (jj_scan_token(INTEGER_LITERAL)) return true;
     return false;
   }
 
@@ -3011,6 +3002,11 @@
     return false;
   }
 
+  final private boolean jj_3R_67() {
+    if (jj_scan_token(FLOATING_POINT_LITERAL)) return true;
+    return false;
+  }
+
   final private boolean jj_3R_24() {
     Token xsp;
     xsp = jj_scanpos;
@@ -3176,11 +3172,6 @@
     return false;
   }
 
-  final private boolean jj_3_2() {
-    if (jj_scan_token(DOUBLE_ESCAPE)) return true;
-    return false;
-  }
-
   final private boolean jj_3R_94() {
     if (jj_3R_70()) return true;
     return false;
@@ -3201,6 +3192,11 @@
     return false;
   }
 
+  final private boolean jj_3_2() {
+    if (jj_scan_token(DOUBLE_ESCAPE)) return true;
+    return false;
+  }
+
   final private boolean jj_3R_76() {
     if (jj_3R_40()) return true;
     return false;
@@ -3295,11 +3291,6 @@
     return false;
   }
 
-  final private boolean jj_3R_25() {
-    if (jj_3R_24()) return true;
-    return false;
-  }
-
   final private boolean jj_3R_77() {
     Token xsp;
     xsp = jj_scanpos;
@@ -3319,7 +3310,7 @@
     return false;
   }
 
-  final private boolean jj_3_1() {
+  final private boolean jj_3R_25() {
     if (jj_3R_24()) return true;
     return false;
   }
@@ -3340,11 +3331,21 @@
     return false;
   }
 
+  final private boolean jj_3_1() {
+    if (jj_3R_24()) return true;
+    return false;
+  }
+
   final private boolean jj_3R_50() {
     if (jj_3R_71()) return true;
     return false;
   }
 
+  final private boolean jj_3R_90() {
+    if (jj_3R_73()) return true;
+    return false;
+  }
+
   final private boolean jj_3R_49() {
     if (jj_3R_70()) return true;
     return false;
@@ -3371,18 +3372,18 @@
     return false;
   }
 
-  final private boolean jj_3R_90() {
+  final private boolean jj_3R_89() {
     if (jj_3R_73()) return true;
     return false;
   }
 
-  final private boolean jj_3R_47() {
-    if (jj_3R_68()) return true;
+  final private boolean jj_3R_37() {
+    if (jj_3R_40()) return true;
     return false;
   }
 
-  final private boolean jj_3R_89() {
-    if (jj_3R_73()) return true;
+  final private boolean jj_3R_47() {
+    if (jj_3R_68()) return true;
     return false;
   }
 
@@ -3391,11 +3392,6 @@
     return false;
   }
 
-  final private boolean jj_3R_37() {
-    if (jj_3R_40()) return true;
-    return false;
-  }
-
   final private boolean jj_3R_45() {
     if (jj_3R_66()) return true;
     return false;
@@ -3406,6 +3402,16 @@
     return false;
   }
 
+  final private boolean jj_3R_36() {
+    if (jj_3R_24()) return true;
+    return false;
+  }
+
+  final private boolean jj_3R_32() {
+    if (jj_3R_60()) return true;
+    return false;
+  }
+
   final private boolean jj_3R_44() {
     if (jj_3R_40()) return true;
     return false;
@@ -3419,21 +3425,11 @@
     return false;
   }
 
-  final private boolean jj_3R_36() {
-    if (jj_3R_24()) return true;
-    return false;
-  }
-
   final private boolean jj_3R_43() {
     if (jj_3R_65()) return true;
     return false;
   }
 
-  final private boolean jj_3R_32() {
-    if (jj_3R_60()) return true;
-    return false;
-  }
-
   final private boolean jj_3R_42() {
     if (jj_3R_64()) return true;
     return false;
@@ -3478,6 +3474,21 @@
     return false;
   }
 
+  final private boolean jj_3_10() {
+    if (jj_3R_33()) return true;
+    return false;
+  }
+
+  final private boolean jj_3R_64() {
+    if (jj_scan_token(WORD)) return true;
+    return false;
+  }
+
+  final private boolean jj_3R_31() {
+    if (jj_3R_40()) return true;
+    return false;
+  }
+
   public ParserTokenManager token_source;
   public Token token, jj_nt;
   private int jj_ntk;

Modified: velocity/engine/trunk/src/parser/Parser.jjt
URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/parser/Parser.jjt?rev=736027&r1=736026&r2=736027&view=diff
==============================================================================
--- velocity/engine/trunk/src/parser/Parser.jjt (original)
+++ velocity/engine/trunk/src/parser/Parser.jjt Tue Jan 20 06:19:54 2009
@@ -108,6 +108,11 @@
      *  This Map contains a list of all of the dynamic directives.
      */
     private Map directives = new HashMap();
+    
+    /**
+     * Keep track of defined macros, used for escape processing
+     */
+    private Map macroNames = new HashMap();
 
     /**
      *  Name of current template we are parsing.  Passed to us in parse()
@@ -266,7 +271,7 @@
         {
            bRecognizedDirective = true;
         }
-        else if ( rsvc.isVelocimacro(dirTag, currentTemplateName))
+        else if (macroNames.containsKey(dirTag))
         {
             bRecognizedDirective = true;
         }
@@ -1585,7 +1590,13 @@
 
         if (doItNow)
         {
-            Macro.processAndRegister(rsvc, t, jjtThis, currentTemplateName);
+            // Further checking of macro arguments
+            Macro.checkArgs(rsvc, t, jjtThis, currentTemplateName);
+            
+            // Add the macro name so that we can peform escape processing
+            // on defined macros
+            String macroName = jjtThis.jjtGetChild(0).getFirstToken().image;
+            macroNames.put(macroName, macroName);
         }
 
         /*

Modified: velocity/engine/trunk/src/test/org/apache/velocity/test/ParseExceptionTestCase.java
URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/test/org/apache/velocity/test/ParseExceptionTestCase.java?rev=736027&r1=736026&r2=736027&view=diff
==============================================================================
--- velocity/engine/trunk/src/test/org/apache/velocity/test/ParseExceptionTestCase.java (original)
+++ velocity/engine/trunk/src/test/org/apache/velocity/test/ParseExceptionTestCase.java Tue
Jan 20 06:19:54 2009
@@ -230,11 +230,11 @@
             ve.evaluate(context,writer,"testMacroInvoke", "#macro(   foo $a) $a #end #foo(woogie)");
             fail("Should have thown a ParseErrorException");
         }
-        catch (ParseErrorException e)
+        catch (org.apache.velocity.exception.TemplateInitException e)
         {
             assertEquals("testMacroInvoke",e.getTemplateName());
             assertEquals(1,e.getLineNumber());
-            assertEquals(31,e.getColumnNumber());
+            assertEquals(27,e.getColumnNumber());
         }
         finally
         {



Mime
View raw message