[ 
https://issues.apache.org/jira/browse/HADOOP-10487?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13968858#comment-13968858
 ] 

Chris Nauroth commented on HADOOP-10487:
----------------------------------------

+1 for the proposal to change this.  I've not heard before that repeated access 
of the object prior to assignment can serve as an adequate protection.  I'd 
think it's still possible for a non-synchronized method to observe a non-null 
{{conf}} containing partial state if writes were reordered.  Even if it does 
work now, that might just be an implementation detail.

One challenge may be {{UserGroupInformation#reset}}, a testing method which 
wants to change {{conf}} back to null.

> Racy code in UserGroupInformation#ensureInitialized()
> -----------------------------------------------------
>
>                 Key: HADOOP-10487
>                 URL: https://issues.apache.org/jira/browse/HADOOP-10487
>             Project: Hadoop Common
>          Issue Type: Bug
>            Reporter: Haohui Mai
>            Assignee: Haohui Mai
>
> UserGroupInformation#ensureInitialized() uses the double-check-locking 
> pattern to reduce the synchronization cost:
> {code}
>   private static void ensureInitialized() {
>     if (conf == null) {
>       synchronized(UserGroupInformation.class) {
>         if (conf == null) { // someone might have beat us
>           initialize(new Configuration(), false);
>         }
>       }
>     }
>   }
> {code}
> As [~tlipcon] pointed out in the original jira (HADOOP-9748). This pattern is 
> incorrect. Please see more details in 
> http://en.wikipedia.org/wiki/Double-checked_locking and 
> http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html
> This jira proposes to use the static class holder pattern to do it correctly.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Reply via email to