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
{
