[
https://issues.apache.org/jira/browse/VELOCITY-536?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12486059
]
Christopher Schultz commented on VELOCITY-536:
----------------------------------------------
Regarding double-checked locking (DCL), it is typically used in conjunction
with a reference type where many things can go wrong.
In this case, the DCL is being performed on a 32-bit primitive type, which is
guaranteed by the VM to be assigned atomically. Since there's no object
initialization to perform (as there would be if you do something like "foo =
new FooType()"), there are no set-but-not-initialized issues, here.
The only potential problem is that the VM or JIT could re-order instructions
within the synchronized block and end up setting init=true before the
constructor has completed and/or the reference has been set. So, although
32-bit primitives will work with DCL, the fact that we are using a 32-bit
primitive to protect a reference complicates things.
The question to ask ourselves is "how bad is the contention for
synchronization"? Is this a method that will be called a lot? How any threads
should be expected to call this method? Is it only an issue during the initial
parsing, etc. of the template? Or, will it be called many times leading to a
severe performance degradation? Uncontested locks are pretty fast these days in
Java, so it might not be worth the risk.
Another option would be to modify the architecture of the code such that we
minimize the use of this method such that the synchronization has less of an
impact in a steady-state scenario. It's fine if the initialization code for a
template or macro has some slowdown, but repeated processing should avoid this
if possible.
My recommendation would be to remove the use of the DCL idiom, and remove
"volatile" from the declaration of "init", since it can't be relied upon. It
looks like it's just in there to support the DCL in the first place.
More information on DCL can be found here:
http://www.javaworld.com/javaworld/jw-02-2001/jw-0209-double.html
and here:
http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html
> Velocity Engine throws NullPointer Exception when two people click on the
> same page at the same time for the first time
> -----------------------------------------------------------------------------------------------------------------------
>
> Key: VELOCITY-536
> URL: https://issues.apache.org/jira/browse/VELOCITY-536
> Project: Velocity
> Issue Type: Bug
> Components: Engine
> Affects Versions: 1.5
> Reporter: Lei Gu
> Attachments: 536-patch.txt, ASTDirective.java, ASTSetDirective.java,
> VelocimacroProxy.java
>
>
> Multi-thread concurrency issue
> During our concurrency testing, we observed NullPointer exceptions being
> thrown when two people hit the same page at the same time for the first time.
> Upon further investigation, it turns out that we need to synchronize the init
> method on ASTDirective, ASTSetDirective, and render method on
> ASTSetDirective, and VelocimacroProxy.
> Basically, the problem is introduced as the following; when two threads
> attempts to parse and render the same template at the same time. Thread1
> finishes parsing first and proceeds to the render method call, while thread 2
> is still busy parsing and will overwrite the existing parse tree that is
> being used by thread 1 for rendering purpose. Thus under certainly condition
> a NullPointer exception will be thrown.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
