You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@velocity.apache.org by "Lei Gu (JIRA)" <de...@velocity.apache.org> on 2007/04/02 16:43:32 UTC

[jira] Created: (VELOCITY-536) Velocity Engine throws NullPointer Exception when two people click on the same page at the same time for the first time

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


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: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


Re: [jira] Commented: (VELOCITY-536) Velocity Engine throws

Posted by "Henning P. Schmiedehausen" <hp...@intermeta.de>.
Lei Gu <Le...@authoria.com> writes:

Can you give me a reference for that. Anyway, we will keep 1.3
compatibility for the Velocity 1.5.x series and 1.4 compatibility for
the Velocity 1.x series, so citing Java 5 is not really helping the
case. ;-)

	Best regards
		Henning


>We are on JDK 1.5 and the memory model works for DCL if combined with
>volatile key word. In JDK 1.5, volatile key word guarantees two threads see
>the variable in the same order with the same value.
>Thanks.
>-- Lei
> 

>Velocity - Dev mailing list-2 wrote:
>> 
>> 
>>     [
>> https://issues.apache.org/jira/browse/VELOCITY-536?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12486071
>> ] 
>> 
>> Nathan Bubna commented on VELOCITY-536:
>> ---------------------------------------
>> 
>> But the DCL'ed block only needs to happen once per instance.  Isn't the
>> whole point to make the second thread skip the block completely?   We only
>> need one thread per instance to go through, period.   Once one thread has
>> started going through the block, it will finish and keep other threads out
>> of there for that instance, order notwithstanding.
>> 
>> And again, this is not a constructor nor a completely abstract use of DCL
>> that we're talking about.  This is a very specific use.
>> 
>>> 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: dev-unsubscribe@velocity.apache.org
>> For additional commands, e-mail: dev-help@velocity.apache.org
>> 
>> 
>> 

>-- 
>View this message in context: http://www.nabble.com/-jira--Created%3A-%28VELOCITY-536%29-Velocity-Engine-throws-NullPointer-Exception-when-two-people-click-on-the-same-page-at-the-same-time-for-the-first-time-tf3506177.html#a9796621
>Sent from the Velocity - Dev mailing list archive at Nabble.com.


>---------------------------------------------------------------------
>To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
>For additional commands, e-mail: dev-help@velocity.apache.org

-- 
Henning P. Schmiedehausen  -- hps@intermeta.de | J2EE, Linux,               |gls
91054 Buckenhof, Germany   -- +49 9131 506540  | Apache person              |eau
Open Source Consulting, Development, Design    | Velocity - Turbine guy     |rwc
                                                                            |m k
INTERMETA - Gesellschaft fuer Mehrwertdienste mbH - RG Fuerth, HRB 7350     |a s
Sitz der Gesellschaft: Buckenhof. Geschaeftsfuehrer: Henning Schmiedehausen |n

	       "Save the cheerleader. Save the world."

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


Re: [jira] Commented: (VELOCITY-536) Velocity Engine throws NullPointer Exception when two people click on the same page at the same time for the first time

Posted by Lei Gu <Le...@authoria.com>.
We are on JDK 1.5 and the memory model works for DCL if combined with
volatile key word. In JDK 1.5, volatile key word guarantees two threads see
the variable in the same order with the same value.
Thanks.
-- Lei
 

Velocity - Dev mailing list-2 wrote:
> 
> 
>     [
> https://issues.apache.org/jira/browse/VELOCITY-536?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12486071
> ] 
> 
> Nathan Bubna commented on VELOCITY-536:
> ---------------------------------------
> 
> But the DCL'ed block only needs to happen once per instance.  Isn't the
> whole point to make the second thread skip the block completely?   We only
> need one thread per instance to go through, period.   Once one thread has
> started going through the block, it will finish and keep other threads out
> of there for that instance, order notwithstanding.
> 
> And again, this is not a constructor nor a completely abstract use of DCL
> that we're talking about.  This is a very specific use.
> 
>> 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: dev-unsubscribe@velocity.apache.org
> For additional commands, e-mail: dev-help@velocity.apache.org
> 
> 
> 

-- 
View this message in context: http://www.nabble.com/-jira--Created%3A-%28VELOCITY-536%29-Velocity-Engine-throws-NullPointer-Exception-when-two-people-click-on-the-same-page-at-the-same-time-for-the-first-time-tf3506177.html#a9796621
Sent from the Velocity - Dev mailing list archive at Nabble.com.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


[jira] Commented: (VELOCITY-536) Velocity Engine throws NullPointer Exception when two people click on the same page at the same time for the first time

Posted by "Will Glass-Husain (JIRA)" <de...@velocity.apache.org>.
    [ https://issues.apache.org/jira/browse/VELOCITY-536?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12486043 ] 

Will Glass-Husain commented on VELOCITY-536:
--------------------------------------------

Thanks for this as well.  More info at:

http://www.mail-archive.com/dev@velocity.apache.org/msg01553.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: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


[jira] Commented: (VELOCITY-536) Velocity Engine throws NullPointer Exception when two people click on the same page at the same time for the first time

Posted by "Lei Gu (JIRA)" <de...@velocity.apache.org>.
    [ https://issues.apache.org/jira/browse/VELOCITY-536?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12504433 ] 

Lei Gu commented on VELOCITY-536:
---------------------------------

Hi Will,
Yes, that looks very reasonable to me. Thanks for taking care of this issue.
-- Lei

> 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
>             Fix For: 1.6
>
>         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: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


[jira] Commented: (VELOCITY-536) Velocity Engine throws NullPointer Exception when two people click on the same page at the same time for the first time

Posted by "Christopher Schultz (JIRA)" <de...@velocity.apache.org>.
    [ https://issues.apache.org/jira/browse/VELOCITY-536?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12486066 ] 

Christopher Schultz commented on VELOCITY-536:
----------------------------------------------

"Even though those instructions may be re-ordered, they should both be completed before another thread enters the block. So their order doesn't matter."

Since "init" is being checked before the synchronized block, the ordering of the "init=true" is certainly relevant. If the VM or JIT sets init=true before constructing the object, the other thread is hosed because it skips the synchronized block entirely and gets a reference that is potentially invalid.

JSR-133 has addressed re-orderings with respect to volatile members, and I'm not sure I fully understand why this fixes DCL, but, as Nathan points out, we cannot assume Java 1.5 availability, so we should avoid DCL at least for now.


> 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: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


[jira] Resolved: (VELOCITY-536) Velocity Engine throws NullPointer Exception when two people click on the same page at the same time for the first time

Posted by "Will Glass-Husain (JIRA)" <de...@velocity.apache.org>.
     [ https://issues.apache.org/jira/browse/VELOCITY-536?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Will Glass-Husain resolved VELOCITY-536.
----------------------------------------

    Resolution: Fixed

> 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
>             Fix For: 1.6
>
>         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: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


[jira] Commented: (VELOCITY-536) Velocity Engine throws NullPointer Exception when two people click on the same page at the same time for the first time

Posted by "Christopher Schultz (JIRA)" <de...@velocity.apache.org>.
    [ 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: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


[jira] Updated: (VELOCITY-536) Velocity Engine throws NullPointer Exception when two people click on the same page at the same time for the first time

Posted by "Lei Gu (JIRA)" <de...@velocity.apache.org>.
     [ https://issues.apache.org/jira/browse/VELOCITY-536?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Lei Gu updated VELOCITY-536:
----------------------------

    Attachment: 536-patch.txt

Patch fixes

> 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: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


[jira] Updated: (VELOCITY-536) Velocity Engine throws NullPointer Exception when two people click on the same page at the same time for the first time

Posted by "Will Glass-Husain (JIRA)" <de...@velocity.apache.org>.
     [ https://issues.apache.org/jira/browse/VELOCITY-536?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Will Glass-Husain updated VELOCITY-536:
---------------------------------------

    Fix Version/s: 1.6

> 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
>             Fix For: 1.6
>
>         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: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


[jira] Commented: (VELOCITY-536) Velocity Engine throws NullPointer Exception when two people click on the same page at the same time for the first time

Posted by "Will Glass-Husain (JIRA)" <de...@velocity.apache.org>.
    [ https://issues.apache.org/jira/browse/VELOCITY-536?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12522374 ] 

Will Glass-Husain commented on VELOCITY-536:
--------------------------------------------

patch applied - thanks again!

> 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
>             Fix For: 1.6
>
>         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: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


[jira] Commented: (VELOCITY-536) Velocity Engine throws NullPointer Exception when two people click on the same page at the same time for the first time

Posted by "Will Glass-Husain (JIRA)" <de...@velocity.apache.org>.
    [ https://issues.apache.org/jira/browse/VELOCITY-536?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12486045 ] 

Will Glass-Husain commented on VELOCITY-536:
--------------------------------------------

Incidentally, there's no need to attach the complete files - the patch file is enough.   (Using Eclipse or the utility "patch" it's easy to generate the modified source).

> 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: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


[jira] Commented: (VELOCITY-536) Velocity Engine throws NullPointer Exception when two people click on the same page at the same time for the first time

Posted by "Nathan Bubna (JIRA)" <de...@velocity.apache.org>.
    [ https://issues.apache.org/jira/browse/VELOCITY-536?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12486071 ] 

Nathan Bubna commented on VELOCITY-536:
---------------------------------------

But the DCL'ed block only needs to happen once per instance.  Isn't the whole point to make the second thread skip the block completely?   We only need one thread per instance to go through, period.   Once one thread has started going through the block, it will finish and keep other threads out of there for that instance, order notwithstanding.

And again, this is not a constructor nor a completely abstract use of DCL that we're talking about.  This is a very specific use.

> 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: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


[jira] Commented: (VELOCITY-536) Velocity Engine throws NullPointer Exception when two people click on the same page at the same time for the first time

Posted by "Nathan Bubna (JIRA)" <de...@velocity.apache.org>.
    [ https://issues.apache.org/jira/browse/VELOCITY-536?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12486054 ] 

Nathan Bubna commented on VELOCITY-536:
---------------------------------------

Null check?  I see boolean checks...

Anyway, double-checked locking is apparently OK in JDK 1.5 if you are checking a field declared as volatile.  I learned this here:
http://crazybob.org/2007/01/lazy-loading-singletons.html    Since we are not targetting JDK 1.5 yet, we probably should not use DCL.

Regardless of that, i thought the issues with double-checked locking were surrounding lack of atomicity in construction of the checked object (and perhaps other surprising memory model nuances).   Since the object being checked is an already created boolean primitive, i don't think this is a problem.  Also, it solved the problem they were experiencing...

Of course, i'm definitely not a memory model expert and would happily take correction here in order to learn better. :)

> 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: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


[jira] Commented: (VELOCITY-536) Velocity Engine throws NullPointer Exception when two people click on the same page at the same time for the first time

Posted by "Nathan Bubna (JIRA)" <de...@velocity.apache.org>.
    [ https://issues.apache.org/jira/browse/VELOCITY-536?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12486096 ] 

Nathan Bubna commented on VELOCITY-536:
---------------------------------------

Looking more carefully at this, rather than trusting my first look...

Ok, i see that there is potential for a second thread to jump ahead of the first one which is busy performing the meat of the init.  This second thread then might run into trouble if it tries to do anything with the instance before the first thread finishes the setup/initialization.

So, Christopher, you were right, there is an issue here, just not the original one.  Sorry, and thanks for pressing me to keep thinking about it. :)

So, *if* the user is running things on pre-1.5, then JIT/VM can re-order the setting of init=true within the synchronized block.   Then, *if* the VM decides to do that and another thread comes along and finds init=true, thus skipping the synchronization, it could pass up the original thread which is still busy performing the initialization.  *If* it then gets on to where it tries to use the incompletely initialized (but fully instantiated ;) instance, it could get into trouble.

So, while Lei's patch completely eliminates the chances of having the initialization code called multiple times, it does introduce the chance of even less-frequent (and thus much harder to debug) synchronization problems for those running on older JVMs.  Apply Murphy's Law, and we can be sure it will happen. :(   We'd better just synchronize the block/method, without trying to skip it via double checking.   If anyone's interested, we could leave a note that once we require 1.5, we can use DCL with the volatile keyword happily.

> 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: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


[jira] Updated: (VELOCITY-536) Velocity Engine throws NullPointer Exception when two people click on the same page at the same time for the first time

Posted by "Lei Gu (JIRA)" <de...@velocity.apache.org>.
     [ https://issues.apache.org/jira/browse/VELOCITY-536?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Lei Gu updated VELOCITY-536:
----------------------------

    Attachment: VelocimacroProxy.java
                ASTDirective.java
                ASTSetDirective.java

Fixes for the multi-threaded concurrency are to synchronized properly on init method on ASTSetDirective, ASTDirective, render method on ASTSetDirective and VelocimacroProxy.

> 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: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


[jira] Commented: (VELOCITY-536) Velocity Engine throws NullPointer Exception when two people click on the same page at the same time for the first time

Posted by "Nathan Bubna (JIRA)" <de...@velocity.apache.org>.
    [ https://issues.apache.org/jira/browse/VELOCITY-536?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12486062 ] 

Nathan Bubna commented on VELOCITY-536:
---------------------------------------

"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."

But the synchronization should only allow one thread at a time into the block in which setup() is called and init is set to true.   Even though those instructions may be re-ordered, they should both be completed before another thread enters the block.  So their order doesn't matter.  The reference being checked (the init boolean) will be set properly before a second thread comes to the double check of the init reference.

Please not that i'm not at all attached to using DCL here.   Christopher's suggestions are the best in the long run anyway.  Of course, someone needs to be willing to do that work and test it.   As things are, Lei has tested that this solution solves his problem, and i think it should be safe from the usual DCL pitfalls.  Until someone comes up with the superior patch, i'm inclined to take this one. :)

> 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: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


[jira] Commented: (VELOCITY-536) Velocity Engine throws NullPointer Exception when two people click on the same page at the same time for the first time

Posted by "Will Glass-Husain (JIRA)" <de...@velocity.apache.org>.
    [ https://issues.apache.org/jira/browse/VELOCITY-536?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12504115 ] 

Will Glass-Husain commented on VELOCITY-536:
--------------------------------------------

Hi Lei,

Went back and looked at this issue.  I'd like to commit, but I think the double-checked locking won't work.

I think if we change code like 
 
if(!init)
    {
      synchronized(this)
      {
         if ( !init )
         {
             nodeTree.init( context, rsvc);
                 init = true;
        }
}      		

to:

      synchronized(this)
      {
         if ( !init )
         {
             nodeTree.init( context, rsvc);
                 init = true;
        }

it'd be fine.  I'm really not worried about the performance hit of the synchronization. most things I've read says this isn't significant in modern JVM's.  

Seem reasonable?

> 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
>             Fix For: 1.6
>
>         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: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


[jira] Commented: (VELOCITY-536) Velocity Engine throws NullPointer Exception when two people click on the same page at the same time for the first time

Posted by "Henning Schmiedehausen (JIRA)" <de...@velocity.apache.org>.
    [ https://issues.apache.org/jira/browse/VELOCITY-536?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12487506 ] 

Henning Schmiedehausen commented on VELOCITY-536:
-------------------------------------------------

Hm, I'm late to that show. :-(  I did not read up all the thread, but please consider http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html which is the bible on why DCL is broken. Even though there is mentioned that it works for 32 bit integers, the summary is "this is not worth the trouble".

I'm completely with Christopher here. DCL code should be actively removed from a code base, not added. Even if it is the corner case for "32 bit primitives".

> 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: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org


[jira] Commented: (VELOCITY-536) Velocity Engine throws NullPointer Exception when two people click on the same page at the same time for the first time

Posted by "Will Glass-Husain (JIRA)" <de...@velocity.apache.org>.
    [ https://issues.apache.org/jira/browse/VELOCITY-536?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12486047 ] 

Will Glass-Husain commented on VELOCITY-536:
--------------------------------------------

Quick question on the patch.  Should the null check be inside the synchronized loop?  I worry this might fall into the anti-pattern "double-checked locking" when the null test is outside of the loop.  Anyone experienced in such matters want to take a look?

> 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: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org